Merge "Fix false positives in field getter lint check"

4 files changed, 73 insertions, 0 deletions

diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/FieldGetterDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/FieldGetterDetector.java
index b73b9ae..a1fd9fa 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/FieldGetterDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/FieldGetterDetector.java
@@ -22,6 +22,7 @@ import com.android.tools.lint.detector.api.ClassContext;
 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.LintUtils;
 import com.android.tools.lint.detector.api.Location;
 import com.android.tools.lint.detector.api.Scope;
 import com.android.tools.lint.detector.api.Severity;
@@ -35,6 +36,7 @@ import org.objectweb.asm.tree.FieldInsnNode;
 import org.objectweb.asm.tree.InsnList;
 import org.objectweb.asm.tree.MethodInsnNode;
 import org.objectweb.asm.tree.MethodNode;
+import org.objectweb.asm.tree.VarInsnNode;
 
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -95,10 +97,28 @@ public class FieldGetterDetector extends Detector implements Detector.ClassScann
             return;
         }
 
+        if ((method.access & Opcodes.ACC_STATIC) != 0) {
+            // Not an instance method
+            return;
+        }
+
+        if (instruction.getOpcode() != Opcodes.INVOKEVIRTUAL) {
+            return;
+        }
+
         MethodInsnNode node = (MethodInsnNode) instruction;
         String name = node.name;
         String owner = node.owner;
 
+        AbstractInsnNode prev = LintUtils.getPrevInstruction(instruction);
+        if (prev == null || prev.getOpcode() != Opcodes.ALOAD) {
+            return;
+        }
+        VarInsnNode prevVar = (VarInsnNode) prev;
+        if (prevVar.var != 0) { // Not on "this", variable 0 in instance methods?
+            return;
+        }
+
         if (((name.startsWith("get") && name.length() > 3     //$NON-NLS-1$
                 && Character.isUpperCase(name.charAt(3)))
             || (name.startsWith("is") && name.length() > 2    //$NON-NLS-1$
@@ -153,6 +173,8 @@ public class FieldGetterDetector extends Detector implements Detector.ClassScann
                 }
             }
         }
+
+        mPendingCalls = null;
     }
 
     // Holder class for getters to be checked
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FieldGetterDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FieldGetterDetectorTest.java
index 81c6f20..aa7607f 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FieldGetterDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FieldGetterDetectorTest.java
@@ -117,4 +117,19 @@ public class FieldGetterDetectorTest extends AbstractCheckTest {
                     "bytecode/GetterTest.jar.data=>bin/test/pkg/bogus.class"
                     ));
     }
+
+    public void testCornerCases() throws Exception {
+        assertEquals(
+            "src/test/pkg/TestFieldGetter.java:21: Warning: Calling getter method getPath() on self is slower than field access (path) [FieldGetter]\n" +
+            "        getPath(); // Should be flagged\n" +
+            "        ~~~~~~~\n" +
+            "0 errors, 1 warnings\n",
+
+            lintProject(
+                "bytecode/classpath-jar=>.classpath",
+                "apicheck/minsdk1.xml=>AndroidManifest.xml",
+                "bytecode/TestFieldGetter.java.txt=>src/test/pkg/TestFieldGetter.java",
+                "bytecode/TestFieldGetter.class.data=>bin/classes/test/pkg/TestFieldGetter.class"
+                ));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/TestFieldGetter.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/TestFieldGetter.class.data
new file mode 100644
index 0000000..d922751
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/TestFieldGetter.class.data
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/TestFieldGetter.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/TestFieldGetter.java.txt
new file mode 100644
index 0000000..00da161
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/TestFieldGetter.java.txt
@@ -0,0 +1,36 @@
+package test.pkg;
+
+import java.io.File;
+import java.util.List;
+
+import android.content.Context;
+
+public class TestFieldGetter {
+    private int path;
+    private int foo;
+
+    public int getPath() {
+        return path;
+    }
+
+    public int getFoo() {
+        return foo;
+    }
+
+    public void test(TestFieldGetter other) {
+        getPath(); // Should be flagged
+        other.getPath(); // Ignore
+        File file = new File("/dummy");
+        file.getPath(); // Ignore
+    }
+
+    public static void test2(TestFieldGetter other) {
+        other.getPath(); // Ignore
+    }
+
+    public class Inner extends TestFieldGetter {
+        public void test() {
+            getFoo(); // Ignore
+        }
+    }
+}
\ No newline at end of file