From e97536a52fd5d3ee3d895d6605ccd723f7d5c3f6 Mon Sep 17 00:00:00 2001 From: Tor Norbye Date: Mon, 17 Sep 2012 15:31:34 -0700 Subject: Add labelFor lint check Change-Id: If2b905197d5d81c6f98315a0d48ff37091e8282b --- common/src/com/android/SdkConstants.java | 3 + .../tools/lint/client/api/DefaultSdkInfo.java | 7 +- .../tools/lint/checks/BuiltinIssueRegistry.java | 3 +- .../tools/lint/checks/LabelForDetector.java | 163 +++++++++++++++++++++ .../tools/lint/checks/LabelForDetectorTest.java | 77 ++++++++++ .../tools/lint/checks/data/apicheck/minsdk17.xml | 23 +++ .../tools/lint/checks/data/res/layout/labelfor.xml | 75 ++++++++++ .../checks/data/res/layout/labelfor_ignore.xml | 16 ++ 8 files changed, 363 insertions(+), 4 deletions(-) create mode 100644 lint/libs/lint_checks/src/com/android/tools/lint/checks/LabelForDetector.java create mode 100644 lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/LabelForDetectorTest.java create mode 100644 lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/minsdk17.xml create mode 100644 lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/labelfor.xml create mode 100644 lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/labelfor_ignore.xml diff --git a/common/src/com/android/SdkConstants.java b/common/src/com/android/SdkConstants.java index 53277c5..1e1d14f 100644 --- a/common/src/com/android/SdkConstants.java +++ b/common/src/com/android/SdkConstants.java @@ -666,6 +666,8 @@ public final class SdkConstants { public static final String VIEW_SWITCHER = "ViewSwitcher"; //$NON-NLS-1$ public static final String EXPANDABLE_LIST_VIEW = "ExpandableListView"; //$NON-NLS-1$ public static final String HORIZONTAL_SCROLL_VIEW = "HorizontalScrollView"; //$NON-NLS-1$ + public static final String MULTI_AUTO_COMPLETE_TEXT_VIEW = "MultiAutoCompleteTextView"; //$NON-NLS-1$ + public static final String AUTO_COMPLETE_TEXT_VIEW = "AutoCompleteTextView"; //$NON-NLS-1$ // Tags: Drawables public static final String TAG_BITMAP = "bitmap"; //$NON-NLS-1$ @@ -721,6 +723,7 @@ public final class SdkConstants { public static final String ATTR_LAYOUT = "layout"; //$NON-NLS-1$ public static final String ATTR_ROW_COUNT = "rowCount"; //$NON-NLS-1$ public static final String ATTR_COLUMN_COUNT = "columnCount"; //$NON-NLS-1$ + public static final String ATTR_LABEL_FOR = "labelFor"; //$NON-NLS-1$ public static final String ATTR_BASELINE_ALIGNED = "baselineAligned"; //$NON-NLS-1$ public static final String ATTR_CONTENT_DESCRIPTION = "contentDescription"; //$NON-NLS-1$ public static final String ATTR_IME_ACTION_LABEL = "imeActionLabel"; //$NON-NLS-1$ diff --git a/lint/libs/lint_api/src/com/android/tools/lint/client/api/DefaultSdkInfo.java b/lint/libs/lint_api/src/com/android/tools/lint/client/api/DefaultSdkInfo.java index 1e53834..b5ae26d 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/client/api/DefaultSdkInfo.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/client/api/DefaultSdkInfo.java @@ -21,6 +21,7 @@ import static com.android.SdkConstants.ABS_LIST_VIEW; import static com.android.SdkConstants.ABS_SEEK_BAR; import static com.android.SdkConstants.ABS_SPINNER; import static com.android.SdkConstants.ADAPTER_VIEW; +import static com.android.SdkConstants.AUTO_COMPLETE_TEXT_VIEW; import static com.android.SdkConstants.BUTTON; import static com.android.SdkConstants.CHECKED_TEXT_VIEW; import static com.android.SdkConstants.CHECK_BOX; @@ -35,6 +36,7 @@ import static com.android.SdkConstants.IMAGE_BUTTON; import static com.android.SdkConstants.IMAGE_VIEW; import static com.android.SdkConstants.LINEAR_LAYOUT; import static com.android.SdkConstants.LIST_VIEW; +import static com.android.SdkConstants.MULTI_AUTO_COMPLETE_TEXT_VIEW; import static com.android.SdkConstants.PROGRESS_BAR; import static com.android.SdkConstants.RADIO_BUTTON; import static com.android.SdkConstants.RADIO_GROUP; @@ -191,6 +193,8 @@ class DefaultSdkInfo extends SdkInfo { PARENTS.put(SCROLL_VIEW, FRAME_LAYOUT); PARENTS.put(GRID_VIEW, ABS_LIST_VIEW); PARENTS.put(WEB_VIEW, ABSOLUTE_LAYOUT); + PARENTS.put(AUTO_COMPLETE_TEXT_VIEW, EDIT_TEXT); + PARENTS.put(MULTI_AUTO_COMPLETE_TEXT_VIEW, AUTO_COMPLETE_TEXT_VIEW); PARENTS.put("CheckedTextView", TEXT_VIEW); //$NON-NLS-1$ PARENTS.put("MediaController", FRAME_LAYOUT); //$NON-NLS-1$ @@ -207,12 +211,9 @@ class DefaultSdkInfo extends SdkInfo { PARENTS.put("TimePicker", FRAME_LAYOUT); //$NON-NLS-1$ PARENTS.put("VideoView", SURFACE_VIEW); //$NON-NLS-1$ PARENTS.put("ZoomButton", IMAGE_BUTTON); //$NON-NLS-1$ - PARENTS.put("AutoCompleteTextView", EDIT_TEXT); //$NON-NLS-1$ PARENTS.put("RatingBar", ABS_SEEK_BAR); //$NON-NLS-1$ PARENTS.put("ViewFlipper", VIEW_ANIMATOR); //$NON-NLS-1$ PARENTS.put("NumberPicker", LINEAR_LAYOUT); //$NON-NLS-1$ - PARENTS.put("MultiAutoCompleteTextView", //$NON-NLS-1$ - "AutoCompleteTextView"); //$NON-NLS-1$ assert PARENTS.size() <= CLASS_COUNT : PARENTS.size(); 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 ca73df0..c5e4e64 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,10 +55,11 @@ public class BuiltinIssueRegistry extends IssueRegistry { private static final List sIssues; static { - final int initialCapacity = 109; + final int initialCapacity = 110; List issues = new ArrayList(initialCapacity); issues.add(AccessibilityDetector.ISSUE); + issues.add(LabelForDetector.ISSUE); issues.add(MathDetector.ISSUE); issues.add(FieldGetterDetector.ISSUE); issues.add(SdCardDetector.ISSUE); diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/LabelForDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/LabelForDetector.java new file mode 100644 index 0000000..74e510d --- /dev/null +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/LabelForDetector.java @@ -0,0 +1,163 @@ +/* + * 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.SdkConstants.ANDROID_URI; +import static com.android.SdkConstants.ATTR_ID; +import static com.android.SdkConstants.ATTR_LABEL_FOR; +import static com.android.SdkConstants.AUTO_COMPLETE_TEXT_VIEW; +import static com.android.SdkConstants.EDIT_TEXT; +import static com.android.SdkConstants.ID_PREFIX; +import static com.android.SdkConstants.MULTI_AUTO_COMPLETE_TEXT_VIEW; +import static com.android.SdkConstants.NEW_ID_PREFIX; +import static com.android.tools.lint.detector.api.LintUtils.stripIdPrefix; + +import com.android.annotations.NonNull; +import com.android.annotations.Nullable; +import com.android.tools.lint.detector.api.Category; +import com.android.tools.lint.detector.api.Context; +import com.android.tools.lint.detector.api.Issue; +import com.android.tools.lint.detector.api.LayoutDetector; +import com.android.tools.lint.detector.api.Location; +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 com.android.tools.lint.detector.api.XmlContext; +import com.google.common.collect.Sets; + +import org.w3c.dom.Attr; +import org.w3c.dom.Element; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +/** + * Detector which finds unlabeled text fields + */ +public class LabelForDetector extends LayoutDetector { + /** The main issue discovered by this detector */ + public static final Issue ISSUE = Issue.create( + "LabelFor", //$NON-NLS-1$ + "Ensures that text fields are marked with a labelFor attribute", + "Text fields should be labelled with a `labelFor` attribute, " + + "provided your `minSdkVersion` is at least 17.\n" + + "\n" + + "If your view is labeled but by a label in a different layout which " + + "includes this one, just suppress this warning from lint.", + Category.A11Y, + 2, + Severity.WARNING, + LabelForDetector.class, + Scope.RESOURCE_FILE_SCOPE); + + private Set mLabels; + private List mTextFields; + + /** Constructs a new {@link LabelForDetector} */ + public LabelForDetector() { + } + + @Override + public @NonNull Speed getSpeed() { + return Speed.FAST; + } + + @Override + @Nullable + public Collection getApplicableAttributes() { + return Collections.singletonList(ATTR_LABEL_FOR); + } + + @Override + public Collection getApplicableElements() { + return Arrays.asList( + EDIT_TEXT, + AUTO_COMPLETE_TEXT_VIEW, + MULTI_AUTO_COMPLETE_TEXT_VIEW + ); + } + + @Override + public void afterCheckFile(@NonNull Context context) { + if (mTextFields != null) { + if (mLabels == null) { + mLabels = Collections.emptySet(); + } + + for (Element element : mTextFields) { + String id = element.getAttributeNS(ANDROID_URI, ATTR_ID); + boolean missing = true; + if (mLabels.contains(id)) { + missing = false; + } else if (id.startsWith(NEW_ID_PREFIX)) { + missing = !mLabels.contains(ID_PREFIX + stripIdPrefix(id)); + } else if (id.startsWith(ID_PREFIX)) { + missing = !mLabels.contains(NEW_ID_PREFIX + stripIdPrefix(id)); + } + + if (missing) { + XmlContext xmlContext = (XmlContext) context; + Location location = xmlContext.getLocation(element); + String message; + if (id == null || id.isEmpty()) { + message = "No label views point to this text field with a " + + "labelFor attribute"; + } else { + message = String.format("No label views point to this text field with " + + "an android:labelFor=\"@+id/%1$s\" attribute", id); + } + xmlContext.report(ISSUE, element, location, message, null); + } + + } + } + + mLabels = null; + mTextFields = null; + } + + @Override + public void visitAttribute(@NonNull XmlContext context, @NonNull Attr attribute) { + if (mLabels == null) { + mLabels = Sets.newHashSet(); + } + mLabels.add(attribute.getValue()); + } + + @Override + public void visitElement(@NonNull XmlContext context, @NonNull Element element) { + // NOTE: This should NOT be checking *minSdkVersion*, but *targetSdkVersion* + // or even buildTarget instead. However, there's a risk that this will flag + // way too much and make the rule annoying until API 17 support becomes + // more widespread, so for now limit the check to those projects *really* + // working with 17. When API 17 reaches a certain amount of adoption, change + // this to flag all apps supporting 17, including those supporting earlier + // versions as well. + if (context.getMainProject().getMinSdk() < 17) { + return; + } + + if (mTextFields == null) { + mTextFields = new ArrayList(); + } + mTextFields.add(element); + } +} diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/LabelForDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/LabelForDetectorTest.java new file mode 100644 index 0000000..7e139af --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/LabelForDetectorTest.java @@ -0,0 +1,77 @@ +/* + * 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 LabelForDetectorTest extends AbstractCheckTest { + @Override + protected Detector getDetector() { + return new LabelForDetector(); + } + + public void test() throws Exception { + assertEquals( + "res/layout/labelfor.xml:54: Warning: No label views point to this text field with an android:labelFor=\"@+id/@+id/editText2\" attribute [LabelFor]\n" + + " AndroidManifest.xml", + "res/layout/labelfor.xml" + )); + } + + public void testSuppressed() throws Exception { + assertEquals( + "No warnings.", + + lintProject( + "apicheck/minsdk17.xml=>AndroidManifest.xml", + "res/layout/labelfor_ignore.xml" + )); + } + + + public void testOk() throws Exception { + assertEquals( + "No warnings.", + + lintProject( + "apicheck/minsdk17.xml=>AndroidManifest.xml", + "res/layout/accessibility.xml" + )); + } + + public void testNotApplicable() throws Exception { + assertEquals( + "No warnings.", + + lintProject( + "apicheck/minsdk14.xml=>AndroidManifest.xml", + "res/layout/labelfor.xml" + )); + } +} + diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/minsdk17.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/minsdk17.xml new file mode 100644 index 0000000..1837db9 --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/apicheck/minsdk17.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/labelfor.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/labelfor.xml new file mode 100644 index 0000000..7eec47e --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/labelfor.xml @@ -0,0 +1,75 @@ + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/labelfor_ignore.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/labelfor_ignore.xml new file mode 100644 index 0000000..41d2821 --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/labelfor_ignore.xml @@ -0,0 +1,16 @@ + + + + + + -- cgit v1.1