Open-source new checks (#317)

Open-sources `AvoidUsingNotNullOperator`, `InflationInItemDecoration`, and `DoNotCallViewToString` checks.

CHANGELOG.md

@@ -1,6 +1,11 @@
 Changelog
 =========
 
+**Unreleased**
+--------------
+
+- Open-source `AvoidUsingNotNullOperator`, `InflationInItemDecoration`, and `DoNotCallViewToString` checks.
+
 0.8.0
 -----
 

slack-lint-checks/src/main/java/slack/lint/NotNullOperatorDetector.kt

@@ -0,0 +1,54 @@
+// Copyright (C) 2024 Slack Technologies, LLC
+// SPDX-License-Identifier: Apache-2.0
+package slack.lint
+
+import com.android.tools.lint.client.api.UElementHandler
+import com.android.tools.lint.detector.api.Category
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+import com.android.tools.lint.detector.api.JavaContext
+import com.android.tools.lint.detector.api.Severity
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import kotlin.jvm.java
+import org.jetbrains.uast.UPostfixExpression
+import org.jetbrains.uast.kotlin.isKotlin
+import slack.lint.util.sourceImplementation
+
+class NotNullOperatorDetector : Detector(), SourceCodeScanner {
+  override fun getApplicableUastTypes() = listOf(UPostfixExpression::class.java)
+
+  override fun createUastHandler(context: JavaContext): UElementHandler? {
+    if (!isKotlin(context.uastFile?.lang)) return null
+
+    return object : UElementHandler() {
+      override fun visitPostfixExpression(node: UPostfixExpression) {
+        if (node.operator.text == "!!") {
+          // Report a warning for the usage of `!!`
+          context.report(
+            ISSUE,
+            node,
+            context.getLocation(node.operatorIdentifier ?: node),
+            "Avoid using the `!!` operator",
+          )
+        }
+      }
+    }
+  }
+
+  companion object {
+    val ISSUE: Issue =
+      Issue.create(
+        "AvoidUsingNotNullOperator",
+        "Avoid using the !! operator in Kotlin",
+        """
+        The `!!` operator is a not-null assertion in Kotlin that will lead to a \
+        `NullPointerException` if the value is null. It's better to use safe \
+        null-handling mechanisms like `?.`, `?:`, `?.let`, etc.
+        """,
+        Category.CORRECTNESS,
+        6,
+        Severity.WARNING,
+        sourceImplementation<NotNullOperatorDetector>(),
+      )
+  }
+}

slack-lint-checks/src/main/java/slack/lint/SlackIssueRegistry.kt

@@ -19,6 +19,8 @@ import slack.lint.resources.WrongResourceImportAliasDetector
 import slack.lint.retrofit.RetrofitUsageDetector
 import slack.lint.rx.RxSubscribeOnMainDetector
 import slack.lint.text.SpanMarkPointMissingMaskDetector
+import slack.lint.ui.DoNotCallViewToString
+import slack.lint.ui.ItemDecorationViewBindingDetector
 
 @AutoService(IssueRegistry::class)
 class SlackIssueRegistry : IssueRegistry() {
@@ -66,5 +68,8 @@ class SlackIssueRegistry : IssueRegistry() {
     add(ParcelizeFunctionPropertyDetector.ISSUE)
     add(ExceptionMessageDetector.ISSUE)
     add(MustUseNamedParamsDetector.ISSUE)
+    add(NotNullOperatorDetector.ISSUE)
+    add(DoNotCallViewToString.ISSUE)
+    add(ItemDecorationViewBindingDetector.ISSUE)
   }
 }

slack-lint-checks/src/main/java/slack/lint/ui/DoNotCallViewToString.kt

@@ -0,0 +1,52 @@
+// Copyright (C) 2024 Slack Technologies, LLC
+// SPDX-License-Identifier: Apache-2.0
+package slack.lint.ui
+
+import com.android.tools.lint.client.api.UElementHandler
+import com.android.tools.lint.detector.api.Category
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+import com.android.tools.lint.detector.api.JavaContext
+import com.android.tools.lint.detector.api.Severity
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import com.intellij.psi.util.InheritanceUtil
+import org.jetbrains.uast.UCallExpression
+import slack.lint.util.sourceImplementation
+
+class DoNotCallViewToString : Detector(), SourceCodeScanner {
+  override fun getApplicableUastTypes() = listOf(UCallExpression::class.java)
+
+  override fun createUastHandler(context: JavaContext): UElementHandler {
+    return object : UElementHandler() {
+      override fun visitCallExpression(node: UCallExpression) {
+        if (node.methodName != "toString") return
+        val method = node.resolve() ?: return
+        val containingClass = method.containingClass ?: return
+        if (InheritanceUtil.isInheritor(containingClass, "android.view.View")) {
+          context.report(
+            ISSUE,
+            node,
+            context.getNameLocation(node),
+            "Do not call `View.toString()`",
+          )
+        }
+      }
+    }
+  }
+
+  companion object {
+    val ISSUE: Issue =
+      Issue.Companion.create(
+        "DoNotCallViewToString",
+        "Do not use `View.toString()`",
+        """
+        `View.toString()` and its overrides can often print surprisingly detailed information about \
+        the current view state, and has led to PII logging issues in the past.
+      """,
+        Category.Companion.SECURITY,
+        9,
+        Severity.ERROR,
+        sourceImplementation<DoNotCallViewToString>(),
+      )
+  }
+}

slack-lint-checks/src/main/java/slack/lint/ui/ItemDecorationViewBindingDetector.kt

@@ -0,0 +1,77 @@
+// Copyright (C) 2024 Slack Technologies, LLC
+// SPDX-License-Identifier: Apache-2.0
+package slack.lint.ui
+
+import com.android.tools.lint.client.api.UElementHandler
+import com.android.tools.lint.detector.api.Category
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+import com.android.tools.lint.detector.api.JavaContext
+import com.android.tools.lint.detector.api.Severity
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import com.intellij.psi.PsiClass
+import com.intellij.psi.util.InheritanceUtil.isInheritor
+import kotlin.jvm.java
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.getContainingUClass
+import slack.lint.util.implements
+import slack.lint.util.sourceImplementation
+
+/**
+ * Lint detector that ensures `ItemDecoration` does not inflate a view. If the view contains
+ * `TextView`, this textual information cannot be announced to screen readers by TalkBack.
+ */
+class ItemDecorationViewBindingDetector : Detector(), SourceCodeScanner {
+  override fun getApplicableUastTypes() = listOf(UCallExpression::class.java)
+
+  override fun createUastHandler(context: JavaContext): UElementHandler =
+    object : UElementHandler() {
+
+      override fun visitCallExpression(node: UCallExpression) {
+        val containingClass = node.getContainingUClass() ?: return
+        if (!containingClass.implements(ITEM_DECORATION_CLASS_NAME)) return
+
+        val method = node.resolve() ?: return
+        val methodClass = method.containingClass
+
+        if (
+          LAYOUT_INFLATER_METHOD_NAME == method.name && methodClass?.isViewBindingClass() == true
+        ) {
+          context.report(ISSUE, node, context.getNameLocation(node), ISSUE_DESCRIPTION)
+        }
+      }
+    }
+
+  private fun PsiClass.isViewBindingClass(): Boolean {
+    return LAYOUT_INFLATER_PACKAGE_NAME == this.qualifiedName ||
+      isInheritor(this, VIEW_BINDING_PACKAGE_NAME)
+  }
+
+  companion object {
+    private const val ISSUE_ID = "InflationInItemDecoration"
+    private const val ITEM_DECORATION_CLASS_NAME =
+      "androidx.recyclerview.widget.RecyclerView.ItemDecoration"
+    private const val LAYOUT_INFLATER_METHOD_NAME = "inflate"
+    private const val LAYOUT_INFLATER_PACKAGE_NAME = "android.view.LayoutInflater"
+    private const val VIEW_BINDING_PACKAGE_NAME = "androidx.viewbinding.ViewBinding"
+
+    private const val ISSUE_BRIEF_DESCRIPTION = "Avoid inflating a view to display text"
+    private const val ISSUE_DESCRIPTION =
+      """
+        ViewBinding should not be used in `ItemDecoration`. If an inflated view contains \
+        meaningful textual information, it will not be visible to TalkBack. This means \
+        that screen reader users will not be able to know what is on the screen.
+      """
+
+    val ISSUE =
+      Issue.create(
+        id = ISSUE_ID,
+        briefDescription = ISSUE_BRIEF_DESCRIPTION,
+        explanation = ISSUE_DESCRIPTION,
+        category = Category.A11Y,
+        priority = 10,
+        severity = Severity.WARNING,
+        implementation = sourceImplementation<ItemDecorationViewBindingDetector>(),
+      )
+  }
+}

slack-lint-checks/src/test/java/slack/lint/NotNullOperatorDetectorTest.kt

@@ -0,0 +1,66 @@
+// Copyright (C) 2024 Slack Technologies, LLC
+// SPDX-License-Identifier: Apache-2.0
+package slack.lint
+
+import kotlin.text.trimIndent
+import org.junit.Test
+
+class NotNullOperatorDetectorTest : BaseSlackLintTest() {
+  override fun getDetector() = NotNullOperatorDetector()
+
+  override fun getIssues() = listOf(NotNullOperatorDetector.ISSUE)
+
+  @Test
+  fun testDocumentationExample() {
+    lint()
+      .files(
+        kotlin(
+          """
+            package foo
+
+            class Test {
+              fun doNothing(t: String?): Boolean {
+                t/* this is legal */!!.length
+                return t!!.length == 1
+              }
+            }
+          """
+        )
+      )
+      .run()
+      .expect(
+        """
+          src/foo/Test.kt:6: Warning: Avoid using the !! operator [AvoidUsingNotNullOperator]
+                          t/* this is legal */!!.length
+                                              ~~
+          src/foo/Test.kt:7: Warning: Avoid using the !! operator [AvoidUsingNotNullOperator]
+                          return t!!.length == 1
+                                  ~~
+          0 errors, 2 warnings
+        """
+          .trimIndent()
+      )
+  }
+
+  @Test
+  fun `test clean`() {
+    lint()
+      .files(
+        kotlin(
+          """
+            package foo
+
+            class Test {
+              fun doNothing(t: String?): Boolean {
+                // Should not match despite the !! string in it
+                "!!".length++
+                return t?.length == 1
+              }
+            }
+          """
+        )
+      )
+      .run()
+      .expectClean()
+  }
+}