Skip to content

Commit 48c318e

Browse files
pyricaupyricau
committed
Handling Manifest errors with a dedicated compile error and stops the annotation processing. androidannotations#327 
1 parent ce9fb5e commit 48c318e

7 files changed

Lines changed: 185 additions & 54 deletions

File tree

‎AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/AndroidAnnotationProcessor.java‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@
110110
import org.androidannotations.generation.CodeModelGenerator;
111111
import org.androidannotations.helper.AndroidManifest;
112112
import org.androidannotations.helper.AndroidManifestFinder;
113+
import org.androidannotations.helper.Option;
113114
import org.androidannotations.helper.TimeStats;
114115
import org.androidannotations.model.AndroidRes;
115116
import org.androidannotations.model.AndroidSystemServices;
@@ -355,19 +356,23 @@ private void processThrowing(Set<? extends TypeElement> annotations, RoundEnviro
355356

356357
AnnotationElementsHolder extractedModel = extractAnnotations(annotations, roundEnv);
357358

358-
AndroidManifest androidManifest = extractAndroidManifest();
359+
Option<AndroidManifest> androidManifestOption = extractAndroidManifest();
360+
361+
if (androidManifestOption.isAbsent()) {
362+
return;
363+
}
364+
365+
AndroidManifest androidManifest = androidManifestOption.get();
359366

360367
IRClass rClass = findRClasses(androidManifest);
361368

362369
AndroidSystemServices androidSystemServices = new AndroidSystemServices();
363370

364371
AnnotationElements validatedModel = validateAnnotations(extractedModel, rClass, androidSystemServices, androidManifest);
365372

366-
if (validatedModel != null) {
367-
ProcessResult processResult = processAnnotations(validatedModel, rClass, androidSystemServices, androidManifest);
373+
ProcessResult processResult = processAnnotations(validatedModel, rClass, androidSystemServices, androidManifest);
368374

369-
generateSources(processResult);
370-
}
375+
generateSources(processResult);
371376
}
372377

373378
private boolean nothingToDo(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
@@ -382,10 +387,10 @@ private AnnotationElementsHolder extractAnnotations(Set<? extends TypeElement> a
382387
return extractedModel;
383388
}
384389

385-
private AndroidManifest extractAndroidManifest() {
390+
private Option<AndroidManifest> extractAndroidManifest() {
386391
timeStats.start("Extract Manifest");
387392
AndroidManifestFinder finder = new AndroidManifestFinder(processingEnv);
388-
AndroidManifest manifest = finder.extractAndroidManifest();
393+
Option<AndroidManifest> manifest = finder.extractAndroidManifest();
389394
timeStats.stop("Extract Manifest");
390395
return manifest;
391396
}
@@ -408,13 +413,8 @@ private IRClass findRClasses(AndroidManifest androidManifest) throws IOException
408413

409414
private AnnotationElements validateAnnotations(AnnotationElementsHolder extractedModel, IRClass rClass, AndroidSystemServices androidSystemServices, AndroidManifest androidManifest) {
410415
timeStats.start("Validate Annotations");
411-
AnnotationElements validatedAnnotations;
412-
if (rClass != null) {
413-
ModelValidator modelValidator = buildModelValidator(rClass, androidSystemServices, androidManifest);
414-
validatedAnnotations = modelValidator.validate(extractedModel);
415-
} else {
416-
validatedAnnotations = null;
417-
}
416+
ModelValidator modelValidator = buildModelValidator(rClass, androidSystemServices, androidManifest);
417+
AnnotationElements validatedAnnotations = modelValidator.validate(extractedModel);
418418
timeStats.stop("Validate Annotations");
419419
return validatedAnnotations;
420420
}

‎AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/helper/AndroidManifestFinder.java‎

Lines changed: 55 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -50,19 +50,14 @@ public AndroidManifestFinder(ProcessingEnvironment processingEnv) {
5050
this.processingEnv = processingEnv;
5151
}
5252

53-
public AndroidManifest extractAndroidManifest() {
54-
try {
55-
return extractAndroidManifestThrowing();
56-
} catch (Exception e) {
57-
if (e instanceof RuntimeException) {
58-
throw (RuntimeException) e;
59-
}
60-
throw new RuntimeException(e);
53+
public Option<AndroidManifest> extractAndroidManifest() {
54+
Option<File> androidManifestFileOption = findManifestFile();
55+
56+
if (androidManifestFileOption.isAbsent()) {
57+
return Option.absent();
6158
}
62-
}
6359

64-
private AndroidManifest extractAndroidManifestThrowing() throws Exception {
65-
File androidManifestFile = findManifestFileThrowing();
60+
File androidManifestFile = androidManifestFileOption.get();
6661

6762
String projectDirectory = androidManifestFile.getParent();
6863

@@ -71,39 +66,41 @@ private AndroidManifest extractAndroidManifestThrowing() throws Exception {
7166
boolean libraryProject = false;
7267
if (projectProperties.exists()) {
7368
Properties properties = new Properties();
74-
properties.load(new FileInputStream(projectProperties));
75-
76-
if (properties.containsKey("android.library")) {
77-
String androidLibraryProperty = properties.getProperty("android.library");
78-
libraryProject = androidLibraryProperty.equals("true");
79-
80-
Messager messager = processingEnv.getMessager();
81-
messager.printMessage(Kind.NOTE, "Found android.library property in project.properties, value: " + libraryProject);
69+
try {
70+
properties.load(new FileInputStream(projectProperties));
71+
if (properties.containsKey("android.library")) {
72+
String androidLibraryProperty = properties.getProperty("android.library");
73+
libraryProject = androidLibraryProperty.equals("true");
74+
75+
Messager messager = processingEnv.getMessager();
76+
messager.printMessage(Kind.NOTE, "Found android.library property in project.properties, value: " + libraryProject);
77+
}
78+
} catch (IOException ignored) {
8279
}
83-
8480
}
8581

86-
return parseThrowing(androidManifestFile, libraryProject);
82+
return parse(androidManifestFile, libraryProject);
8783
}
8884

89-
private File findManifestFileThrowing() throws Exception {
85+
private Option<File> findManifestFile() {
9086
if (processingEnv.getOptions().containsKey(ANDROID_MANIFEST_FILE_OPTION)) {
9187
return findManifestInSpecifiedPath();
9288
} else {
9389
return findManifestInParentsDirectories();
9490
}
9591
}
9692

97-
private File findManifestInSpecifiedPath() {
93+
private Option<File> findManifestInSpecifiedPath() {
9894
String path = processingEnv.getOptions().get(ANDROID_MANIFEST_FILE_OPTION);
9995
File androidManifestFile = new File(path);
10096
Messager messager = processingEnv.getMessager();
10197
if (!androidManifestFile.exists()) {
102-
throw new IllegalStateException("Could not find the AndroidManifest.xml file in specified path : " + path);
98+
messager.printMessage(Kind.ERROR, "Could not find the AndroidManifest.xml file in specified path : " + path);
99+
return Option.absent();
103100
} else {
104101
messager.printMessage(Kind.NOTE, "AndroidManifest.xml file found: " + androidManifestFile.toString());
105102
}
106-
return androidManifestFile;
103+
return Option.of(androidManifestFile);
107104
}
108105

109106
/**
@@ -113,10 +110,18 @@ private File findManifestInSpecifiedPath() {
113110
* find the AndroidManifest.xml file. Any better solution will be
114111
* appreciated.
115112
*/
116-
private File findManifestInParentsDirectories() throws IOException, URISyntaxException {
113+
private Option<File> findManifestInParentsDirectories() {
117114
Filer filer = processingEnv.getFiler();
118115

119-
JavaFileObject dummySourceFile = filer.createSourceFile("dummy" + System.currentTimeMillis());
116+
Messager messager = processingEnv.getMessager();
117+
118+
JavaFileObject dummySourceFile;
119+
try {
120+
dummySourceFile = filer.createSourceFile("dummy" + System.currentTimeMillis());
121+
} catch (IOException ignored) {
122+
messager.printMessage(Kind.ERROR, "Could not find the AndroidManifest.xml file, unable to create a dummy source file to locate the source folder");
123+
return Option.absent();
124+
}
120125
String dummySourceFilePath = dummySourceFile.toUri().toString();
121126

122127
if (dummySourceFilePath.startsWith("file:")) {
@@ -127,10 +132,13 @@ private File findManifestInParentsDirectories() throws IOException, URISyntaxExc
127132
dummySourceFilePath = "file://" + dummySourceFilePath;
128133
}
129134

130-
Messager messager = processingEnv.getMessager();
131-
messager.printMessage(Kind.NOTE, "Dummy source file: " + dummySourceFilePath);
132-
133-
URI cleanURI = new URI(dummySourceFilePath);
135+
URI cleanURI;
136+
try {
137+
cleanURI = new URI(dummySourceFilePath);
138+
} catch (URISyntaxException e) {
139+
messager.printMessage(Kind.ERROR, "Could not find the AndroidManifest.xml file, path to dummy source file cannot be parsed: " + dummySourceFilePath);
140+
return Option.absent();
141+
}
134142

135143
File dummyFile = new File(cleanURI);
136144

@@ -153,27 +161,36 @@ private File findManifestInParentsDirectories() throws IOException, URISyntaxExc
153161
}
154162

155163
if (!androidManifestFile.exists()) {
156-
throw new IllegalStateException("Could not find the AndroidManifest.xml file, going up from path [" + sourcesGenerationFolder.getAbsolutePath() + "] found using dummy file [" + dummySourceFilePath + "] (max atempts: " + MAX_PARENTS_FROM_SOURCE_FOLDER + ")");
164+
messager.printMessage(Kind.ERROR, "Could not find the AndroidManifest.xml file, going up from path [" + sourcesGenerationFolder.getAbsolutePath() + "] found using dummy file [" + dummySourceFilePath + "] (max atempts: " + MAX_PARENTS_FROM_SOURCE_FOLDER + ")");
165+
return Option.absent();
157166
} else {
158167
messager.printMessage(Kind.NOTE, "AndroidManifest.xml file found: " + androidManifestFile.toString());
159168
}
160169

161-
return androidManifestFile;
170+
return Option.of(androidManifestFile);
162171
}
163172

164-
private AndroidManifest parseThrowing(File androidManifestFile, boolean libraryProject) throws Exception {
173+
private Option<AndroidManifest> parse(File androidManifestFile, boolean libraryProject) {
165174

166175
DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance();
167-
DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder();
168-
Document doc = docBuilder.parse(androidManifestFile);
176+
177+
Document doc;
178+
try {
179+
DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder();
180+
doc = docBuilder.parse(androidManifestFile);
181+
} catch (Exception e) {
182+
Messager messager = processingEnv.getMessager();
183+
messager.printMessage(Kind.ERROR, "Could not parse the AndroidManifest.xml file at path " + androidManifestFile);
184+
return Option.absent();
185+
}
169186

170187
Element documentElement = doc.getDocumentElement();
171188
documentElement.normalize();
172189

173190
String applicationPackage = documentElement.getAttribute("package");
174191

175192
if (libraryProject) {
176-
return AndroidManifest.createLibraryManifest(applicationPackage);
193+
return Option.of(AndroidManifest.createLibraryManifest(applicationPackage));
177194
}
178195

179196
NodeList applicationNodes = documentElement.getElementsByTagName("application");
@@ -214,7 +231,7 @@ private AndroidManifest parseThrowing(File androidManifestFile, boolean libraryP
214231
componentQualifiedNames.addAll(receiverQualifiedNames);
215232
componentQualifiedNames.addAll(providerQualifiedNames);
216233

217-
return AndroidManifest.createManifest(applicationPackage, applicationQualifiedName, componentQualifiedNames);
234+
return Option.of(AndroidManifest.createManifest(applicationPackage, applicationQualifiedName, componentQualifiedNames));
218235
}
219236

220237
private List<String> extractComponentNames(String applicationPackage, NodeList componentNodes) {

‎AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/helper/Option.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package org.androidannotations.helper;
2+
3+
/**
4+
* I'd love to use Guava's Optional, but we're trying to keep the dependency
5+
* level to a minimum.
6+
*/
7+
public class Option<T> {
8+
9+
private static final Option<?> ABSENT = new Option<Object>(null, false);
10+
11+
public static <T> Option<T> of(T value) {
12+
return new Option<T>(value, true);
13+
}
14+
15+
@SuppressWarnings("unchecked")
16+
public static <T> Option<T> absent() {
17+
return (Option<T>) ABSENT;
18+
}
19+
20+
private final T reference;
21+
22+
private final boolean isPresent;
23+
24+
private Option(T reference, boolean isPresent) {
25+
this.reference = reference;
26+
this.isPresent = isPresent;
27+
}
28+
29+
public boolean isPresent() {
30+
return isPresent;
31+
}
32+
33+
public boolean isAbsent() {
34+
return !isPresent;
35+
}
36+
37+
public T get() {
38+
if (!isPresent) {
39+
throw new IllegalStateException("value is absent");
40+
}
41+
return reference;
42+
}
43+
44+
public T getOr(T defaultValue) {
45+
if (isPresent) {
46+
return reference;
47+
} else {
48+
return defaultValue;
49+
}
50+
}
51+
52+
}

‎AndroidAnnotations/androidannotations/src/test/java/org/androidannotations/manifest/AndroidManifestFinderTest.java‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ public void setup() {
3838
@Test
3939
public void fails_if_no_manifest() throws Exception {
4040
CompileResult result = compileFiles(SomeClass.class);
41-
assertCompilationError(result);
41+
assertCompilationErrorWithNoSource(result);
42+
assertCompilationErrorCount(1, result);
4243
}
4344

4445
@Test
@@ -48,6 +49,14 @@ public void finds_specified_manifest() {
4849
assertCompilationSuccessful(result);
4950
}
5051

52+
@Test
53+
public void fails_if_cannot_find_specified_manifest() {
54+
addProcessorParameter("androidManifestFile", "/some/random/path/AndroidManifest.xml");
55+
CompileResult result = compileFiles(SomeClass.class);
56+
assertCompilationErrorWithNoSource(result);
57+
assertCompilationErrorCount(1, result);
58+
}
59+
5160
@Test
5261
public void finds_manifest_in_generated_source_parent_folder() throws Exception {
5362
copyManifestToParentOfOutputDirectory();
@@ -56,6 +65,14 @@ public void finds_manifest_in_generated_source_parent_folder() throws Exception
5665
deleteManifestFromParentOfOutputDirectory();
5766
}
5867

68+
@Test
69+
public void fails_if_cannot_parse_manifest() {
70+
addManifestProcessorParameter(AndroidManifestFinderTest.class, "ParseErrorManifest.xml");
71+
CompileResult result = compileFiles(SomeClass.class);
72+
assertCompilationErrorWithNoSource(result);
73+
assertCompilationErrorCount(1, result);
74+
}
75+
5976
private void deleteManifestFromParentOfOutputDirectory() {
6077
manifestFileInParentOfOutputDirectory().delete();
6178
}

‎AndroidAnnotations/androidannotations/src/test/java/org/androidannotations/utils/AAProcessorTestHelper.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@
2222
public class AAProcessorTestHelper extends ProcessorTestHelper {
2323

2424
public void addManifestProcessorParameter(Class<?> classOfPackagingContainingManifest) {
25-
String manifestPath = classOfPackagingContainingManifest.getResource("AndroidManifest.xml").getPath();
25+
addManifestProcessorParameter(classOfPackagingContainingManifest, "AndroidManifest.xml");
26+
}
27+
28+
public void addManifestProcessorParameter(Class<?> classOfPackagingContainingManifest, String manifestFileName) {
29+
String manifestPath = classOfPackagingContainingManifest.getResource(manifestFileName).getPath();
2630
addProcessorParameter("androidManifestFile", manifestPath);
2731
}
2832

‎AndroidAnnotations/androidannotations/src/test/java/org/androidannotations/utils/ProcessorTestHelper.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,28 @@ public static void assertCompilationError(CompileResult result) {
109109
fail("Expected a compilation error, diagnostics: " + result.diagnostics);
110110
}
111111

112+
public static void assertCompilationErrorWithNoSource(CompileResult result) {
113+
for (Diagnostic<? extends JavaFileObject> diagnostic : result.diagnostics) {
114+
if (diagnostic.getKind() == Kind.ERROR && diagnostic.getSource() == null) {
115+
return;
116+
}
117+
}
118+
fail("Expected a compilation error with no source, diagnostics: " + result.diagnostics);
119+
}
120+
121+
public static void assertCompilationErrorCount(int expectedErrorCount, CompileResult result) {
122+
int errorCount = 0;
123+
for (Diagnostic<? extends JavaFileObject> diagnostic : result.diagnostics) {
124+
if (diagnostic.getKind() == Kind.ERROR) {
125+
errorCount++;
126+
}
127+
}
128+
129+
if (errorCount != expectedErrorCount) {
130+
fail("Expected " + expectedErrorCount + " compilation error, found " + errorCount + " diagnostics: " + result.diagnostics);
131+
}
132+
}
133+
112134
public static void assertCompilationErrorOn(File expectedErrorClassFile, String expectedContentInError, CompileResult result) throws IOException {
113135
assertCompilationDiagnostingOn(Kind.ERROR, expectedErrorClassFile, expectedContentInError, result);
114136
}

‎AndroidAnnotations/androidannotations/src/test/resources/org/androidannotations/manifest/ParseErrorManifest.xml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<!--
3+
4+
Copyright (C) 2010-2012 eBusiness Information, Excilys Group
5+
6+
Licensed under the Apache License, Version 2.0 (the "License"); you may not
7+
use this file except in compliance with the License. You may obtain a copy of
8+
the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed To in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
14+
WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
15+
License for the specific language governing permissions and limitations under
16+
the License.
17+
18+
-->
19+
OMG This isn't XML!!

0 commit comments

Comments
 (0)