diff options
Diffstat (limited to 'lint')
| -rw-r--r-- | lint/cli/src/test/java/com/android/tools/lint/checks/RecycleDetectorTest.java | 41 | ||||
| -rw-r--r-- | lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/CommitTest.class.data | bin | 0 -> 2433 bytes | |||
| -rw-r--r-- | lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/CommitTest.java.txt | 66 | ||||
| -rw-r--r-- | lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/DialogFragment.class.data | bin | 0 -> 7113 bytes | |||
| -rw-r--r-- | lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/BuiltinIssueRegistry.java | 5 | ||||
| -rw-r--r-- | lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/CleanupDetector.java (renamed from lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/RecycleDetector.java) | 108 |
6 files changed, 191 insertions, 29 deletions
diff --git a/lint/cli/src/test/java/com/android/tools/lint/checks/RecycleDetectorTest.java b/lint/cli/src/test/java/com/android/tools/lint/checks/RecycleDetectorTest.java index ebef046..df6f451 100644 --- a/lint/cli/src/test/java/com/android/tools/lint/checks/RecycleDetectorTest.java +++ b/lint/cli/src/test/java/com/android/tools/lint/checks/RecycleDetectorTest.java @@ -22,10 +22,10 @@ import com.android.tools.lint.detector.api.Detector; public class RecycleDetectorTest extends AbstractCheckTest { @Override protected Detector getDetector() { - return new RecycleDetector(); + return new CleanupDetector(); } - public void test() throws Exception { + public void testRecycle() throws Exception { assertEquals( "src/test/pkg/RecycleTest.java:56: Warning: This TypedArray should be recycled after use with #recycle() [Recycle]\n" + " final TypedArray a = getContext().obtainStyledAttributes(attrs,\n" + @@ -73,4 +73,41 @@ public class RecycleDetectorTest extends AbstractCheckTest { "bytecode/RecycleTest.class.data=>bin/classes/test/pkg/RecycleTest.class" )); } + + public void testCommit() throws Exception { + assertEquals("" + + "src/test/pkg/CommitTest.java:24: Warning: This FragmentTransaction should be recycled after use with #recycle() [CommitTransaction]\n" + + " getFragmentManager().beginTransaction(); // Missing commit\n" + + " ~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/CommitTest.java:29: Warning: This FragmentTransaction should be recycled after use with #recycle() [CommitTransaction]\n" + + " FragmentTransaction transaction2 = getFragmentManager().beginTransaction(); // Missing commit\n" + + " ~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/CommitTest.java:38: Warning: This FragmentTransaction should be recycled after use with #recycle() [CommitTransaction]\n" + + " getFragmentManager().beginTransaction(); // Missing commit\n" + + " ~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/CommitTest.java:64: Warning: This FragmentTransaction should be recycled after use with #recycle() [Recycle]\n" + + " getSupportFragmentManager().beginTransaction();\n" + + " ~~~~~~~~~~~~~~~~\n" + + "0 errors, 4 warnings\n", + + lintProject( + "apicheck/classpath=>.classpath", + "apicheck/minsdk4.xml=>AndroidManifest.xml", + "project.properties1=>project.properties", + "bytecode/CommitTest.java.txt=>src/test/pkg/CommitTest.java", + "bytecode/CommitTest.class.data=>bin/classes/test/pkg/CommitTest.class" + )); + } + + public void testCommit2() throws Exception { + assertEquals("" + + "No warnings.", + + lintProject( + "apicheck/classpath=>.classpath", + "apicheck/minsdk4.xml=>AndroidManifest.xml", + "project.properties1=>project.properties", + "bytecode/DialogFragment.class.data=>bin/classes/test/pkg/DialogFragment.class" + )); + } } diff --git a/lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/CommitTest.class.data b/lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/CommitTest.class.data Binary files differnew file mode 100644 index 0000000..76f35c0 --- /dev/null +++ b/lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/CommitTest.class.data diff --git a/lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/CommitTest.java.txt b/lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/CommitTest.java.txt new file mode 100644 index 0000000..49395d7 --- /dev/null +++ b/lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/CommitTest.java.txt @@ -0,0 +1,66 @@ +package test.pkg; + +import android.app.Activity; +import android.app.FragmentManager; +import android.app.FragmentTransaction; + +@SuppressWarnings("unused") +public class CommitTest extends Activity { + public void ok1() { + getFragmentManager().beginTransaction().commit(); + } + + public void ok2() { + FragmentTransaction transaction = getFragmentManager().beginTransaction(); + transaction.commit(); + } + + public void ok3() { + FragmentTransaction transaction = getFragmentManager().beginTransaction(); + transaction.commitAllowingStateLoss(); + } + + public void error1() { + getFragmentManager().beginTransaction(); // Missing commit + } + + public void error() { + FragmentTransaction transaction1 = getFragmentManager().beginTransaction(); + FragmentTransaction transaction2 = getFragmentManager().beginTransaction(); // Missing commit + transaction1.commit(); + } + + public void error3_public() { + error3(); + } + + private void error3() { + getFragmentManager().beginTransaction(); // Missing commit + } + + public void ok4(FragmentManager manager, String tag) { + FragmentTransaction ft = manager.beginTransaction(); + ft.add(null, tag); + ft.commit(); + } + + // Support library + + private android.support.v4.app.FragmentManager getSupportFragmentManager() { + return null; + } + + public void ok5() { + getSupportFragmentManager().beginTransaction().commit(); + } + + public void ok6(android.support.v4.app.FragmentManager manager, String tag) { + android.support.v4.app.FragmentTransaction ft = manager.beginTransaction(); + ft.add(null, tag); + ft.commit(); + } + + public void error4() { + getSupportFragmentManager().beginTransaction(); + } +} diff --git a/lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/DialogFragment.class.data b/lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/DialogFragment.class.data Binary files differnew file mode 100644 index 0000000..f57a263 --- /dev/null +++ b/lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/DialogFragment.class.data diff --git a/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/BuiltinIssueRegistry.java b/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/BuiltinIssueRegistry.java index b1ce430..97a9624 100644 --- a/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/BuiltinIssueRegistry.java +++ b/lint/libs/lint_checks/src/main/java/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 = 136; + final int initialCapacity = 137; List<Issue> issues = new ArrayList<Issue>(initialCapacity); issues.add(AccessibilityDetector.ISSUE); @@ -184,7 +184,8 @@ public class BuiltinIssueRegistry extends IssueRegistry { issues.add(JavaPerformanceDetector.USE_VALUEOF); issues.add(JavaPerformanceDetector.USE_SPARSEARRAY); issues.add(WakelockDetector.ISSUE); - issues.add(RecycleDetector.ISSUE); + issues.add(CleanupDetector.RECYCLE_RESOURCE); + issues.add(CleanupDetector.COMMIT_FRAGMENT); issues.add(SetJavaScriptEnabledDetector.ISSUE); issues.add(ToastDetector.ISSUE); issues.add(SharedPrefsDetector.ISSUE); diff --git a/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/RecycleDetector.java b/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/CleanupDetector.java index fac98b0..11365e2 100644 --- a/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/RecycleDetector.java +++ b/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/CleanupDetector.java @@ -45,11 +45,12 @@ import java.util.Arrays; import java.util.List; /** - * Checks for missing {@code recycle} calls on resources that encourage it + * Checks for missing {@code recycle} calls on resources that encourage it, and + * for missing {@code commit} calls on FragmentTransactions, etc. */ -public class RecycleDetector extends Detector implements ClassScanner { +public class CleanupDetector extends Detector implements ClassScanner { /** Problems with missing recycle calls */ - public static final Issue ISSUE = Issue.create( + public static final Issue RECYCLE_RESOURCE = Issue.create( "Recycle", //$NON-NLS-1$ "Looks for missing recycle() calls on resources", @@ -60,9 +61,22 @@ public class RecycleDetector extends Detector implements ClassScanner { Category.PERFORMANCE, 7, Severity.WARNING, - RecycleDetector.class, + CleanupDetector.class, Scope.CLASS_FILE_SCOPE); + /** Problems with missing commit calls. */ + public static final Issue COMMIT_FRAGMENT = Issue.create( + "CommitTransaction", //$NON-NLS-1$ + "Looks for missing commit() calls on FragmentTransactions", + + "After creating a `FragmentTransaction`, you typically need to commit it as well", + + Category.CORRECTNESS, + 7, + Severity.WARNING, + CleanupDetector.class, + Scope.CLASS_FILE_SCOPE); + // Target method names private static final String RECYCLE = "recycle"; //$NON-NLS-1$ private static final String OBTAIN = "obtain"; //$NON-NLS-1$ @@ -71,6 +85,9 @@ public class RecycleDetector extends Detector implements ClassScanner { private static final String OBTAIN_ATTRIBUTES = "obtainAttributes"; //$NON-NLS-1$ private static final String OBTAIN_TYPED_ARRAY = "obtainTypedArray"; //$NON-NLS-1$ private static final String OBTAIN_STYLED_ATTRIBUTES = "obtainStyledAttributes"; //$NON-NLS-1$ + private static final String BEGIN_TRANSACTION = "beginTransaction"; //$NON-NLS-1$ + private static final String COMMIT = "commit"; //$NON-NLS-1$ + private static final String COMMIT_ALLOWING_LOSS = "commitAllowingStateLoss"; //$NON-NLS-1$ // Target owners private static final String VELOCITY_TRACKER_CLS = "android/view/VelocityTracker";//$NON-NLS-1$ @@ -81,6 +98,10 @@ public class RecycleDetector extends Detector implements ClassScanner { private static final String HANDLER_CLS = "android/os/Handler"; //$NON-NLS-1$ private static final String RESOURCES_CLS = "android/content/res/Resources"; //$NON-NLS-1$ private static final String PARCEL_CLS = "android/os/Parcel"; //$NON-NLS-1$ + private static final String FRAGMENT_MANAGER_CLS = "android/app/FragmentManager"; //$NON-NLS-1$ + private static final String FRAGMENT_MANAGER_V4_CLS = "android/support/v4/app/FragmentManager"; //$NON-NLS-1$ + private static final String FRAGMENT_TRANSACTION_CLS = "android/app/FragmentTransaction"; //$NON-NLS-1$ + private static final String FRAGMENT_TRANSACTION_V4_CLS = "android/support/v4/app/FragmentTransaction"; //$NON-NLS-1$ // Target description signatures private static final String TYPED_ARRAY_SIG = "Landroid/content/res/TypedArray;"; //$NON-NLS-1$ @@ -96,9 +117,11 @@ public class RecycleDetector extends Detector implements ClassScanner { private boolean mRecyclesMotionEvent; private boolean mObtainsParcel; private boolean mRecyclesParcel; + private boolean mObtainsTransaction; + private boolean mCommitsTransaction; - /** Constructs a new {@link RecycleDetector} */ - public RecycleDetector() { + /** Constructs a new {@link CleanupDetector} */ + public CleanupDetector() { } @Override @@ -109,7 +132,8 @@ public class RecycleDetector extends Detector implements ClassScanner { || mObtainsTracker && !mRecyclesTracker || mObtainsMessage && !mRecyclesMessage || mObtainsParcel && !mRecyclesParcel - || mObtainsMotionEvent && !mRecyclesMotionEvent) { + || mObtainsMotionEvent && !mRecyclesMotionEvent + || mObtainsTransaction && !mCommitsTransaction) { context.getDriver().requestRepeat(this, Scope.CLASS_FILE_SCOPE); } } @@ -127,7 +151,10 @@ public class RecycleDetector extends Detector implements ClassScanner { OBTAIN_ATTRIBUTES, OBTAIN_TYPED_ARRAY, OBTAIN_MESSAGE, - OBTAIN_NO_HISTORY + OBTAIN_NO_HISTORY, + BEGIN_TRANSACTION, + COMMIT, + COMMIT_ALLOWING_LOSS ); } @@ -153,11 +180,17 @@ public class RecycleDetector extends Detector implements ClassScanner { } else if (owner.equals(PARCEL_CLS)) { mRecyclesParcel = true; } + } else if ((COMMIT.equals(name) || COMMIT_ALLOWING_LOSS.equals(name)) + && desc.equals("()I")) { //$NON-NLS-1$ + if (owner.equals(FRAGMENT_TRANSACTION_CLS) + || owner.equals(FRAGMENT_TRANSACTION_V4_CLS)) { + mCommitsTransaction = true; + } } else if (owner.equals(MOTION_EVENT_CLS)) { if (OBTAIN.equals(name) || OBTAIN_NO_HISTORY.equals(name)) { mObtainsMotionEvent = true; if (phase == 2 && !mRecyclesMotionEvent) { - context.report(ISSUE, method, call, context.getLocation(call), + context.report(RECYCLE_RESOURCE, method, call, context.getLocation(call), getErrorMessage(MOTION_EVENT_CLS), null); } else if (phase == 1 @@ -170,7 +203,7 @@ public class RecycleDetector extends Detector implements ClassScanner { if (owner.equals(HANDLER_CLS) && desc.endsWith(MESSAGE_SIG)) { mObtainsMessage = true; if (phase == 2 && !mRecyclesMessage) { - context.report(ISSUE, method, call, context.getLocation(call), + context.report(RECYCLE_RESOURCE, method, call, context.getLocation(call), getErrorMessage(MESSAGE_CLS), null); } } @@ -178,7 +211,7 @@ public class RecycleDetector extends Detector implements ClassScanner { if (owner.equals(VELOCITY_TRACKER_CLS)) { mObtainsTracker = true; if (phase == 2 && !mRecyclesTracker) { - context.report(ISSUE, method, call, context.getLocation(call), + context.report(RECYCLE_RESOURCE, method, call, context.getLocation(call), getErrorMessage(VELOCITY_TRACKER_CLS), null); } @@ -186,14 +219,14 @@ public class RecycleDetector extends Detector implements ClassScanner { // TODO: Handle Message constructor? mObtainsMessage = true; if (phase == 2 && !mRecyclesMessage) { - context.report(ISSUE, method, call, context.getLocation(call), + context.report(RECYCLE_RESOURCE, method, call, context.getLocation(call), getErrorMessage(MESSAGE_CLS), null); } } else if (owner.equals(PARCEL_CLS)) { mObtainsParcel = true; if (phase == 2 && !mRecyclesParcel) { - context.report(ISSUE, method, call, context.getLocation(call), + context.report(RECYCLE_RESOURCE, method, call, context.getLocation(call), getErrorMessage(PARCEL_CLS), null); } else if (phase == 1 @@ -209,7 +242,7 @@ public class RecycleDetector extends Detector implements ClassScanner { && desc.endsWith(TYPED_ARRAY_SIG)) { mObtainsTypedArray = true; if (phase == 2 && !mRecyclesTypedArray) { - context.report(ISSUE, method, call, context.getLocation(call), + context.report(RECYCLE_RESOURCE, method, call, context.getLocation(call), getErrorMessage(TYPED_ARRAY_CLS), null); } else if (phase == 1 @@ -218,11 +251,27 @@ public class RecycleDetector extends Detector implements ClassScanner { mRecyclesTypedArray = true; } } + } else if (BEGIN_TRANSACTION.equals(name) + && (owner.equals(FRAGMENT_MANAGER_CLS) || owner.equals(FRAGMENT_MANAGER_V4_CLS))) { + mObtainsTransaction = true; + if (phase == 2 && !mCommitsTransaction) { + context.report(COMMIT_FRAGMENT, method, call, context.getLocation(call), + getErrorMessage(FRAGMENT_MANAGER_CLS), null); + } else if (phase == 1 + && checkMethodFlow(context, classNode, method, call, + owner.equals(FRAGMENT_MANAGER_CLS) + ? FRAGMENT_TRANSACTION_CLS : FRAGMENT_TRANSACTION_V4_CLS)) { + // Already reported error above; don't do global check + mCommitsTransaction = true; + } } } /** Computes an error message for a missing recycle of the given type */ private static String getErrorMessage(String owner) { + if (FRAGMENT_MANAGER_CLS.equals(owner)) { + return "This transaction should be completed with a commit() call"; + } String className = owner.substring(owner.lastIndexOf('/') + 1); return String.format("This %1$s should be recycled after use with #recycle()", className); @@ -239,15 +288,17 @@ public class RecycleDetector extends Detector implements ClassScanner { */ private static boolean checkMethodFlow(ClassContext context, ClassNode classNode, MethodNode method, MethodInsnNode call, String recycleOwner) { - RecycleTracker interpreter = new RecycleTracker(context, method, call, recycleOwner); + CleanupTracker interpreter = new CleanupTracker(context, method, call, recycleOwner); ResourceAnalyzer analyzer = new ResourceAnalyzer(interpreter); interpreter.setAnalyzer(analyzer); try { analyzer.analyze(classNode.name, method); - if (!interpreter.isRecycled() && !interpreter.isEscaped()) { + if (!interpreter.isCleanedUp() && !interpreter.isEscaped()) { Location location = context.getLocation(call); String message = getErrorMessage(recycleOwner); - context.report(ISSUE, method, call, location, message, null); + Issue issue = call.owner.equals(FRAGMENT_MANAGER_CLS) + ? COMMIT_FRAGMENT : RECYCLE_RESOURCE; + context.report(issue, method, call, location, message, null); return true; } } catch (AnalyzerException e) { @@ -263,20 +314,21 @@ public class RecycleDetector extends Detector implements ClassScanner { * value flows out of the method (to a field, or a method call), it will * also consider the resource recycled. */ - private static class RecycleTracker extends Interpreter { - private static final Value INSTANCE = BasicValue.INT_VALUE; // Only identity matters, not value + private static class CleanupTracker extends Interpreter { + // Only identity matters, not value + private static final Value INSTANCE = BasicValue.INT_VALUE; private static final Value RECYCLED = BasicValue.FLOAT_VALUE; private static final Value UNKNOWN = BasicValue.UNINITIALIZED_VALUE; private final ClassContext mContext; private final MethodNode mMethod; private final MethodInsnNode mObtainNode; - private boolean mIsRecycled; + private boolean mIsCleanedUp; private boolean mEscapes; private final String mRecycleOwner; private ResourceAnalyzer mAnalyzer; - public RecycleTracker( + public CleanupTracker( @NonNull ClassContext context, @NonNull MethodNode method, @NonNull MethodInsnNode obtainNode, @@ -301,8 +353,8 @@ public class RecycleDetector extends Detector implements ClassScanner { * * @return true if the resource was recycled */ - public boolean isRecycled() { - return mIsRecycled; + public boolean isCleanedUp() { + return mIsCleanedUp; } /** @@ -358,7 +410,7 @@ public class RecycleDetector extends Detector implements ClassScanner { if (node.getOpcode() == Opcodes.INVOKEVIRTUAL) { if (call.name.equals(RECYCLE) && call.owner.equals(mRecycleOwner)) { if (values != null && values.size() == 1 && values.get(0) == INSTANCE) { - mIsRecycled = true; + mIsCleanedUp = true; Frame frame = mAnalyzer.getCurrentFrame(); if (frame != null) { int localSize = frame.getLocals(); @@ -376,6 +428,12 @@ public class RecycleDetector extends Detector implements ClassScanner { } return RECYCLED; } + } else if ((call.name.equals(COMMIT) || call.name.equals(COMMIT_ALLOWING_LOSS)) + && call.owner.equals(mRecycleOwner)) { + if (values != null && values.size() == 1 && values.get(0) == INSTANCE) { + mIsCleanedUp = true; + return INSTANCE; + } } } } @@ -405,7 +463,7 @@ public class RecycleDetector extends Detector implements ClassScanner { Location location = mContext.getLocation(call); String message = String.format("This %1$s has already been recycled", mRecycleOwner.substring(mRecycleOwner.lastIndexOf('/') + 1)); - mContext.report(ISSUE, mMethod, call, location, message, null); + mContext.report(RECYCLE_RESOURCE, mMethod, call, location, message, null); } } } |