diff options
6 files changed, 277 insertions, 2 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) { diff --git a/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java b/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java index b4e4a7d..08a5c19 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/client/api/LintDriver.java @@ -175,6 +175,16 @@ public class LintDriver { } /** + * Returns the current {@link IssueRegistry}. + * + * @return the current {@link IssueRegistry} + */ + @NonNull + public IssueRegistry getRegistry() { + return mRegistry; + } + + /** * Returns the project containing a given file, or null if not found. This searches * only among the currently checked project and its library projects, not among all * possible projects being scanned sequentially. diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/AnnotationDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/AnnotationDetector.java new file mode 100644 index 0000000..aa0c84d --- /dev/null +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/AnnotationDetector.java @@ -0,0 +1,169 @@ +/* + * Copyright (C) 2012 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 com.android.tools.lint.checks; + +import static com.android.tools.lint.detector.api.LintConstants.FQCN_SUPPRESS_LINT; +import static com.android.tools.lint.detector.api.LintConstants.SUPPRESS_LINT; + +import com.android.annotations.NonNull; +import com.android.tools.lint.client.api.IssueRegistry; +import com.android.tools.lint.detector.api.Category; +import com.android.tools.lint.detector.api.Context; +import com.android.tools.lint.detector.api.Detector; +import com.android.tools.lint.detector.api.Issue; +import com.android.tools.lint.detector.api.JavaContext; +import com.android.tools.lint.detector.api.Scope; +import com.android.tools.lint.detector.api.Severity; +import com.android.tools.lint.detector.api.Speed; + +import java.io.File; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; + +import lombok.ast.Annotation; +import lombok.ast.AnnotationElement; +import lombok.ast.AnnotationValue; +import lombok.ast.ArrayInitializer; +import lombok.ast.AstVisitor; +import lombok.ast.Expression; +import lombok.ast.ForwardingAstVisitor; +import lombok.ast.Modifiers; +import lombok.ast.Node; +import lombok.ast.StrictListAccessor; +import lombok.ast.StringLiteral; +import lombok.ast.VariableDefinition; + +/** + * Checks annotations to make sure they are valid + */ +public class AnnotationDetector extends Detector implements Detector.JavaScanner { + /** Placing SuppressLint on a local variable doesn't work for class-file based checks */ + public static final Issue ISSUE = Issue.create( + "LocalSuppress", //$NON-NLS-1$ + "Looks for @SuppressLint annotations in locations where it doesn't work for class based checks", + + "The @SuppressAnnotation is used to suppress Lint warnings in Java files. However, " + + "while many lint checks analyzes the Java source code, where they can find " + + "annotations on (for example) local variables, some checks are analyzing the " + + ".class files. And in class files, annotations only appear on classes, fields " + + "and methods. Annotations placed on local variables disappear. If you attempt " + + "to suppress a lint error for a class-file based lint check, the suppress " + + "annotation not work. You must move the annotation out to the surrounding method.", + + Category.CORRECTNESS, + 3, + Severity.ERROR, + AnnotationDetector.class, + Scope.JAVA_FILE_SCOPE); + + /** Constructs a new {@link AnnotationDetector} check */ + public AnnotationDetector() { + } + + @Override + public boolean appliesTo(@NonNull Context context, @NonNull File file) { + return true; + } + + @Override + public @NonNull Speed getSpeed() { + return Speed.FAST; + } + + // ---- Implements JavaScanner ---- + + @Override + public List<Class<? extends Node>> getApplicableNodeTypes() { + return Collections.<Class<? extends Node>>singletonList(lombok.ast.Annotation.class); + } + + @Override + public AstVisitor createJavaVisitor(@NonNull JavaContext context) { + return new AnnotationChecker(context); + } + + private static class AnnotationChecker extends ForwardingAstVisitor { + private final JavaContext mContext; + + public AnnotationChecker(JavaContext context) { + mContext = context; + } + + @Override + public boolean visitAnnotation(Annotation node) { + String type = node.astAnnotationTypeReference().getTypeName(); + if (SUPPRESS_LINT.equals(type) || FQCN_SUPPRESS_LINT.equals(type)) { + Node parent = node.getParent(); + if (parent instanceof Modifiers) { + parent = parent.getParent(); + if (parent instanceof VariableDefinition) { + for (AnnotationElement element : node.astElements()) { + AnnotationValue valueNode = element.astValue(); + if (valueNode == null) { + continue; + } + if (valueNode instanceof StringLiteral) { + StringLiteral literal = (StringLiteral) valueNode; + String id = literal.astValue(); + if (!checkId(node, id)) { + return super.visitAnnotation(node); + } + } else if (valueNode instanceof ArrayInitializer) { + ArrayInitializer array = (ArrayInitializer) valueNode; + StrictListAccessor<Expression, ArrayInitializer> expressions = + array.astExpressions(); + if (expressions == null) { + continue; + } + Iterator<Expression> arrayIterator = expressions.iterator(); + while (arrayIterator.hasNext()) { + Expression arrayElement = arrayIterator.next(); + if (arrayElement instanceof StringLiteral) { + String id = ((StringLiteral) arrayElement).astValue(); + if (!checkId(node, id)) { + return super.visitAnnotation(node); + } + } + } + } + } + } + } + } + + return super.visitAnnotation(node); + } + + private boolean checkId(Annotation node, String id) { + IssueRegistry registry = mContext.getDriver().getRegistry(); + Issue issue = registry.getIssue(id); + if (issue != null && !issue.getScope().contains(Scope.JAVA_FILE)) { + // This issue doesn't have AST access: annotations are not + // available for local variables or parameters + mContext.report(ISSUE,mContext.getLocation(node), String.format( + "The @SuppresLint annotation cannot be used on a local" + + " variable with the lint check '%1$s': move out to the " + + "surrounding method", id), + null); + return false; + } + + return true; + } + } +} diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java index 1dec5f6..dd1e973 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java @@ -54,7 +54,7 @@ public class BuiltinIssueRegistry extends IssueRegistry { private static final List<Issue> sIssues; static { - final int initialCapacity = 100; + final int initialCapacity = 101; List<Issue> issues = new ArrayList<Issue>(initialCapacity); issues.add(AccessibilityDetector.ISSUE); @@ -157,6 +157,7 @@ public class BuiltinIssueRegistry extends IssueRegistry { issues.add(SharedPrefsDetector.ISSUE); issues.add(NonInternationalizedSmsDetector.ISSUE); issues.add(PrivateKeyDetector.ISSUE); + issues.add(AnnotationDetector.ISSUE); assert initialCapacity >= issues.size() : issues.size(); diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AnnotationDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AnnotationDetectorTest.java new file mode 100644 index 0000000..962b559 --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AnnotationDetectorTest.java @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2012 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 com.android.tools.lint.checks; + +import com.android.tools.lint.detector.api.Detector; +import com.android.tools.lint.detector.api.Issue; + +import java.util.List; + +@SuppressWarnings("javadoc") +public class AnnotationDetectorTest extends AbstractCheckTest { + public void test() throws Exception { + assertEquals( + "WrongAnnotation.java:11: Error: The @SuppresLint annotation cannot be used on a local variable with the lint check 'NewApi': move out to the surrounding method\n" + + "WrongAnnotation.java:13: Error: The @SuppresLint annotation cannot be used on a local variable with the lint check 'NewApi': move out to the surrounding method\n" + + "WrongAnnotation.java:8: Error: The @SuppresLint annotation cannot be used on a local variable with the lint check 'NewApi': move out to the surrounding method\n" + + "WrongAnnotation.java:9: Error: The @SuppresLint annotation cannot be used on a local variable with the lint check 'NewApi': move out to the surrounding method", + + lintProject( + "src/test/pkg/WrongAnnotation.java.txt=>src/test/pkg/WrongAnnotation.java" + )); + } + + @Override + protected Detector getDetector() { + return new AnnotationDetector(); + } + + @Override + protected List<Issue> getIssues() { + List<Issue> issues = super.getIssues(); + + // Need these issues on to be found by the registry as well to look up scope + // in id references (these ids are referenced in the unit test java file below) + issues.add(ApiDetector.UNSUPPORTED); + issues.add(SdCardDetector.ISSUE); + + return issues; + } +} diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/WrongAnnotation.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/WrongAnnotation.java.txt new file mode 100644 index 0000000..9256055 --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/WrongAnnotation.java.txt @@ -0,0 +1,19 @@ +package com.example.test2; + +import android.annotation.SuppressLint; + +public class WrongAnnotation { + @Override + @SuppressLint("NewApi") // Valid: class-file check on method + public static void foobar(View view, @SuppressLint("NewApi") int foo) { // Invalid: class-file check + @SuppressLint("NewApi") // Invalid + boolean a; + @SuppressLint({"SdCardPath", "NewApi"}) // Invalid: class-file based check on local variable + boolean b; + @android.annotation.SuppressLint({"SdCardPath", "NewApi"}) // Invalid (FQN) + boolean c; + @SuppressLint("SdCardPath") // Valid: AST-based check + boolean d; + } +} + |