diff options
author | Mathew Inwood <mathewi@google.com> | 2019-11-29 15:30:52 +0000 |
---|---|---|
committer | Mathew Inwood <mathewi@google.com> | 2019-12-02 11:42:07 +0000 |
commit | 2867ddf511431286b16224af4552300f178a14b0 (patch) | |
tree | 0905e10fe0b46353f15fa7f6e3ebd96daaac769d /tools/processors | |
parent | 82fee5847845950fcf6ac1be2c057f269a9a7b76 (diff) |
Handle overrideSourcePosition in unsupportedappusageprocessor.
The property is currently set by the AIDL compiler to indicate the original
position of the annotation in the AIDL file. Use this position in
preference to the annotation position within Java code if it's present.
Also rework the tests to use the google compile-testing library rather than
my crufty javac library. This makes testing failure conditions much nicer.
Bug: 145120552
Test: atest unsupportedappusage-processor-test
Test: m framework-annotation-proc
Change-Id: Icaac9b40672dce028095b578c19950b688506c0d
Diffstat (limited to 'tools/processors')
4 files changed, 174 insertions, 38 deletions
diff --git a/tools/processors/unsupportedappusage/src/android/processor/unsupportedappusage/SourcePosition.java b/tools/processors/unsupportedappusage/src/android/processor/unsupportedappusage/SourcePosition.java new file mode 100644 index 000000000000..4ae093c3c2e4 --- /dev/null +++ b/tools/processors/unsupportedappusage/src/android/processor/unsupportedappusage/SourcePosition.java @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.processor.unsupportedappusage; + +/** + * Represents a source position within a source file + */ +public class SourcePosition { + public final String filename; + public final int startLine; + public final int startCol; + public final int endLine; + public final int endCol; + + public SourcePosition(String filename, int startLine, int startCol, int endLine, int endCol) { + this.filename = filename; + this.startLine = startLine; + this.startCol = startCol; + this.endLine = endLine; + this.endCol = endCol; + } +} diff --git a/tools/processors/unsupportedappusage/src/android/processor/unsupportedappusage/UnsupportedAppUsageProcessor.java b/tools/processors/unsupportedappusage/src/android/processor/unsupportedappusage/UnsupportedAppUsageProcessor.java index 5bb956a1fea2..ca2c2759d702 100644 --- a/tools/processors/unsupportedappusage/src/android/processor/unsupportedappusage/UnsupportedAppUsageProcessor.java +++ b/tools/processors/unsupportedappusage/src/android/processor/unsupportedappusage/UnsupportedAppUsageProcessor.java @@ -43,6 +43,7 @@ import javax.lang.model.element.AnnotationValue; import javax.lang.model.element.Element; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; +import javax.tools.Diagnostic; /** * Annotation processor for {@link UnsupportedAppUsage} annotations. @@ -68,6 +69,7 @@ public class UnsupportedAppUsageProcessor extends AbstractProcessor { private static final ImmutableSet<String> SUPPORTED_ANNOTATION_NAMES = SUPPORTED_ANNOTATIONS.stream().map(annotation -> annotation.getCanonicalName()).collect( ImmutableSet.toImmutableSet()); + private static final String OVERRIDE_SOURCE_POSITION_PROPERTY = "overrideSourcePosition"; @Override public SourceVersion getSupportedSourceVersion() { @@ -126,6 +128,9 @@ public class UnsupportedAppUsageProcessor extends AbstractProcessor { StringBuilder sb = new StringBuilder(); for (Map.Entry<? extends ExecutableElement, ? extends AnnotationValue> e : annotation.getElementValues().entrySet()) { + if (e.getKey().getSimpleName().toString().equals(OVERRIDE_SOURCE_POSITION_PROPERTY)) { + continue; + } if (sb.length() > 0) { sb.append("&"); } @@ -136,6 +141,34 @@ public class UnsupportedAppUsageProcessor extends AbstractProcessor { return sb.toString(); } + private SourcePosition getSourcePositionOverride( + Element annotatedElement, AnnotationMirror annotation) { + for (Map.Entry<? extends ExecutableElement, ? extends AnnotationValue> e + : annotation.getElementValues().entrySet()) { + if (e.getKey().getSimpleName().toString().equals(OVERRIDE_SOURCE_POSITION_PROPERTY)) { + String[] position = e.getValue().getValue().toString().split(":"); + if (position.length != 5) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, String.format( + "Expected %s to have format file:startLine:startCol:endLine:endCol", + OVERRIDE_SOURCE_POSITION_PROPERTY), annotatedElement, annotation); + return null; + } + try { + return new SourcePosition(position[0], Integer.parseInt(position[1]), + Integer.parseInt(position[2]), Integer.parseInt(position[3]), + Integer.parseInt(position[4])); + } catch (NumberFormatException nfe) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, String.format( + "Expected %s to have format file:startLine:startCol:endLine:endCol; " + + "error parsing integer: %s", OVERRIDE_SOURCE_POSITION_PROPERTY, + nfe.getMessage()), annotatedElement, annotation); + return null; + } + } + } + return null; + } + /** * Maps an annotated element to the source position of the @UnsupportedAppUsage annotation * attached to it. It returns CSV in the format: @@ -152,16 +185,25 @@ public class UnsupportedAppUsageProcessor extends AbstractProcessor { JavacElements javacElem = (JavacElements) processingEnv.getElementUtils(); AnnotationMirror unsupportedAppUsage = getUnsupportedAppUsageAnnotationMirror(annotatedElement); - Pair<JCTree, JCTree.JCCompilationUnit> pair = - javacElem.getTreeAndTopLevel(annotatedElement, unsupportedAppUsage, null); - Position.LineMap lines = pair.snd.lineMap; + SourcePosition position = getSourcePositionOverride(annotatedElement, unsupportedAppUsage); + if (position == null) { + Pair<JCTree, JCTree.JCCompilationUnit> pair = + javacElem.getTreeAndTopLevel(annotatedElement, unsupportedAppUsage, null); + Position.LineMap lines = pair.snd.lineMap; + position = new SourcePosition( + pair.snd.getSourceFile().getName(), + lines.getLineNumber(pair.fst.pos().getStartPosition()), + lines.getColumnNumber(pair.fst.pos().getStartPosition()), + lines.getLineNumber(pair.fst.pos().getEndPosition(pair.snd.endPositions)), + lines.getColumnNumber(pair.fst.pos().getEndPosition(pair.snd.endPositions))); + } return Joiner.on(",").join( signature, - pair.snd.getSourceFile().getName(), - lines.getLineNumber(pair.fst.pos().getStartPosition()), - lines.getColumnNumber(pair.fst.pos().getStartPosition()), - lines.getLineNumber(pair.fst.pos().getEndPosition(pair.snd.endPositions)), - lines.getColumnNumber(pair.fst.pos().getEndPosition(pair.snd.endPositions)), + position.filename, + position.startLine, + position.startCol, + position.endLine, + position.endCol, encodeAnnotationProperties(unsupportedAppUsage)); } diff --git a/tools/processors/unsupportedappusage/test/Android.bp b/tools/processors/unsupportedappusage/test/Android.bp index 49ea3d4bbc96..e10fd47e8db1 100644 --- a/tools/processors/unsupportedappusage/test/Android.bp +++ b/tools/processors/unsupportedappusage/test/Android.bp @@ -18,7 +18,7 @@ java_test_host { srcs: ["src/**/*.java"], static_libs: [ - "libjavac", + "compile-testing-prebuilt", "unsupportedappusage-annotation-processor-lib", "truth-host-prebuilt", "mockito-host", diff --git a/tools/processors/unsupportedappusage/test/src/android/processor/unsupportedappusage/UnsupportedAppUsageProcessorTest.java b/tools/processors/unsupportedappusage/test/src/android/processor/unsupportedappusage/UnsupportedAppUsageProcessorTest.java index 012e88f17924..75158eebdfdd 100644 --- a/tools/processors/unsupportedappusage/test/src/android/processor/unsupportedappusage/UnsupportedAppUsageProcessorTest.java +++ b/tools/processors/unsupportedappusage/test/src/android/processor/unsupportedappusage/UnsupportedAppUsageProcessorTest.java @@ -18,61 +18,67 @@ package android.processor.unsupportedappusage; import static com.google.common.truth.Truth.assertThat; -import com.android.javac.Javac; +import com.google.testing.compile.Compilation; +import com.google.testing.compile.CompilationSubject; +import com.google.testing.compile.Compiler; +import com.google.testing.compile.JavaFileObjects; -import com.google.common.base.Joiner; - -import org.junit.Before; import org.junit.Test; import java.io.IOException; import java.util.Map; +import java.util.Optional; + +import javax.tools.JavaFileObject; +import javax.tools.StandardLocation; public class UnsupportedAppUsageProcessorTest { - private Javac mJavac; - - @Before - public void setup() throws IOException { - mJavac = new Javac(); - mJavac.addSource("dalvik.annotation.compat.UnsupportedAppUsage", Joiner.on('\n').join( - "package dalvik.annotation.compat;", - "public @interface UnsupportedAppUsage {", - " String expectedSignature() default \"\";\n", - " String someProperty() default \"\";", - "}")); - } + private static final JavaFileObject ANNOTATION = JavaFileObjects.forSourceLines( + "dalvik.dalvik.annotation.compat.UnsupportedAppUsage", + "package dalvik.annotation.compat;", + "public @interface UnsupportedAppUsage {", + " String expectedSignature() default \"\";\n", + " String someProperty() default \"\";", + " String overrideSourcePosition() default \"\";", + "}"); - private CsvReader compileAndReadCsv() throws IOException { - mJavac.compileWithAnnotationProcessor(new UnsupportedAppUsageProcessor()); - return new CsvReader( - mJavac.getOutputFile("unsupportedappusage/unsupportedappusage_index.csv")); + private CsvReader compileAndReadCsv(JavaFileObject source) throws IOException { + Compilation compilation = + Compiler.javac().withProcessors(new UnsupportedAppUsageProcessor()) + .compile(ANNOTATION, source); + CompilationSubject.assertThat(compilation).succeeded(); + Optional<JavaFileObject> csv = compilation.generatedFile(StandardLocation.CLASS_OUTPUT, + "unsupportedappusage/unsupportedappusage_index.csv"); + assertThat(csv.isPresent()).isTrue(); + + return new CsvReader(csv.get().openInputStream()); } @Test public void testSignatureFormat() throws Exception { - mJavac.addSource("a.b.Class", Joiner.on('\n').join( + JavaFileObject src = JavaFileObjects.forSourceLines("a.b.Class", "package a.b;", "import dalvik.annotation.compat.UnsupportedAppUsage;", "public class Class {", " @UnsupportedAppUsage", " public void method() {}", - "}")); - assertThat(compileAndReadCsv().getContents().get(0)).containsEntry( + "}"); + assertThat(compileAndReadCsv(src).getContents().get(0)).containsEntry( "signature", "La/b/Class;->method()V" ); } @Test public void testSourcePosition() throws Exception { - mJavac.addSource("a.b.Class", Joiner.on('\n').join( + JavaFileObject src = JavaFileObjects.forSourceLines("a.b.Class", "package a.b;", // 1 "import dalvik.annotation.compat.UnsupportedAppUsage;", // 2 "public class Class {", // 3 " @UnsupportedAppUsage", // 4 " public void method() {}", // 5 - "}")); - Map<String, String> row = compileAndReadCsv().getContents().get(0); + "}"); + Map<String, String> row = compileAndReadCsv(src).getContents().get(0); assertThat(row).containsEntry("startline", "4"); assertThat(row).containsEntry("startcol", "3"); assertThat(row).containsEntry("endline", "4"); @@ -81,16 +87,68 @@ public class UnsupportedAppUsageProcessorTest { @Test public void testAnnotationProperties() throws Exception { - mJavac.addSource("a.b.Class", Joiner.on('\n').join( + JavaFileObject src = JavaFileObjects.forSourceLines("a.b.Class", "package a.b;", // 1 "import dalvik.annotation.compat.UnsupportedAppUsage;", // 2 "public class Class {", // 3 " @UnsupportedAppUsage(someProperty=\"value\")", // 4 " public void method() {}", // 5 - "}")); - assertThat(compileAndReadCsv().getContents().get(0)).containsEntry( + "}"); + assertThat(compileAndReadCsv(src).getContents().get(0)).containsEntry( "properties", "someProperty=%22value%22"); } + @Test + public void testSourcePositionOverride() throws Exception { + JavaFileObject src = JavaFileObjects.forSourceLines("a.b.Class", + "package a.b;", // 1 + "import dalvik.annotation.compat.UnsupportedAppUsage;", // 2 + "public class Class {", // 3 + " @UnsupportedAppUsage(overrideSourcePosition=\"otherfile.aidl:30:10:31:20\")", + " public void method() {}", // 5 + "}"); + Map<String, String> row = compileAndReadCsv(src).getContents().get(0); + assertThat(row).containsEntry("file", "otherfile.aidl"); + assertThat(row).containsEntry("startline", "30"); + assertThat(row).containsEntry("startcol", "10"); + assertThat(row).containsEntry("endline", "31"); + assertThat(row).containsEntry("endcol", "20"); + assertThat(row).containsEntry("properties", ""); + } + + @Test + public void testSourcePositionOverrideWrongFormat() throws Exception { + JavaFileObject src = JavaFileObjects.forSourceLines("a.b.Class", + "package a.b;", // 1 + "import dalvik.annotation.compat.UnsupportedAppUsage;", // 2 + "public class Class {", // 3 + " @UnsupportedAppUsage(overrideSourcePosition=\"invalid\")", // 4 + " public void method() {}", // 5 + "}"); + Compilation compilation = + Compiler.javac().withProcessors(new UnsupportedAppUsageProcessor()) + .compile(ANNOTATION, src); + CompilationSubject.assertThat(compilation).failed(); + CompilationSubject.assertThat(compilation).hadErrorContaining( + "Expected overrideSourcePosition to have format " + + "file:startLine:startCol:endLine:endCol").inFile(src).onLine(4); + } + + @Test + public void testSourcePositionOverrideInvalidInt() throws Exception { + JavaFileObject src = JavaFileObjects.forSourceLines("a.b.Class", + "package a.b;", // 1 + "import dalvik.annotation.compat.UnsupportedAppUsage;", // 2 + "public class Class {", // 3 + " @UnsupportedAppUsage(overrideSourcePosition=\"otherfile.aidl:a:b:c:d\")", // 4 + " public void method() {}", // 5 + "}"); + Compilation compilation = + Compiler.javac().withProcessors(new UnsupportedAppUsageProcessor()) + .compile(ANNOTATION, src); + CompilationSubject.assertThat(compilation).failed(); + CompilationSubject.assertThat(compilation).hadErrorContaining( + "error parsing integer").inFile(src).onLine(4); + } } |