diff options
| -rw-r--r-- | lint/cli/src/test/java/com/android/tools/lint/checks/CleanupDetectorTest.java (renamed from lint/cli/src/test/java/com/android/tools/lint/checks/RecycleDetectorTest.java) | 53 | ||||
| -rw-r--r-- | lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/CommitTest.class.data | bin | 2433 -> 3612 bytes | |||
| -rw-r--r-- | lint/cli/src/test/java/com/android/tools/lint/checks/data/bytecode/CommitTest.java.txt | 25 | ||||
| -rw-r--r-- | lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/CleanupDetector.java | 52 |
4 files changed, 81 insertions, 49 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/CleanupDetectorTest.java index df6f451..01d462a 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/CleanupDetectorTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2012 The Android Open Source Project + * Copyright (C) 2013 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. @@ -19,7 +19,7 @@ package com.android.tools.lint.checks; import com.android.tools.lint.detector.api.Detector; @SuppressWarnings("javadoc") -public class RecycleDetectorTest extends AbstractCheckTest { +public class CleanupDetectorTest extends AbstractCheckTest { @Override protected Detector getDetector() { return new CleanupDetector(); @@ -36,12 +36,6 @@ public class RecycleDetectorTest extends AbstractCheckTest { "src/test/pkg/RecycleTest.java:79: Warning: This VelocityTracker should be recycled after use with #recycle() [Recycle]\n" + " VelocityTracker tracker = VelocityTracker.obtain();\n" + " ~~~~~~\n" + - "src/test/pkg/RecycleTest.java:85: Warning: This Message should be recycled after use with #recycle() [Recycle]\n" + - " Message message1 = getHandler().obtainMessage();\n" + - " ~~~~~~~~~~~~~\n" + - "src/test/pkg/RecycleTest.java:86: Warning: This Message should be recycled after use with #recycle() [Recycle]\n" + - " Message message2 = Message.obtain();\n" + - " ~~~~~~\n" + "src/test/pkg/RecycleTest.java:92: Warning: This MotionEvent should be recycled after use with #recycle() [Recycle]\n" + " MotionEvent event1 = MotionEvent.obtain(null);\n" + " ~~~~~~\n" + @@ -63,7 +57,7 @@ public class RecycleDetectorTest extends AbstractCheckTest { "src/test/pkg/RecycleTest.java:129: Warning: This Parcel should be recycled after use with #recycle() [Recycle]\n" + " Parcel myparcel = Parcel.obtain();\n" + " ~~~~~~\n" + - "0 errors, 12 warnings\n", + "0 errors, 10 warnings\n", lintProject( "apicheck/classpath=>.classpath", @@ -75,20 +69,20 @@ public class RecycleDetectorTest extends AbstractCheckTest { } 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", + assertEquals("" + + "src/test/pkg/CommitTest.java:25: Warning: This transaction should be completed with a commit() call [CommitTransaction]\n" + + " getFragmentManager().beginTransaction(); // Missing commit\n" + + " ~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/CommitTest.java:30: Warning: This transaction should be completed with a commit() call [CommitTransaction]\n" + + " FragmentTransaction transaction2 = getFragmentManager().beginTransaction(); // Missing commit\n" + + " ~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/CommitTest.java:39: Warning: This transaction should be completed with a commit() call [CommitTransaction]\n" + + " getFragmentManager().beginTransaction(); // Missing commit\n" + + " ~~~~~~~~~~~~~~~~\n" + + "src/test/pkg/CommitTest.java:65: Warning: This transaction should be completed with a commit() call [Recycle]\n" + + " getSupportFragmentManager().beginTransaction();\n" + + " ~~~~~~~~~~~~~~~~\n" + + "0 errors, 4 warnings\n", lintProject( "apicheck/classpath=>.classpath", @@ -110,4 +104,17 @@ public class RecycleDetectorTest extends AbstractCheckTest { "bytecode/DialogFragment.class.data=>bin/classes/test/pkg/DialogFragment.class" )); } + + public void testHasReturnType() throws Exception { + assertTrue(CleanupDetector.hasReturnType("android/app/FragmentTransaction", + "(Landroid/app/Fragment;)Landroid/app/FragmentTransaction;")); + assertTrue(CleanupDetector.hasReturnType("android/app/FragmentTransaction", + "()Landroid/app/FragmentTransaction;")); + assertFalse(CleanupDetector.hasReturnType("android/app/FragmentTransaction", + "()Landroid/app/FragmentTransactions;")); + assertFalse(CleanupDetector.hasReturnType("android/app/FragmentTransaction", + "()Landroid/app/FragmentTransactions")); + assertFalse(CleanupDetector.hasReturnType("android/app/FragmentTransaction", + "()android/app/FragmentTransaction;")); + } } 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 differindex 76f35c0..842fb21 100644 --- 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 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 index 49395d7..24edaf3 100644 --- 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 @@ -1,6 +1,7 @@ package test.pkg; import android.app.Activity; +import android.app.Fragment; import android.app.FragmentManager; import android.app.FragmentTransaction; @@ -63,4 +64,28 @@ public class CommitTest extends Activity { public void error4() { getSupportFragmentManager().beginTransaction(); } + + android.support.v4.app.Fragment mFragment1 = null; + Fragment mFragment2 = null; + + public void ok7() { + getSupportFragmentManager().beginTransaction().add(android.R.id.content, mFragment1).commit(); + } + + public void ok8() { + getFragmentManager().beginTransaction().add(android.R.id.content, mFragment2).commit(); + } + + public void ok10() { + // Test chaining + FragmentManager fragmentManager = getFragmentManager(); + fragmentManager.beginTransaction().addToBackStack("test").attach(mFragment2).detach(mFragment2) + .disallowAddToBackStack().hide(mFragment2).setBreadCrumbShortTitle("test") + .show(mFragment2).setCustomAnimations(0, 0).commit(); + } + + public void ok9() { + FragmentManager fragmentManager = getFragmentManager(); + fragmentManager.beginTransaction().commit(); + } } diff --git a/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/CleanupDetector.java b/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/CleanupDetector.java index 11365e2..1541d23 100644 --- a/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/CleanupDetector.java +++ b/lint/libs/lint_checks/src/main/java/com/android/tools/lint/checks/CleanupDetector.java @@ -18,6 +18,7 @@ package com.android.tools.lint.checks; import com.android.annotations.NonNull; import com.android.annotations.Nullable; +import com.android.annotations.VisibleForTesting; import com.android.tools.lint.detector.api.Category; import com.android.tools.lint.detector.api.ClassContext; import com.android.tools.lint.detector.api.Context; @@ -81,7 +82,6 @@ public class CleanupDetector extends Detector implements ClassScanner { private static final String RECYCLE = "recycle"; //$NON-NLS-1$ private static final String OBTAIN = "obtain"; //$NON-NLS-1$ private static final String OBTAIN_NO_HISTORY = "obtainNoHistory"; //$NON-NLS-1$ - private static final String OBTAIN_MESSAGE = "obtainMessage"; //$NON-NLS-1$ 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$ @@ -94,7 +94,6 @@ public class CleanupDetector extends Detector implements ClassScanner { private static final String TYPED_ARRAY_CLS = "android/content/res/TypedArray"; //$NON-NLS-1$ private static final String CONTEXT_CLS = "android/content/Context"; //$NON-NLS-1$ private static final String MOTION_EVENT_CLS = "android/view/MotionEvent"; //$NON-NLS-1$ - private static final String MESSAGE_CLS = "android/os/Message"; //$NON-NLS-1$ 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$ @@ -105,14 +104,11 @@ public class CleanupDetector extends Detector implements ClassScanner { // Target description signatures private static final String TYPED_ARRAY_SIG = "Landroid/content/res/TypedArray;"; //$NON-NLS-1$ - private static final String MESSAGE_SIG = "Landroid/os/Message;"; //$NON-NLS-1$ private boolean mObtainsTypedArray; private boolean mRecyclesTypedArray; private boolean mObtainsTracker; private boolean mRecyclesTracker; - private boolean mObtainsMessage; - private boolean mRecyclesMessage; private boolean mObtainsMotionEvent; private boolean mRecyclesMotionEvent; private boolean mObtainsParcel; @@ -130,7 +126,6 @@ public class CleanupDetector extends Detector implements ClassScanner { if (phase == 1) { if (mObtainsTypedArray && !mRecyclesTypedArray || mObtainsTracker && !mRecyclesTracker - || mObtainsMessage && !mRecyclesMessage || mObtainsParcel && !mRecyclesParcel || mObtainsMotionEvent && !mRecyclesMotionEvent || mObtainsTransaction && !mCommitsTransaction) { @@ -150,7 +145,6 @@ public class CleanupDetector extends Detector implements ClassScanner { OBTAIN, OBTAIN_ATTRIBUTES, OBTAIN_TYPED_ARRAY, - OBTAIN_MESSAGE, OBTAIN_NO_HISTORY, BEGIN_TRANSACTION, COMMIT, @@ -173,8 +167,6 @@ public class CleanupDetector extends Detector implements ClassScanner { mRecyclesTypedArray = true; } else if (owner.equals(VELOCITY_TRACKER_CLS)) { mRecyclesTracker = true; - } else if (owner.equals(MESSAGE_CLS)) { - mRecyclesMessage = true; } else if (owner.equals(MOTION_EVENT_CLS)) { mRecyclesMotionEvent = true; } else if (owner.equals(PARCEL_CLS)) { @@ -199,14 +191,6 @@ public class CleanupDetector extends Detector implements ClassScanner { mRecyclesMotionEvent = true; } } - } else if (OBTAIN_MESSAGE.equals(name)) { - if (owner.equals(HANDLER_CLS) && desc.endsWith(MESSAGE_SIG)) { - mObtainsMessage = true; - if (phase == 2 && !mRecyclesMessage) { - context.report(RECYCLE_RESOURCE, method, call, context.getLocation(call), - getErrorMessage(MESSAGE_CLS), null); - } - } } else if (OBTAIN.equals(name)) { if (owner.equals(VELOCITY_TRACKER_CLS)) { mObtainsTracker = true; @@ -215,14 +199,6 @@ public class CleanupDetector extends Detector implements ClassScanner { getErrorMessage(VELOCITY_TRACKER_CLS), null); } - } else if (owner.equals(MESSAGE_CLS) && desc.endsWith(MESSAGE_SIG)) { - // TODO: Handle Message constructor? - mObtainsMessage = true; - if (phase == 2 && !mRecyclesMessage) { - 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) { @@ -269,7 +245,7 @@ public class CleanupDetector extends Detector implements ClassScanner { /** Computes an error message for a missing recycle of the given type */ private static String getErrorMessage(String owner) { - if (FRAGMENT_MANAGER_CLS.equals(owner)) { + if (FRAGMENT_TRANSACTION_CLS.equals(owner) || FRAGMENT_TRANSACTION_V4_CLS.equals(owner)) { return "This transaction should be completed with a commit() call"; } String className = owner.substring(owner.lastIndexOf('/') + 1); @@ -308,6 +284,23 @@ public class CleanupDetector extends Detector implements ClassScanner { return false; } + @VisibleForTesting + static boolean hasReturnType(String owner, String desc) { + int descLen = desc.length(); + int ownerLen = owner.length(); + if (descLen < ownerLen + 3) { + return false; + } + if (desc.charAt(descLen - 1) != ';') { + return false; + } + int typeBegin = descLen - 2 - ownerLen; + if (desc.charAt(typeBegin - 1) != ')' || desc.charAt(typeBegin) != 'L') { + return false; + } + return desc.regionMatches(typeBegin + 1, owner, 0, ownerLen); + } + /** * ASM interpreter which tracks the instances of the allocated resource, and * checks whether it is eventually passed to a {@code recycle()} call. If the @@ -434,6 +427,13 @@ public class CleanupDetector extends Detector implements ClassScanner { mIsCleanedUp = true; return INSTANCE; } + } else if (call.owner.equals(mRecycleOwner) + && hasReturnType(mRecycleOwner, call.desc)) { + // Called method which returns self. This helps handle cases where you call + // createTransaction().method1().method2().method3().commit() -- if + // method1, 2 and 3 all return "this" then the commit call is really + // called on the createTransaction instance + return INSTANCE; } } } |