Add IgnoreWithoutReasonDetector

This will add the new IgnoreWithoutReasonDetector to the built in lint
checks. It checks that there is a reason defined when using the @Ignored
annotation from JUnit.

Cherrypick from AOSP:
https://android-review.googlesource.com/c/platform/tools/base/+/783316
plus simple quickfix, and added testing category

Bug: None
Test: Unit test included
Change-Id: I06d1f72c2b31f3f563f96e4c4fb6e15630e9ecd7
diff --git a/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/Category.kt b/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/Category.kt
index c6dc924..78f57c9 100644
--- a/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/Category.kt
+++ b/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/Category.kt
@@ -170,15 +170,15 @@
 
         /** Issues around interoperability between Java, Kotlin, etc */
         @JvmField
-        val INTEROPERABILITY = create("Interoperability", 48)
+        val INTEROPERABILITY = create("Interoperability", 46)
 
         /** Issues around interoperability calling Java from Kotlin */
         @JvmField
-        val INTEROPERABILITY_KOTLIN = create(INTEROPERABILITY, "Kotlin Interoperability", 46)
+        val INTEROPERABILITY_KOTLIN = create(INTEROPERABILITY, "Kotlin Interoperability", 44)
 
         /** Issues around interoperability calling Kotlin from Java */
         @JvmField
-        val INTEROPERABILITY_JAVA = create(INTEROPERABILITY, "Java Interoperability", 44)
+        val INTEROPERABILITY_JAVA = create(INTEROPERABILITY, "Java Interoperability", 42)
 
         /** Issues related to Chrome OS devices  */
         @JvmField
@@ -186,7 +186,11 @@
 
         /** Issues related to right to left and bidirectional text support  */
         @JvmField
-        val RTL = create(I18N, "Bidirectional Text", 40)
+        val RTL = create(I18N, "Bidirectional Text", 49)
+
+        /** Issues related to writing correct tests  */
+        @JvmField
+        val TESTING = create(null, "Testing", 48)
 
         /** Issues related to increased application size  */
         @JvmField
diff --git a/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/LintFix.java b/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/LintFix.java
index 1d01d63..c519cf9 100644
--- a/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/LintFix.java
+++ b/lint/libs/lint-api/src/main/java/com/android/tools/lint/detector/api/LintFix.java
@@ -30,6 +30,7 @@
 import java.util.Set;
 import java.util.regex.Matcher;
 import org.intellij.lang.annotations.Language;
+import org.intellij.lang.annotations.RegExp;
 import org.jetbrains.annotations.Nls;
 
 /**
@@ -459,6 +460,7 @@
         @Nls @Nullable protected String familyName;
         private String newText;
         private String oldText;
+        private String selectPattern;
         private boolean shortenNames;
         private boolean reformat;
         private boolean robot;
@@ -561,6 +563,15 @@
         }
 
         /**
+         * Sets a pattern to select; if it contains parentheses, group(1) will be selected. To just
+         * set the caret, use an empty group.
+         */
+        public ReplaceStringBuilder select(@RegExp @Nullable String selectPattern) {
+            this.selectPattern = selectPattern;
+            return this;
+        }
+
+        /**
          * The text to replace the old text or pattern with. Note that the special syntax \g{n} can
          * be used to reference the n'th group, if and only if this replacement is using {@link
          * #pattern(String)}}.
@@ -673,6 +684,7 @@
                     familyName,
                     oldText,
                     oldPattern,
+                    selectPattern,
                     newText != null ? newText : "",
                     shortenNames,
                     reformat,
@@ -1212,7 +1224,7 @@
                 int mark,
                 boolean robot,
                 boolean independent) {
-            super(displayName);
+            super(displayName, familyName);
             this.namespace = namespace;
             this.attribute = attribute;
             this.value = value;
@@ -1269,6 +1281,10 @@
          * replacement range.
          */
         @Nullable public final String oldPattern;
+
+        /** Pattern to select; if it contains parentheses, group(1) will be selected */
+        @Nullable public final String selectPattern;
+
         /** The replacement string. */
         @NonNull public final String replacement;
 
@@ -1309,6 +1325,7 @@
                 @Nullable String familyName,
                 @Nullable String oldString,
                 @Nullable String oldPattern,
+                @Nullable String selectPattern,
                 @NonNull String replacement,
                 boolean shortenNames,
                 boolean reformat,
@@ -1318,6 +1335,7 @@
             super(displayName, familyName);
             this.oldString = oldString;
             this.oldPattern = oldPattern;
+            this.selectPattern = selectPattern;
             this.replacement = replacement;
             this.shortenNames = shortenNames;
             this.reformat = reformat;
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 11f7056..f28cf32 100755
--- 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
@@ -169,6 +169,7 @@
         issues.add(IconDetector.NOTIFICATION_ICON_COMPATIBILITY);
         issues.add(IconDetector.WEBP_ELIGIBLE);
         issues.add(IconDetector.WEBP_UNSUPPORTED);
+        issues.add(IgnoreWithoutReasonDetector.ISSUE);
         issues.add(IncludeDetector.ISSUE);
         issues.add(InefficientWeightDetector.BASELINE_WEIGHTS);
         issues.add(InefficientWeightDetector.INEFFICIENT_WEIGHT);
diff --git a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetector.kt b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetector.kt
new file mode 100755
index 0000000..665c80d
--- /dev/null
+++ b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetector.kt
@@ -0,0 +1,100 @@
+/*
+ * Copyright (C) 2018 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 com.android.SdkConstants.ATTR_VALUE
+import com.android.tools.lint.client.api.UElementHandler
+import com.android.tools.lint.detector.api.Category
+import com.android.tools.lint.detector.api.ConstantEvaluator
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Implementation
+import com.android.tools.lint.detector.api.Issue
+import com.android.tools.lint.detector.api.JavaContext
+import com.android.tools.lint.detector.api.LintFix
+import com.android.tools.lint.detector.api.Scope
+import com.android.tools.lint.detector.api.Severity
+import org.jetbrains.uast.UAnnotated
+import org.jetbrains.uast.UClass
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UMethod
+import java.util.EnumSet
+
+/**
+ * It checks that there is a reason defined when using the @Ignored annotation from JUnit.
+ */
+class IgnoreWithoutReasonDetector : Detector(), Detector.UastScanner {
+    companion object {
+        @JvmField
+        val ISSUE = Issue.create(
+            id = "IgnoreWithoutReason",
+            briefDescription = "@Ignore without Reason",
+            explanation = """
+            Ignoring a test without a reason makes it difficult to figure out the problem later.
+            Please define an explicit reason why it is ignored, and when it can be resolved.""",
+            category = Category.TESTING,
+            priority = 2,
+            severity = Severity.WARNING,
+            implementation = Implementation(
+                IgnoreWithoutReasonDetector::class.java,
+                EnumSet.of(Scope.JAVA_FILE, Scope.TEST_SOURCES),
+                Scope.JAVA_FILE_SCOPE
+            )
+        )
+    }
+
+    override fun getApplicableUastTypes(): List<Class<out UElement>> =
+        listOf(UMethod::class.java, UClass::class.java)
+
+    override fun createUastHandler(context: JavaContext): UElementHandler = IgnoreAnnotationVisitor(context)
+
+    internal class IgnoreAnnotationVisitor(private val context: JavaContext) : UElementHandler() {
+        override fun visitMethod(node: UMethod) = processAnnotations(node, node)
+
+        override fun visitClass(node: UClass) = processAnnotations(node, node)
+
+        private fun processAnnotations(element: UElement, annotated: UAnnotated) {
+            val annotations = context.evaluator.getAllAnnotations(annotated, false)
+            val ignoreAnnotation = annotations.firstOrNull { it.qualifiedName == "org.junit.Ignore" }
+
+            if (ignoreAnnotation != null) {
+                val attribute = ignoreAnnotation.findAttributeValue(ATTR_VALUE)
+                val hasDescription =
+                    attribute != null &&
+                    run {
+                        val value = ConstantEvaluator.evaluate(context, attribute) as? String
+                        value != null && value.isNotBlank() && value != "TODO"
+                    }
+                if (!hasDescription) {
+                    val fix =
+                        if (attribute == null || ignoreAnnotation.attributeValues.isEmpty()) {
+                            LintFix.create()
+                                .name("Give reason")
+                                .replace()
+                                .end()
+                                .with("(\"TODO\")")
+                                .select("TODO")
+                                .build()
+                        } else {
+                            null
+                        }
+                    context.report(ISSUE, element, context.getLocation(ignoreAnnotation),
+                        "Test is ignored without giving any explanation.", fix)
+                }
+            }
+        }
+    }
+}
diff --git a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/LintFixVerifier.java b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/LintFixVerifier.java
index daca957..d6f97a5 100644
--- a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/LintFixVerifier.java
+++ b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/LintFixVerifier.java
@@ -503,7 +503,37 @@
             }
         }
 
-        return contents.substring(0, startOffset) + replacement + contents.substring(endOffset);
+        String s = contents.substring(0, startOffset) + replacement + contents.substring(endOffset);
+
+        // Insert selection/caret markers if configured for this fix
+        if (replaceFix.selectPattern != null) {
+            Pattern pattern = Pattern.compile(replaceFix.selectPattern);
+            Matcher matcher = pattern.matcher(s);
+            if (matcher.find(start.getOffset())) {
+                int selectStart;
+                int selectEnd;
+                if (matcher.groupCount() > 0) {
+                    selectStart = matcher.start(1);
+                    selectEnd = matcher.end(1);
+                } else {
+                    selectStart = matcher.start();
+                    selectEnd = matcher.end();
+                }
+                if (selectStart == selectEnd) {
+                    s = s.substring(0, selectStart) + "|" + s.substring(selectEnd);
+
+                } else {
+                    s =
+                            s.substring(0, selectStart)
+                                    + "["
+                                    + s.substring(selectStart, selectEnd)
+                                    + "]"
+                                    + s.substring(selectEnd);
+                }
+            }
+        }
+
+        return s;
     }
 
     private void appendDiff(
diff --git a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintClient.java b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintClient.java
index 037193c..002254b 100644
--- a/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintClient.java
+++ b/lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/TestLintClient.java
@@ -791,13 +791,16 @@
             if (oldString != null) {
                 int startIndex = contents.indexOf(oldString, start.getOffset());
                 if (startIndex == -1 || startIndex > end.getOffset()) {
-                    fail(
-                            "Did not find \""
-                                    + oldString
-                                    + "\" in \""
-                                    + locationRange
-                                    + "\" as suggested in the quickfix for issue "
-                                    + issue);
+                    if (!(oldString.equals(LintFix.ReplaceString.INSERT_BEGINNING)
+                            || oldString.equals(LintFix.ReplaceString.INSERT_END))) {
+                        fail(
+                                "Did not find \""
+                                        + oldString
+                                        + "\" in \""
+                                        + locationRange
+                                        + "\" as suggested in the quickfix for issue "
+                                        + issue);
+                    }
                 }
             } else if (oldPattern != null) {
                 Pattern pattern = Pattern.compile(oldPattern);
diff --git a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetectorTest.kt b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetectorTest.kt
new file mode 100755
index 0000000..98dd9a8
--- /dev/null
+++ b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/checks/IgnoreWithoutReasonDetectorTest.kt
@@ -0,0 +1,196 @@
+/*
+ * Copyright (C) 2018 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 com.android.testutils.TestUtils
+import com.android.tools.lint.checks.infrastructure.TestFile
+import com.android.tools.lint.checks.infrastructure.TestFiles.java
+import com.android.tools.lint.checks.infrastructure.TestLintTask
+import org.junit.Test
+
+class IgnoreWithoutReasonDetectorTest {
+    private fun lint(): TestLintTask {
+        return TestLintTask().sdkHome(TestUtils.getSdk())
+    }
+
+    private val stubJUnitTest: TestFile = java(
+        """
+        package org.junit;
+
+        @SuppressWarnings("ClassNameDiffersFromFileName")
+        public @interface Test { }"""
+    ).indented()
+
+    private val stubJUnitIgnore: TestFile = java(
+        """
+        package org.junit;
+
+        @SuppressWarnings("ClassNameDiffersFromFileName")
+        public @interface Ignore {
+            String value() default "";
+        }"""
+    ).indented()
+
+    @Test
+    fun testNoAnnotations() {
+        lint()
+            .files(
+                stubJUnitTest, java(
+                    """
+                package foo;
+
+                import org.junit.Test;
+
+                @SuppressWarnings("ClassNameDiffersFromFileName")
+                class MyTest {
+                  @Test fun something() {
+                  }
+                }"""
+                ).indented()
+            )
+            .issues(IgnoreWithoutReasonDetector.ISSUE)
+            .run()
+            .expectClean()
+    }
+
+    @Test
+    fun testAnnotationWithReasonOnFunction() {
+        lint()
+            .files(
+                stubJUnitTest, stubJUnitIgnore, java(
+                    """
+                package foo;
+
+                import org.junit.Ignore;
+                import org.junit.Test;
+
+                @SuppressWarnings("ClassNameDiffersFromFileName")
+                class MyTest {
+                  @Test @Ignore("reason") fun something() {
+                  }
+                }"""
+                ).indented()
+            )
+            .issues(IgnoreWithoutReasonDetector.ISSUE)
+            .run()
+            .expectClean()
+    }
+
+    @Test
+    fun testAnnotationWithReasonOnClass() {
+        lint()
+            .files(
+                stubJUnitTest, stubJUnitIgnore, java(
+                    """
+                package foo;
+
+                import org.junit.Ignore;
+                import org.junit.Test;
+
+                @SuppressWarnings("ClassNameDiffersFromFileName")
+                @Ignore("reason") class MyTest {
+                  @Test fun something() {
+                  }
+                }"""
+                ).indented()
+            )
+            .issues(IgnoreWithoutReasonDetector.ISSUE)
+            .run()
+            .expectClean()
+    }
+
+    @Test
+    fun testAnnotationWithoutReasonOnClass() {
+        lint()
+            .files(
+                stubJUnitTest, stubJUnitIgnore, java(
+                    """
+                package foo;
+
+                import org.junit.Ignore;
+                import org.junit.Test;
+
+                @SuppressWarnings("ClassNameDiffersFromFileName")
+                @Ignore class MyTest {
+                  @Test fun something() {
+                  }
+                }"""
+                ).indented()
+            )
+            .issues(IgnoreWithoutReasonDetector.ISSUE)
+            .run()
+            .expect(
+                """
+                src/foo/MyTest.java:7: Warning: Test is ignored without giving any explanation. [IgnoreWithoutReason]
+                @Ignore class MyTest {
+                ~~~~~~~
+                0 errors, 1 warnings
+                """
+            )
+    }
+
+    @Test
+    fun testAnnotationWithoutReasonOnFunction() {
+        lint()
+            .files(
+                stubJUnitTest, stubJUnitIgnore, java(
+                    """
+                package foo;
+
+                import org.junit.Ignore;
+                import org.junit.Test;
+
+                @SuppressWarnings({"ClassNameDiffersFromFileName", "DefaultAnnotationParam"})
+                class MyTest {
+                  @Test @Ignore fun something() {
+                  }
+
+                  @Test @Ignore("") fun something() {
+                  }
+
+                  @Test @Ignore("TODO") fun something() {
+                  }
+                }
+                """
+                ).indented()
+            )
+            .issues(IgnoreWithoutReasonDetector.ISSUE)
+            .run()
+            .expect(
+                """
+                src/foo/MyTest.java:8: Warning: Test is ignored without giving any explanation. [IgnoreWithoutReason]
+                  @Test @Ignore fun something() {
+                        ~~~~~~~
+                src/foo/MyTest.java:11: Warning: Test is ignored without giving any explanation. [IgnoreWithoutReason]
+                  @Test @Ignore("") fun something() {
+                        ~~~~~~~~~~~
+                src/foo/MyTest.java:14: Warning: Test is ignored without giving any explanation. [IgnoreWithoutReason]
+                  @Test @Ignore("TODO") fun something() {
+                        ~~~~~~~~~~~~~~~
+                0 errors, 3 warnings
+                """
+            )
+            .expectFixDiffs(
+                """
+                Fix for src/foo/MyTest.java line 8: Give reason:
+                @@ -8 +8
+                -   @Test @Ignore fun something() {
+                +   @Test @Ignore("[TODO]") fun something() {
+                """
+            )
+    }
+}
diff --git a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/detector/api/CategoryTest.kt b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/detector/api/CategoryTest.kt
index ef1cdb9..a258352 100644
--- a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/detector/api/CategoryTest.kt
+++ b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/detector/api/CategoryTest.kt
@@ -51,10 +51,11 @@
                     "Usability\n" +
                     "Accessibility\n" +
                     "Internationalization\n" +
+                    "Internationalization:Bidirectional Text\n" +
+                    "Testing\n" +
                     "Interoperability\n" +
                     "Interoperability:Kotlin Interoperability\n" +
-                    "Interoperability:Java Interoperability\n" +
-                    "Internationalization:Bidirectional Text",
+                    "Interoperability:Java Interoperability",
             Joiner.on("\n").join(categories)
         )
     }
diff --git a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/detector/api/LintFixTest.kt b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/detector/api/LintFixTest.kt
index da94379..4a0af03 100644
--- a/lint/libs/lint-tests/src/test/java/com/android/tools/lint/detector/api/LintFixTest.kt
+++ b/lint/libs/lint-tests/src/test/java/com/android/tools/lint/detector/api/LintFixTest.kt
@@ -62,13 +62,13 @@
         assertThat(foundInteger).isTrue()
         assertThat(foundBigDecimal).isTrue()
 
-        TestCase.assertNotNull(LintFix.getData(quickfixData, BigDecimal::class.java))
+        assertNotNull(LintFix.getData(quickfixData, BigDecimal::class.java))
 
         // Check key conversion to general interface
-        TestCase.assertNull(LintFix.getData(quickfixData, ArrayList::class.java))
-        TestCase.assertNotNull(LintFix.getData(quickfixData, List::class.java))
-        TestCase.assertNull(LintFix.getData(quickfixData, HashMap::class.java))
-        TestCase.assertNotNull(LintFix.getData(quickfixData, Map::class.java))
+        assertNull(LintFix.getData(quickfixData, ArrayList::class.java))
+        assertNotNull(LintFix.getData(quickfixData, List::class.java))
+        assertNull(LintFix.getData(quickfixData, HashMap::class.java))
+        assertNotNull(LintFix.getData(quickfixData, Map::class.java))
     }
 
     fun testClassInheritance() {
@@ -111,11 +111,11 @@
     fun testGroupMatching() {
         val fix = LintFix.create().replace().pattern("abc\\((\\d+)\\)def")
             .with("Number was \\k<1>! I said \\k<1>!").build() as ReplaceString
-        TestCase.assertTrue(fix.oldPattern != null)
+        assertTrue(fix.oldPattern != null)
         val matcher = Pattern.compile(fix.oldPattern).matcher("abc(42)def")
-        TestCase.assertTrue(matcher.matches())
+        assertTrue(matcher.matches())
         val expanded = fix.expandBackReferences(matcher)
-        TestCase.assertEquals("Number was 42! I said 42!", expanded)
+        assertEquals("Number was 42! I said 42!", expanded)
     }
 
     fun testMatching() {