Merge "Add override detector"

9 files changed, 201 insertions, 2 deletions

diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java
index 159180d..2b3ce34 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/ClassContext.java
@@ -26,6 +26,7 @@ import static com.android.tools.lint.detector.api.Location.SearchDirection.FORWA
 import com.android.annotations.NonNull;
 import com.android.annotations.Nullable;
 import com.android.tools.lint.client.api.LintDriver;
+import com.android.tools.lint.detector.api.Location.SearchDirection;
 import com.android.tools.lint.detector.api.Location.SearchHints;
 import com.google.common.annotations.Beta;
 
@@ -521,18 +522,21 @@ public class ClassContext extends Context {
         // to find a method, look up the corresponding line number then search
         // around it for a suitable tag, such as the class name.
         String pattern;
+        SearchDirection searchMode;
         if (methodNode.name.equals(CONSTRUCTOR_NAME)) {
+            searchMode = EOL_BACKWARD;
             if (isAnonymousClass(classNode.name)) {
                 pattern = classNode.superName.substring(classNode.superName.lastIndexOf('/') + 1);
             } else {
                 pattern = classNode.name.substring(classNode.name.lastIndexOf('$') + 1);
             }
         } else {
+            searchMode = BACKWARD;
             pattern = methodNode.name;
         }
 
         return getLocationForLine(findLineNumber(methodNode), pattern, null,
-                SearchHints.create(EOL_BACKWARD).matchJavaSymbol());
+                SearchHints.create(searchMode).matchJavaSymbol());
     }
 
     /**
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Project.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Project.java
index c29cdf3..b2ac0d1 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Project.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/Project.java
@@ -72,6 +72,7 @@ public class Project {
     private String mPackage;
     private int mMinSdk = 1;
     private int mTargetSdk = -1;
+    private int mBuildSdk = -1;
     private boolean mLibrary;
     private String mName;
     private String mProguardPath;
@@ -131,6 +132,22 @@ public class Project {
                     mProguardPath = properties.getProperty(PROGUARD_CONFIG);
                     mMergeManifests = VALUE_TRUE.equals(properties.getProperty(
                             "manifestmerger.enabled")); //$NON-NLS-1$
+                    String target = properties.getProperty("target"); //$NON-NLS-1$
+                    if (target != null) {
+                        int index = target.lastIndexOf('-');
+                        if (index == -1) {
+                            index = target.lastIndexOf(':');
+                        }
+                        if (index != -1) {
+                            String versionString = target.substring(index + 1);
+                            try {
+                                mBuildSdk = Integer.parseInt(versionString);
+                            } catch (NumberFormatException nufe) {
+                                mClient.log(Severity.WARNING, null,
+                                        "Unexpected build target format: %1$s", target);
+                            }
+                        }
+                    }
 
                     for (int i = 1; i < 1000; i++) {
                         String key = String.format(ANDROID_LIBRARY_REFERENCE_FORMAT, i);
@@ -468,6 +485,15 @@ public class Project {
     }
 
     /**
+     * Returns the target API used to build the project, or -1 if not known
+     *
+     * @return the build target API or -1 if unknown
+     */
+    public int getBuildSdk() {
+        return mBuildSdk;
+    }
+
+    /**
      * Initialized the manifest state from the given manifest model
      *
      * @param document the DOM document for the manifest XML document
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java
index e045dd4..46e330e 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ApiDetector.java
@@ -114,6 +114,32 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc
             .addAnalysisScope(Scope.RESOURCE_FILE_SCOPE)
             .addAnalysisScope(Scope.CLASS_FILE_SCOPE);
 
+    /** Accessing an unsupported API */
+    public static final Issue OVERRIDE = Issue.create("Override", //$NON-NLS-1$
+            "Finds method declarations that will accidentally override methods in later versions",
+
+            "Suppose you are building against Android API 8, and you've subclassed Activity. " +
+            "In your subclass you add a new method called `isDestroyed`(). At some later point, " +
+            "a method of the same name and signature is added to Android. Your method will " +
+            "now override the Android method, and possibly break its contract. Your method " +
+            "is not calling `super.isDestroyed()`, since your compilation target doesn't " +
+            "know about the method.\n" +
+            "\n" +
+            "The above scenario is what this lint detector looks for. The above example is " +
+            "real, since `isDestroyed()` was added in API 17, but it will be true for *any* " +
+            "method you have added to a subclass of an Android class where your build target " +
+            "is lower than the version the method was introduced in.\n" +
+            "\n" +
+            "To fix this, either rename your method, or if you are really trying to augment " +
+            "the builtin method if available, switch to a higher build target where you can " +
+            "deliberately add `@Override` on your overriding method, and call `super` if " +
+            "appropriate etc.\n",
+            Category.CORRECTNESS,
+            6,
+            Severity.ERROR,
+            ApiDetector.class,
+            Scope.CLASS_FILE_SCOPE);
+
     private static final String TARGET_API_VMSIG = '/' + TARGET_API + ';';
     private static final String SWITCH_TABLE_PREFIX = "$SWITCH_TABLE$";  //$NON-NLS-1$
     private static final String ORDINAL_METHOD = "ordinal"; //$NON-NLS-1$
@@ -310,6 +336,33 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc
         }
 
         List methodList = classNode.methods;
+        if (methodList.isEmpty()) {
+            return;
+        }
+
+        boolean checkCalls = context.isEnabled(UNSUPPORTED);
+        boolean checkMethods = context.isEnabled(OVERRIDE)
+                && context.getMainProject().getBuildSdk() >= 1;
+        String frameworkParent = null;
+        if (checkMethods) {
+            LintDriver driver = context.getDriver();
+            String owner = classNode.superName;
+            while (owner != null) {
+                // For virtual dispatch, walk up the inheritance chain checking
+                // each inherited method
+                if (owner.startsWith("android/")           //$NON-NLS-1$
+                        || owner.startsWith("java/")       //$NON-NLS-1$
+                        || owner.startsWith("javax/")) {   //$NON-NLS-1$
+                    frameworkParent = owner;
+                    break;
+                }
+                owner = driver.getSuperClass(owner);
+            }
+            if (frameworkParent == null) {
+                checkMethods = false;
+            }
+        }
+
         for (Object m : methodList) {
             MethodNode method = (MethodNode) m;
 
@@ -320,6 +373,35 @@ public class ApiDetector extends ResourceXmlDetector implements Detector.ClassSc
 
             InsnList nodes = method.instructions;
 
+            if (checkMethods && Character.isJavaIdentifierStart(method.name.charAt(0))) {
+                int buildSdk = context.getMainProject().getBuildSdk();
+                String name = method.name;
+                assert frameworkParent != null;
+                int api = mApiDatabase.getCallVersion(frameworkParent, name, method.desc);
+                if (api > buildSdk && buildSdk != -1) {
+                    // TODO: Don't complain if it's annotated with @Override; that means
+                    // somehow the build target isn't correct.
+                    String fqcn;
+                    String owner = classNode.name;
+                    if (CONSTRUCTOR_NAME.equals(name)) {
+                        fqcn = "new " + ClassContext.getFqcn(owner); //$NON-NLS-1$
+                    } else {
+                        fqcn = ClassContext.getFqcn(owner) + '#' + name;
+                    }
+                    String message = String.format(
+                            "This method is not overriding anything with the current build " +
+                            "target, but will in API level %1$d (current target is %2$d): %3$s",
+                            api, buildSdk, fqcn);
+
+                    Location location = context.getLocation(method, classNode);
+                    context.report(OVERRIDE, method, null, location, message, null);
+                }
+            }
+
+            if (!checkCalls) {
+                continue;
+            }
+
             if (CHECK_DECLARATIONS) {
                 // Check types in parameter list and types of local variables
                 List localVariables = method.localVariables;
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 be43e5c..254cb02 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
@@ -55,7 +55,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
     private static final List<Issue> sIssues;
 
     static {
-        final int initialCapacity = 132;
+        final int initialCapacity = 133;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -64,6 +64,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(FieldGetterDetector.ISSUE);
         issues.add(SdCardDetector.ISSUE);
         issues.add(ApiDetector.UNSUPPORTED);
+        issues.add(ApiDetector.OVERRIDE);
         issues.add(InvalidPackageDetector.ISSUE);
         issues.add(DuplicateIdDetector.CROSS_LAYOUT);
         issues.add(DuplicateIdDetector.WITHIN_LAYOUT);
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ApiDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ApiDetectorTest.java
index b5e243b..aa62f58 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ApiDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ApiDetectorTest.java
@@ -664,4 +664,40 @@ public class ApiDetectorTest extends AbstractCheckTest {
                     "apicheck/ApiCallTest10.class.data=>bin/classes/test/pkg/ApiCallTest10.class"
                 ));
     }
+
+    public void testOverrideUnknownTarget() throws Exception {
+        assertEquals(
+            "No warnings.",
+
+            lintProject(
+                    "apicheck/classpath=>.classpath",
+                    "apicheck/minsdk4.xml=>AndroidManifest.xml",
+                    "apicheck/ApiCallTest11.java.txt=>src/test/pkg/ApiCallTest11.java",
+                    "apicheck/ApiCallTest11.class.data=>bin/classes/test/pkg/ApiCallTest11.class"
+                ));
+    }
+
+    public void testOverride() throws Exception {
+        assertEquals(
+            "src/test/pkg/ApiCallTest11.java:13: Error: This method is not overriding anything with the current build target, but will in API level 11 (current target is 3): test.pkg.ApiCallTest11#getActionBar [Override]\n" +
+            "    public ActionBar getActionBar() {\n" +
+            "                     ~~~~~~~~~~~~\n" +
+            "src/test/pkg/ApiCallTest11.java:17: Error: This method is not overriding anything with the current build target, but will in API level 17 (current target is 3): test.pkg.ApiCallTest11#isDestroyed [Override]\n" +
+            "    public boolean isDestroyed() {\n" +
+            "                   ~~~~~~~~~~~\n" +
+            "src/test/pkg/ApiCallTest11.java:39: Error: This method is not overriding anything with the current build target, but will in API level 11 (current target is 3): test.pkg.ApiCallTest11.MyLinear#setDividerDrawable [Override]\n" +
+            "        public void setDividerDrawable(Drawable dividerDrawable) {\n" +
+            "                    ~~~~~~~~~~~~~~~~~~\n" +
+            "3 errors, 0 warnings\n",
+
+            lintProject(
+                    "apicheck/classpath=>.classpath",
+                    "apicheck/minsdk4.xml=>AndroidManifest.xml",
+                    "project.properties1=>project.properties",
+                    "apicheck/ApiCallTest11.java.txt=>src/test/pkg/ApiCallTest11.java",
+                    "apicheck/ApiCallTest11.class.data=>bin/classes/test/pkg/ApiCallTest11.class",
+                    "apicheck/ApiCallTest11$MyLinear.class.data=>bin/classes/test/pkg/ApiCallTest11$MyLinear.class",
+                    "apicheck/ApiCallTest11$MyActivity.class.data=>bin/classes/test/pkg/ApiCallTest11$MyActivity.class"
+                ));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11$MyActivity.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11$MyActivity.class.data
new file mode 100644
index 0000000..5368daf
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11$MyActivity.class.data
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11$MyLinear.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11$MyLinear.class.data
new file mode 100644
index 0000000..5ee9e96
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11$MyLinear.class.data
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11.class.data
new file mode 100644
index 0000000..17e10d6
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11.class.data
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11.java.txt
new file mode 100644
index 0000000..8fb592c
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/ApiCallTest11.java.txt
@@ -0,0 +1,50 @@
+package test.pkg;
+
+import android.annotation.SuppressLint;
+import android.app.ActionBar;
+import android.app.Activity;
+import android.content.Context;
+import android.graphics.drawable.Drawable;
+import android.widget.LinearLayout;
+
+public class ApiCallTest11 extends Activity {
+	MyActivity mActionBarHost;
+
+    public ActionBar getActionBar() {
+        return mActionBarHost.getActionBar();
+    }
+
+    public boolean isDestroyed() {
+        return true;
+    }
+
+    @SuppressLint("Override")
+    public void finishAffinity() {
+    }
+
+    private class MyLinear extends LinearLayout {
+        private Drawable mDividerDrawable;
+
+        public MyLinear(Context context) {
+            super(context);
+        }
+
+       /**
+         * Javadoc here
+         *
+         *
+         *
+         *
+         */
+        public void setDividerDrawable(Drawable dividerDrawable) {
+            mDividerDrawable = dividerDrawable;
+        }
+    }
+
+    private class MyActivity {
+        public ActionBar getActionBar() {
+            return null;
+        }
+    }
+}
+