35049: @SuppressLint("NewApi") doesn't work on local variables

The @SuppressLint annotation can deliberately be placed on not only
classes and methods but on parameters and local variables too.

For AST-based (Java source-based) lint checks, this works fine.

However, some lint checks, such as the API Check, is based on
analyzing the bytecode. Annotations placed on local variables and
parameters do not make it into the .class file, so these annotations
do not work to suppress errors when placed on local variables (or
parameters).

The @TargetApi annotation applies only to the bytecode based API
check, so its metadata only allows the annotation to be placed on
methods and classes and constructors. However, the @SuppressLint
annotation needs to continue to be available for the AST-based checks.

This CL adds a new lint check, a "meta" check, which actually looks
for invalid @SuppressLint annotations, and warns about these. With the
new lint-on-save behavior, this means you instantly get feedback if
you attempt to suppress an annotation in the wrong place.  (Note that
the quickfix for adding annotations has always enforced this and
placed annotations out at the method level, but as shown in issue
35049, developers place them there deliberately themselves.)

This CL also fixes an unrelated problem (shown in issue 34198) that
the add suppress annotation code could sometimes add multiple versions
of the same id into the annotation.

Change-Id: I5bc61c6315edfcfc20103d1e580e389dd8e6a09b

1 files changed, 23 insertions, 1 deletions

diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/AddSuppressAnnotation.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/AddSuppressAnnotation.java
index 1280bc7..f755e1e 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/AddSuppressAnnotation.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/AddSuppressAnnotation.java
@@ -26,6 +26,7 @@ import static org.eclipse.jdt.core.dom.SingleMemberAnnotation.VALUE_PROPERTY;
 import com.android.ide.eclipse.adt.AdtPlugin;
 import com.android.ide.eclipse.adt.AdtUtils;
 import com.android.ide.eclipse.adt.internal.editors.IconFactory;
+import com.android.tools.lint.checks.AnnotationDetector;
 import com.android.tools.lint.checks.ApiDetector;
 import com.android.tools.lint.detector.api.Issue;
 import com.android.tools.lint.detector.api.Scope;
@@ -174,10 +175,14 @@ class AddSuppressAnnotation implements IMarkerResolution2 {
         } else {
             Expression existingValue = existing.getValue();
             if (existingValue instanceof StringLiteral) {
+                StringLiteral stringLiteral = (StringLiteral) existingValue;
+                if (mId.equals(stringLiteral.getLiteralValue())) {
+                    // Already contains the id
+                    return null;
+                }
                 // Create a new array initializer holding the old string plus the new id
                 ArrayInitializer array = ast.newArrayInitializer();
                 StringLiteral old = ast.newStringLiteral();
-                StringLiteral stringLiteral = (StringLiteral) existingValue;
                 old.setLiteralValue(stringLiteral.getLiteralValue());
                 array.expressions().add(old);
                 StringLiteral value = ast.newStringLiteral();
@@ -187,6 +192,17 @@ class AddSuppressAnnotation implements IMarkerResolution2 {
             } else if (existingValue instanceof ArrayInitializer) {
                 // Existing array: just append the new string
                 ArrayInitializer array = (ArrayInitializer) existingValue;
+                List expressions = array.expressions();
+                if (expressions != null) {
+                    for (Object o : expressions) {
+                        if (o instanceof StringLiteral) {
+                            if (mId.equals(((StringLiteral)o).getLiteralValue())) {
+                                // Already contains the id
+                                return null;
+                            }
+                        }
+                    }
+                }
                 StringLiteral value = ast.newStringLiteral();
                 value.setLiteralValue(mId);
                 ListRewrite listRewrite = rewriter.getListRewrite(array, EXPRESSIONS_PROPERTY);
@@ -282,6 +298,7 @@ class AddSuppressAnnotation implements IMarkerResolution2 {
         if (document == null) {
             return;
         }
+
         IWorkingCopyManager manager = JavaUI.getWorkingCopyManager();
         ICompilationUnit compilationUnit = manager.getWorkingCopy(editorInput);
         int offset = 0;
@@ -311,6 +328,11 @@ class AddSuppressAnnotation implements IMarkerResolution2 {
         Issue issue = EclipseLintClient.getRegistry().getIssue(id);
         boolean isClassDetector = issue != null && issue.getScope().contains(Scope.CLASS_FILE);
 
+        // Don't offer to suppress (with an annotation) the annotation checks
+        if (issue == AnnotationDetector.ISSUE) {
+            return;
+        }
+
         NodeFinder nodeFinder = new NodeFinder(root, offset, length);
         ASTNode coveringNode;
         if (offset <= 0) {