diff options
author | Tor Norbye <tnorbye@google.com> | 2012-10-29 08:58:06 -0700 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2012-10-29 08:58:07 -0700 |
commit | eb5b2f119df0e48a8d1e5b4bd8b1d027c278caef (patch) | |
tree | f88df58843f9ac03f3006bb9926486acbd42d53e | |
parent | afe8ecbca7741ef3ccd647c1e0280071ccea024f (diff) | |
parent | c42e3d6ef8092e40045da1f324afdf9685be377c (diff) | |
download | sdk-eb5b2f119df0e48a8d1e5b4bd8b1d027c278caef.zip sdk-eb5b2f119df0e48a8d1e5b4bd8b1d027c278caef.tar.gz sdk-eb5b2f119df0e48a8d1e5b4bd8b1d027c278caef.tar.bz2 |
Merge "Add lint detector for suspicious 0dp sizes"
4 files changed, 223 insertions, 1 deletions
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 22a6c9d..147100d 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 = 121; + final int initialCapacity = 122; List<Issue> issues = new ArrayList<Issue>(initialCapacity); issues.add(AccessibilityDetector.ISSUE); @@ -73,6 +73,7 @@ public class BuiltinIssueRegistry extends IssueRegistry { issues.add(InefficientWeightDetector.INEFFICIENT_WEIGHT); issues.add(InefficientWeightDetector.NESTED_WEIGHTS); issues.add(InefficientWeightDetector.BASELINE_WEIGHTS); + issues.add(InefficientWeightDetector.WRONG_0DP); issues.add(ScrollViewChildDetector.ISSUE); issues.add(DeprecationDetector.ISSUE); issues.add(ObsoleteLayoutParamsDetector.ISSUE); diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/InefficientWeightDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/InefficientWeightDetector.java index 98d1301..9c92c4b 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/InefficientWeightDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/InefficientWeightDetector.java @@ -24,6 +24,8 @@ import static com.android.SdkConstants.ATTR_LAYOUT_WIDTH; import static com.android.SdkConstants.ATTR_ORIENTATION; import static com.android.SdkConstants.LINEAR_LAYOUT; import static com.android.SdkConstants.VALUE_VERTICAL; +import static com.android.SdkConstants.VIEW; +import static com.android.SdkConstants.VIEW_TAG; import com.android.annotations.NonNull; import com.android.tools.lint.detector.api.Category; @@ -90,6 +92,25 @@ public class InefficientWeightDetector extends LayoutDetector { InefficientWeightDetector.class, Scope.RESOURCE_FILE_SCOPE); + /** Using 0dp on the wrong dimension */ + public static final Issue WRONG_0DP = Issue.create( + "Suspicious0dp", //$NON-NLS-1$ + "Looks for 0dp as the width in a vertical LinearLayout or as the height in a " + + "horizontal", + + "Using 0dp as the width in a horizontal LinearLayout with weights is a useful " + + "trick to ensure that only the weights (and not the intrinsic sizes) are used " + + "when sizing the children.\n" + + "\n" + + "However, if you use 0dp for the opposite dimension, the view will be invisible. " + + "This can happen if you change the orientation of a layout without also flipping " + + "the 0dp dimension in all the children.", + Category.CORRECTNESS, + 6, + Severity.ERROR, + InefficientWeightDetector.class, + Scope.RESOURCE_FILE_SCOPE); + /** * Map from element to whether that element has a non-zero linear layout * weight or has an ancestor which does @@ -184,5 +205,78 @@ public class InefficientWeightDetector extends LayoutDetector { } } + + if (context.isEnabled(WRONG_0DP)) { + checkWrong0Dp(context, element, children); + } + } + + private void checkWrong0Dp(XmlContext context, Element element, List<Element> children) { + boolean isVertical = false; + String orientation = element.getAttributeNS(ANDROID_URI, ATTR_ORIENTATION); + if (VALUE_VERTICAL.equals(orientation)) { + isVertical = true; + } + + for (Element child : children) { + String tagName = child.getTagName(); + if (tagName.equals(VIEW)) { + // Might just used for spacing + return; + } + if (tagName.indexOf('.') != -1 || tagName.equals(VIEW_TAG)) { + // Custom views might perform their own dynamic sizing or ignore the layout + // attributes all together + return; + } + + boolean hasWeight = child.hasAttributeNS(ANDROID_URI, ATTR_LAYOUT_WEIGHT); + + Attr widthNode = child.getAttributeNodeNS(ANDROID_URI, ATTR_LAYOUT_WIDTH); + Attr heightNode = child.getAttributeNodeNS(ANDROID_URI, ATTR_LAYOUT_HEIGHT); + + boolean noWidth = false; + boolean noHeight = false; + if (widthNode != null && widthNode.getValue().startsWith("0")) { //$NON-NLS-1$ + noWidth = true; + } + if (heightNode != null && heightNode.getValue().startsWith("0")) { //$NON-NLS-1$ + noHeight = true; + } else if (!noWidth) { + return; + } + + // If you're specifying 0dp for both the width and height you are probably + // trying to hide it deliberately + if (noWidth && noHeight) { + return; + } + assert noWidth || noHeight; + + if (noWidth) { + assert widthNode != null; + if (!hasWeight) { + context.report(WRONG_0DP, widthNode, context.getLocation(widthNode), + "Suspicious size: this will make the view invisible, should be " + + "used with layout_weight", null); + } else if (isVertical) { + context.report(WRONG_0DP, widthNode, context.getLocation(widthNode), + "Suspicious size: this will make the view invisible, probably " + + "intended for layout_height", null); + } + } else { + assert noHeight; + assert heightNode != null; + if (!hasWeight) { + context.report(WRONG_0DP, widthNode, context.getLocation(heightNode), + "Suspicious size: this will make the view invisible, should be " + + "used with layout_weight", null); + } else if (!isVertical) { + context.report(WRONG_0DP, widthNode, context.getLocation(heightNode), + "Suspicious size: this will make the view invisible, probably " + + "intended for layout_width", null); + } + } + } } } diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/InefficientWeightDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/InefficientWeightDetectorTest.java index f7d4a44..ad0da87 100644 --- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/InefficientWeightDetectorTest.java +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/InefficientWeightDetectorTest.java @@ -91,4 +91,28 @@ public class InefficientWeightDetectorTest extends AbstractCheckTest { lintFiles("res/layout/nested_weights2.xml")); } + + public void testWrong0Dp() throws Exception { + assertEquals( + "res/layout/wrong0dp.xml:19: Error: Suspicious size: this will make the view invisible, should be used with layout_weight [Suspicious0dp]\n" + + " android:layout_width=\"0dp\"\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "res/layout/wrong0dp.xml:25: Error: Suspicious size: this will make the view invisible, should be used with layout_weight [Suspicious0dp]\n" + + " android:layout_height=\"0dp\"\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "res/layout/wrong0dp.xml:34: Error: Suspicious size: this will make the view invisible, probably intended for layout_height [Suspicious0dp]\n" + + " android:layout_width=\"0dp\"\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "res/layout/wrong0dp.xml:67: Error: Suspicious size: this will make the view invisible, probably intended for layout_width [Suspicious0dp]\n" + + " android:layout_height=\"0dp\"\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "res/layout/wrong0dp.xml:90: Error: Suspicious size: this will make the view invisible, probably intended for layout_width [Suspicious0dp]\n" + + " android:layout_height=\"0dp\"\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "5 errors, 0 warnings\n", + + lintFiles("res/layout/wrong0dp.xml")); + } + + } diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/wrong0dp.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/wrong0dp.xml new file mode 100644 index 0000000..136329f --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/wrong0dp.xml @@ -0,0 +1,103 @@ +<?xml version="1.0" encoding="utf-8"?> +<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android" + xmlns:tools="http://schemas.android.com/tools" + android:layout_width="match_parent" + android:layout_height="match_parent" + android:orientation="vertical" + tools:ignore="HardcodedText" > + + <!-- Vertical Layout --> + + <LinearLayout + android:layout_width="match_parent" + android:layout_height="match_parent" + android:orientation="vertical" > + + <!-- No weight: Always an error --> + + <Button + android:layout_width="0dp" + android:layout_height="wrap_content" + android:text="Button" /> + + <Button + android:layout_width="wrap_content" + android:layout_height="0dp" + android:text="Button" /> + + <!-- + 0dp not along the orientation axis is wrong; + here layout_height is okay, layout_width is not + --> + + <Button + android:layout_width="0dp" + android:layout_height="wrap_content" + android:layout_weight="1.0" + android:text="Button" /> + + <!-- OK --> + + <Button + android:layout_width="wrap_content" + android:layout_height="0dp" + android:layout_weight="1.0" + android:text="Button" /> + </LinearLayout> + + <!-- Horizontal Layout --> + + <LinearLayout + android:layout_width="match_parent" + android:layout_height="match_parent" + android:orientation="horizontal" > + + <!-- OK --> + + <Button + android:layout_width="0dp" + android:layout_height="wrap_content" + android:layout_weight="1.0" + android:text="Button" /> + + <!-- Not OK --> + + <Button + android:layout_width="wrap_content" + android:layout_height="0dp" + android:layout_weight="1.0" + android:text="Button" /> + </LinearLayout> + + <!-- No orientation specified, so horizontal --> + + <LinearLayout + android:layout_width="match_parent" + android:layout_height="match_parent" > + + <!-- OK --> + + <Button + android:layout_width="0dp" + android:layout_height="wrap_content" + android:layout_weight="1.0" + android:text="Button" /> + + <!-- Not OK --> + + <Button + android:layout_width="wrap_content" + android:layout_height="0dp" + android:layout_weight="1.0" + android:text="Button" /> + + <!-- Check suppressed --> + + <Button + android:layout_width="0dp" + android:layout_height="wrap_content" + android:text="Button" + tools:ignore="Suspicious0dp" /> + </LinearLayout> + +</FrameLayout> |