aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java2
-rw-r--r--lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintUtils.java2
-rw-r--r--lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java3
-rw-r--r--lint/libs/lint_checks/src/com/android/tools/lint/checks/FragmentDetector.java168
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FragmentDetectorTest.java46
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment1.class.databin0 -> 373 bytes
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment2.class.databin0 -> 460 bytes
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment3.class.databin0 -> 377 bytes
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment4.class.databin0 -> 406 bytes
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment5.class.databin0 -> 471 bytes
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment6.class.databin0 -> 487 bytes
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$NotAFragment.class.databin0 -> 465 bytes
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$ValidFragment1.class.databin0 -> 392 bytes
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest.class.databin0 -> 774 bytes
-rw-r--r--lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest.java.txt55
15 files changed, 273 insertions, 3 deletions
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
index 12dffc5..3b7d63f 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
@@ -327,4 +327,6 @@ public class LintConstants {
// Class Names
public static final String CONSTRUCTOR_NAME = "<init>"; //$NON-NLS-1$
+ public static final String FRAGMENT = "android/app/Fragment"; //$NON-NLS-1$
+ public static final String FRAGMENT_V4 = "android/support/v4/app/Fragment"; //$NON-NLS-1$
}
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintUtils.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintUtils.java
index 154ba67..ef987f2 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintUtils.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintUtils.java
@@ -16,7 +16,6 @@
package com.android.tools.lint.detector.api;
-import static com.android.tools.lint.detector.api.LintConstants.CONSTRUCTOR_NAME;
import static com.android.tools.lint.detector.api.LintConstants.DOT_XML;
import static com.android.tools.lint.detector.api.LintConstants.ID_RESOURCE_PREFIX;
import static com.android.tools.lint.detector.api.LintConstants.NEW_ID_RESOURCE_PREFIX;
@@ -33,7 +32,6 @@ import com.google.common.io.Files;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.tree.ClassNode;
import org.objectweb.asm.tree.FieldNode;
-import org.objectweb.asm.tree.MethodNode;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
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 720c0da..0645d56 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
@@ -53,7 +53,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
private static final List<Issue> sIssues;
static {
- final int initialCapacity = 91;
+ final int initialCapacity = 92;
List<Issue> issues = new ArrayList<Issue>(initialCapacity);
issues.add(AccessibilityDetector.ISSUE);
@@ -86,6 +86,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
issues.add(OnClickDetector.ISSUE);
issues.add(RegistrationDetector.ISSUE);
issues.add(HandlerDetector.ISSUE);
+ issues.add(FragmentDetector.ISSUE);
issues.add(TranslationDetector.EXTRA);
issues.add(TranslationDetector.MISSING);
issues.add(HardcodedValuesDetector.ISSUE);
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/FragmentDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/FragmentDetector.java
new file mode 100644
index 0000000..37b22ea
--- /dev/null
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/FragmentDetector.java
@@ -0,0 +1,168 @@
+/*
+ * 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.CONSTRUCTOR_NAME;
+import static com.android.tools.lint.detector.api.LintConstants.FRAGMENT;
+import static com.android.tools.lint.detector.api.LintConstants.FRAGMENT_V4;
+
+import com.android.tools.lint.client.api.LintDriver;
+import com.android.tools.lint.detector.api.Category;
+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.Detector.ClassScanner;
+import com.android.tools.lint.detector.api.Issue;
+import com.android.tools.lint.detector.api.LintUtils;
+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 org.objectweb.asm.Opcodes;
+import org.objectweb.asm.tree.ClassNode;
+import org.objectweb.asm.tree.MethodNode;
+
+import java.io.File;
+import java.util.List;
+
+/**
+ * Checks that Fragment subclasses can be instantiated via
+ * {link {@link Class#newInstance()}}: the class is public, static, and has
+ * a public null constructor.
+ * <p>
+ * This helps track down issues like
+ * http://stackoverflow.com/questions/8058809/fragment-activity-crashes-on-screen-rotate
+ * (and countless duplicates)
+ */
+public class FragmentDetector extends Detector implements ClassScanner {
+ private static final String FRAGMENT_NAME_SUFFIX = "Fragment"; //$NON-NLS-1$
+
+ /** Are fragment subclasses instantiatable? */
+ public static final Issue ISSUE = Issue.create(
+ "ValidFragment", //$NON-NLS-1$
+ "Ensures that Fragment subclasses can be instantiated",
+
+ "From the Fragment documentation:\n" +
+ "*Every* fragment must have an empty constructor, so it can be instantiated when " +
+ "restoring its activity's state. It is strongly recommended that subclasses do not " +
+ "have other constructors with parameters, since these constructors will not be " +
+ "called when the fragment is re-instantiated; instead, arguments can be supplied " +
+ "by the caller with setArguments(Bundle) and later retrieved by the Fragment " +
+ "with getArguments().",
+
+ Category.CORRECTNESS,
+ 6,
+ Severity.WARNING,
+ FragmentDetector.class,
+ Scope.CLASS_FILE_SCOPE).setMoreInfo(
+ "http://developer.android.com/reference/android/app/Fragment.html#Fragment()"); //$NON-NLS-1$
+
+
+ /** Constructs a new {@link FragmentDetector} */
+ public FragmentDetector() {
+ }
+
+ @Override
+ public Speed getSpeed() {
+ return Speed.FAST;
+ }
+
+ @Override
+ public boolean appliesTo(Context context, File file) {
+ return true;
+ }
+
+ // ---- Implements ClassScanner ----
+
+ @Override
+ public void checkClass(ClassContext context, ClassNode classNode) {
+ if ((classNode.access & Opcodes.ACC_ABSTRACT) != 0) {
+ // Ignore abstract classes since they are clearly (and by definition) not intended to
+ // be instantiated. We're looking for accidental non-static or missing constructor
+ // scenarios here.
+ return;
+ }
+
+ LintDriver driver = context.getDriver();
+
+ if (!(driver.isSubclassOf(classNode, FRAGMENT)
+ || driver.isSubclassOf(classNode, FRAGMENT_V4))) {
+ if (!context.getScope().contains(Scope.ALL_JAVA_FILES)) {
+ // Single file checking: Just check that it looks like a fragment class
+ // (since we don't have a full superclass map)
+ if (!classNode.name.endsWith(FRAGMENT_NAME_SUFFIX) ||
+ classNode.superName == null) {
+ return;
+ }
+ } else {
+ return;
+ }
+ }
+
+ if ((classNode.access & Opcodes.ACC_PUBLIC) == 0) {
+ context.report(ISSUE, context.getLocation(classNode), String.format(
+ "This fragment class should be public (%1$s)",
+ ClassContext.createSignature(classNode.name, null, null)),
+ null);
+ return;
+ }
+
+ if (classNode.name.indexOf('$') != -1 && !LintUtils.isStaticInnerClass(classNode)) {
+ context.report(ISSUE, context.getLocation(classNode), String.format(
+ "This fragment inner class should be static (%1$s)",
+ ClassContext.createSignature(classNode.name, null, null)),
+ null);
+ return;
+ }
+
+ boolean hasDefaultConstructor = false;
+ @SuppressWarnings("rawtypes") // ASM API
+ List methodList = classNode.methods;
+ for (Object m : methodList) {
+ MethodNode method = (MethodNode) m;
+ if (method.name.equals(CONSTRUCTOR_NAME)) {
+ if (method.desc.equals("()V")) { //$NON-NLS-1$
+ // The constructor must be public
+ if ((method.access & Opcodes.ACC_PUBLIC) != 0) {
+ hasDefaultConstructor = true;
+ } else {
+ context.report(ISSUE, context.getLocation(method, classNode),
+ "The default constructor must be public",
+ null);
+ // Also mark that we have a constructor so we don't complain again
+ // below since we've already emitted a more specific error related
+ // to the default constructor
+ hasDefaultConstructor = true;
+ }
+ } else if (!method.desc.contains("()")) { //$NON-NLS-1$
+ context.report(ISSUE, context.getLocation(method, classNode),
+ "Avoid non-default constructors in fragments: use a default constructor " +
+ "plus Fragment#setArguments(Bundle) instead",
+ null);
+ }
+ }
+ }
+
+ if (!hasDefaultConstructor) {
+ context.report(ISSUE, context.getLocation(classNode), String.format(
+ "This fragment should provide a default constructor (a public " +
+ "constructor with no arguments) (%1$s)",
+ ClassContext.createSignature(classNode.name, null, null)),
+ null);
+ }
+ }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FragmentDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FragmentDetectorTest.java
new file mode 100644
index 0000000..e25bbbc
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/FragmentDetectorTest.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Eclipse Public License, Version 1.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.eclipse.org/org/documents/epl-v10.php
+ *
+ * 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;
+
+@SuppressWarnings("javadoc")
+public class FragmentDetectorTest extends AbstractCheckTest {
+ @Override
+ protected Detector getDetector() {
+ return new FragmentDetector();
+ }
+
+ public void test() throws Exception {
+ assertEquals(
+ "FragmentTest.java:10: Warning: This fragment class should be public (test.pkg.FragmentTest.Fragment1)\n" +
+ "FragmentTest.java:15: Warning: This fragment inner class should be static (test.pkg.FragmentTest.Fragment2)\n" +
+ "FragmentTest.java:21: Warning: The default constructor must be public\n" +
+ "FragmentTest.java:27: Warning: Avoid non-default constructors in fragments: use a default constructor plus Fragment#setArguments(Bundle) instead\n" +
+ "FragmentTest.java:27: Warning: This fragment should provide a default constructor (a public constructor with no arguments) (test.pkg.FragmentTest.Fragment4)\n" +
+ "FragmentTest.java:36: Warning: Avoid non-default constructors in fragments: use a default constructor plus Fragment#setArguments(Bundle) instead",
+
+ lintProject(
+ "bytecode/FragmentTest$Fragment1.class.data=>bin/classes/test/pkg/FragmentTest$Fragment1.class",
+ "bytecode/FragmentTest$Fragment2.class.data=>bin/classes/test/pkg/FragmentTest$Fragment2.class",
+ "bytecode/FragmentTest$Fragment3.class.data=>bin/classes/test/pkg/FragmentTest$Fragment3.class",
+ "bytecode/FragmentTest$Fragment4.class.data=>bin/classes/test/pkg/FragmentTest$Fragment4.class",
+ "bytecode/FragmentTest$Fragment5.class.data=>bin/classes/test/pkg/FragmentTest$Fragment5.class",
+ "bytecode/FragmentTest$Fragment6.class.data=>bin/classes/test/pkg/FragmentTest$Fragment6.class",
+ "bytecode/FragmentTest$NotAFragment.class.data=>bin/classes/test/pkg/FragmentTest$NotAFragment.class",
+ "bytecode/FragmentTest.java.txt=>src/test/pkg/FragmentTest.java"));
+ }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment1.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment1.class.data
new file mode 100644
index 0000000..11a98dd
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment1.class.data
Binary files differ
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment2.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment2.class.data
new file mode 100644
index 0000000..d77579a
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment2.class.data
Binary files differ
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment3.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment3.class.data
new file mode 100644
index 0000000..b1ec17d
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment3.class.data
Binary files differ
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment4.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment4.class.data
new file mode 100644
index 0000000..f89f8ed
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment4.class.data
Binary files differ
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment5.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment5.class.data
new file mode 100644
index 0000000..4a23e03
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment5.class.data
Binary files differ
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment6.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment6.class.data
new file mode 100644
index 0000000..2e10e12
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$Fragment6.class.data
Binary files differ
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$NotAFragment.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$NotAFragment.class.data
new file mode 100644
index 0000000..8f5b827
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$NotAFragment.class.data
Binary files differ
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$ValidFragment1.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$ValidFragment1.class.data
new file mode 100644
index 0000000..a3354a7
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest$ValidFragment1.class.data
Binary files differ
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest.class.data
new file mode 100644
index 0000000..6cc8387
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest.class.data
Binary files differ
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest.java.txt
new file mode 100644
index 0000000..d27c04a
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/FragmentTest.java.txt
@@ -0,0 +1,55 @@
+package test.pkg;
+
+import android.annotation.SuppressLint;
+import android.app.Fragment;
+
+@SuppressWarnings("unused")
+public class FragmentTest {
+
+ // Should be public
+ private static class Fragment1 extends Fragment {
+
+ }
+
+ // Should be static
+ public class Fragment2 extends Fragment {
+
+ }
+
+ // Should have a public constructor
+ public static class Fragment3 extends Fragment {
+ private Fragment3() {
+ }
+ }
+
+ // Should have a public constructor with no arguments
+ public static class Fragment4 extends Fragment {
+ private Fragment4(int dummy) {
+ }
+ }
+
+ // Should *only* have the default constructor, not the
+ // multi-argument one
+ public static class Fragment5 extends Fragment {
+ public Fragment5() {
+ }
+ public Fragment5(int dummy) {
+ }
+ }
+
+ // Suppressed
+ @SuppressLint("ValidFragment")
+ public static class Fragment6 extends Fragment {
+ private Fragment6() {
+ }
+ }
+
+ public static class ValidFragment1 extends Fragment {
+ public ValidFragment1() {
+ }
+ }
+
+ // (Not a fragment)
+ private class NotAFragment {
+ }
+}