Fix warnings in the linter.

Author: vkryachko

firebase-messaging/src/main/java/com/google/firebase/messaging/FcmExecutors.java

@@ -100,6 +100,8 @@ static ExecutorService newFileExecutor() {
     return Executors.newSingleThreadExecutor(new NamedThreadFactory(THREAD_FILE));
   }
 
+  // TODO(b/258424124): Migrate to go/firebase-android-executors
+  @SuppressLint("ThreadPoolCreation")
   static ExecutorService newIntentHandleExecutor() {
     return PoolableExecutors.factory()
         .newSingleThreadExecutor(

tools/lint/src/main/kotlin/CheckRegistry.kt

@@ -15,6 +15,7 @@
 package com.google.firebase.lint.checks
 
 import com.android.tools.lint.client.api.IssueRegistry
+import com.android.tools.lint.client.api.Vendor
 import com.android.tools.lint.detector.api.CURRENT_API
 import com.android.tools.lint.detector.api.Issue
 
@@ -37,4 +38,12 @@ class CheckRegistry : IssueRegistry() {
     get() = CURRENT_API
   override val minApi: Int
     get() = 2
+
+  override val vendor: Vendor
+    get() =
+      Vendor(
+        "firebase",
+        identifier = "firebase",
+        feedbackUrl = "https://github.com/firebase/firebase-android-sdk/issues"
+      )
 }

tools/lint/src/main/kotlin/DeferredApiDetector.kt

@@ -14,6 +14,9 @@
 
 package com.google.firebase.lint.checks
 
+import com.android.tools.lint.detector.api.AnnotationInfo
+import com.android.tools.lint.detector.api.AnnotationOrigin
+import com.android.tools.lint.detector.api.AnnotationUsageInfo
 import com.android.tools.lint.detector.api.AnnotationUsageType
 import com.android.tools.lint.detector.api.Category
 import com.android.tools.lint.detector.api.Detector
@@ -24,27 +27,24 @@ import com.android.tools.lint.detector.api.Scope
 import com.android.tools.lint.detector.api.Severity
 import com.android.tools.lint.detector.api.SourceCodeScanner
 import com.intellij.psi.PsiMethod
-import org.jetbrains.uast.UAnnotation
 import org.jetbrains.uast.UCallExpression
 import org.jetbrains.uast.UElement
 
+@Suppress("DetectorIsMissingAnnotations")
 class DeferredApiDetector : Detector(), SourceCodeScanner {
   override fun applicableAnnotations(): List<String> = listOf(ANNOTATION)
 
   override fun visitAnnotationUsage(
     context: JavaContext,
-    usage: UElement,
-    type: AnnotationUsageType,
-    annotation: UAnnotation,
-    qualifiedName: String,
-    method: PsiMethod?,
-    annotations: List<UAnnotation>,
-    allMemberAnnotations: List<UAnnotation>,
-    allClassAnnotations: List<UAnnotation>,
-    allPackageAnnotations: List<UAnnotation>
+    element: UElement,
+    annotationInfo: AnnotationInfo,
+    usageInfo: AnnotationUsageInfo
   ) {
-    if (method != null && type == AnnotationUsageType.METHOD_CALL) {
-      check(context, usage as UCallExpression, method)
+    if (
+      usageInfo.type == AnnotationUsageType.METHOD_CALL &&
+        annotationInfo.origin == AnnotationOrigin.METHOD
+    ) {
+      check(context, usageInfo.usage as UCallExpression, annotationInfo.annotated as PsiMethod)
     }
   }
 
@@ -59,7 +59,7 @@ class DeferredApiDetector : Detector(), SourceCodeScanner {
         INVALID_DEFERRED_API_USE,
         usage,
         context.getCallLocation(usage, includeReceiver = false, includeArguments = true),
-        "${method.name} is only safe to call in the context of a Deferred<T> dependency."
+        "${method.name} is only safe to call in the context of a Deferred`<T>` dependency"
       )
   }
 
@@ -75,9 +75,9 @@ class DeferredApiDetector : Detector(), SourceCodeScanner {
         briefDescription = "Invalid use of @DeferredApi",
         explanation =
           """
-                    Ensures that a method which expects to be called in the context of
-                Deferred#whenAvailable(), is actually called this way. This is important for
-                supporting dynamically loaded modules, where certain dependencies become available
+                Ensures that a method which expects to be called in the context of \
+                `Deferred#whenAvailable()`, is actually called this way. This is important for \
+                supporting dynamically loaded modules, where certain dependencies become available \
                 during app's runtime and not available upon app launch.
                 """,
         category = Category.CORRECTNESS,

tools/lint/src/main/kotlin/KotlinInteropDetector.kt

@@ -52,6 +52,7 @@ import org.jetbrains.uast.getContainingUClass
 import org.jetbrains.uast.getContainingUMethod
 import org.jetbrains.uast.getParentOfType
 
+@Suppress("DetectorIsMissingAnnotations")
 class KotlinInteropDetector : Detector(), SourceCodeScanner {
   companion object Issues {
     private val IMPLEMENTATION =
@@ -66,13 +67,13 @@ class KotlinInteropDetector : Detector(), SourceCodeScanner {
         briefDescription = "No Hard Kotlin Keywords",
         explanation =
           """
-            Do not use Kotlin’s hard keywords as the name of methods or fields.
-            These require the use of backticks to escape when calling from Kotlin.
+            Do not use Kotlin’s hard keywords as the name of methods or fields. \
+            These require the use of backticks to escape when calling from Kotlin. \
             Soft keywords, modifier keywords, and special identifiers are allowed.
 
             For example, Mockito’s `when` function requires backticks when used from Kotlin:
 
-                val callable = Mockito.mock(Callable::class.java)
+                val callable = Mockito.mock(Callable::class.java) \
                 Mockito.\`when\`(callable.call()).thenReturn(/* … */)
             """,
         category = Category.INTEROPERABILITY_KOTLIN,
@@ -88,7 +89,7 @@ class KotlinInteropDetector : Detector(), SourceCodeScanner {
         briefDescription = "Lambda Parameters Last",
         explanation =
           """
-            To improve calling this code from Kotlin,
+            To improve calling this code from Kotlin, \
             parameter types eligible for SAM conversion should be last.
             """,
         category = Category.INTEROPERABILITY_KOTLIN,
@@ -104,7 +105,7 @@ class KotlinInteropDetector : Detector(), SourceCodeScanner {
         briefDescription = "Unknown nullness",
         explanation =
           """
-            To improve referencing this code from Kotlin, consider adding
+            To improve referencing this code from Kotlin, consider adding \
             explicit nullness information here with either `@NonNull` or `@Nullable`.
             """,
         category = Category.INTEROPERABILITY_KOTLIN,
@@ -312,7 +313,7 @@ class KotlinInteropDetector : Detector(), SourceCodeScanner {
 
       if (getter == null) {
         // Look for inherited methods
-        cls.superClass?.let { superClass ->
+        cls.javaPsi.superClass?.let { superClass ->
           for (inherited in superClass.findMethodsByName(getterName1, true)) {
             if (inherited.parameterList.parametersCount == 0) {
               getter = inherited
@@ -435,7 +436,7 @@ class KotlinInteropDetector : Detector(), SourceCodeScanner {
       ) {
         val name1 = badGetter!!.name
         if (name1.startsWith("is") && methodName.startsWith("setIs") && name1[2].isUpperCase()) {
-          val newProperty = name1[2].toLowerCase() + name1.substring(3)
+          val newProperty = name1[2].lowercaseChar() + name1.substring(3)
           val message =
             "This method should be called `set${newProperty.capitalize()}` such " +
               "that (along with the `$name1` getter) Kotlin code can access it " +
@@ -463,6 +464,10 @@ class KotlinInteropDetector : Detector(), SourceCodeScanner {
       }
     }
 
+    private fun String.capitalize() = replaceFirstChar { it.uppercase() }
+
+    private fun String.decapitalize() = replaceFirstChar { it.lowercase() }
+
     /** Returns true if the given class has a (possibly inherited) setter of the given type */
     private fun hasSetter(cls: UClass, type: PsiType?, setterName: String): Boolean {
       for (method in cls.findMethodsByName(setterName, true)) {
@@ -570,7 +575,7 @@ class KotlinInteropDetector : Detector(), SourceCodeScanner {
       val replaceLocation =
         if (node is UParameter) {
           location
-        } else if (node is UMethod && node.modifierList != null) {
+        } else if (node is UMethod) {
           // Place the insertion point at the modifiers such that we don't
           // insert the annotation for example after the "public" keyword.
           // We also don't want to place it on the method range itself since

tools/lint/src/main/kotlin/ManifestElementHasNoExportedAttributeDetector.kt

@@ -25,6 +25,7 @@ import com.android.tools.lint.detector.api.XmlContext
 import com.android.tools.lint.detector.api.XmlScanner
 import org.w3c.dom.Element
 
+@Suppress("DetectorIsMissingAnnotations")
 class ManifestElementHasNoExportedAttributeDetector : Detector(), XmlScanner {
 
   enum class Component(val xmlName: String) {

tools/lint/src/main/kotlin/NonAndroidxNullabilityDetector.kt

@@ -43,6 +43,7 @@ private val ANDROIDX_ANNOTATIONS =
     "android.support.annotation.NonNull"
   )
 
+@Suppress("DetectorIsMissingAnnotations")
 class NonAndroidxNullabilityDetector : Detector(), SourceCodeScanner {
   companion object Issues {
     private val IMPLEMENTATION =
@@ -52,7 +53,7 @@ class NonAndroidxNullabilityDetector : Detector(), SourceCodeScanner {
     val NON_ANDROIDX_NULLABILITY =
       Issue.create(
         id = "FirebaseNonAndroidxNullability",
-        briefDescription = "Use androidx nullability annotations.",
+        briefDescription = "Use androidx nullability annotations",
         explanation = "Use androidx nullability annotations instead.",
         category = Category.COMPLIANCE,
         priority = 1,
@@ -124,7 +125,7 @@ class NonAndroidxNullabilityDetector : Detector(), SourceCodeScanner {
       context.report(
         NON_ANDROIDX_NULLABILITY,
         context.getLocation(annotation),
-        "Use androidx nullability annotations.",
+        "Use androidx nullability annotations",
         fix
       )
     }

tools/lint/src/main/kotlin/ProviderAssignmentDetector.kt

@@ -25,32 +25,34 @@ import com.android.tools.lint.detector.api.SourceCodeScanner
 import com.intellij.psi.PsiClass
 import com.intellij.psi.PsiField
 import com.intellij.psi.PsiMethod
+import org.jetbrains.uast.UBinaryExpression
 import org.jetbrains.uast.UCallExpression
 import org.jetbrains.uast.UReferenceExpression
 import org.jetbrains.uast.getParentOfType
-import org.jetbrains.uast.java.JavaUAssignmentExpression
 
 private const val PROVIDER = "com.google.firebase.inject.Provider"
 
+@Suppress("DetectorIsMissingAnnotations")
 class ProviderAssignmentDetector : Detector(), SourceCodeScanner {
   override fun getApplicableMethodNames() = listOf("get")
 
   override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
     if (!isProviderGet(method)) {
       return
     }
-    val assignmentExpression =
-      node.getParentOfType<JavaUAssignmentExpression>(JavaUAssignmentExpression::class.java, true)
-    val assignmentTarget = assignmentExpression?.leftOperand as? UReferenceExpression ?: return
+    val binaryOperation = node.getParentOfType<UBinaryExpression>(true) ?: return
+    if (binaryOperation.operatorIdentifier?.name != "=") return
+
+    val assignmentTarget = binaryOperation.leftOperand as? UReferenceExpression ?: return
 
     // This would only be true if assigning the result of get(),
     // in cases like foo = p.get().someMethod() there would be an intermediate parent
     // and we don't want to trigger in such cases.
-    if (assignmentExpression != node.uastParent?.uastParent) {
+    if (binaryOperation != node.uastParent?.uastParent) {
       return
     }
 
-    if (hasDeferredApiAnnotation(context, assignmentExpression)) {
+    if (hasDeferredApiAnnotation(context, binaryOperation)) {
       return
     }
 
@@ -59,7 +61,7 @@ class ProviderAssignmentDetector : Detector(), SourceCodeScanner {
         context.report(
           INVALID_PROVIDER_ASSIGNMENT,
           context.getCallLocation(node, includeReceiver = false, includeArguments = true),
-          "Provider.get() assignment to a field detected."
+          "`Provider.get()` assignment to a field detected"
         )
       }
     }
@@ -87,9 +89,9 @@ class ProviderAssignmentDetector : Detector(), SourceCodeScanner {
         briefDescription = "Invalid use of Provider<T>",
         explanation =
           """
-                    Ensures that results of Provider.get() are not stored in class fields. Doing
-                    so may lead to bugs in the context of dynamic feature loading. Namely, optional
-                    provider dependencies can become available during the execution of the app, so
+                    Ensures that results of `Provider.get()` are not stored in class fields. Doing \
+                    so may lead to bugs in the context of dynamic feature loading. Namely, optional \
+                    provider dependencies can become available during the execution of the app, so \
                     dependents must be ready to handle this situation.
                 """,
         category = Category.CORRECTNESS,

tools/lint/src/main/kotlin/ThreadPoolDetector.kt

@@ -12,6 +12,7 @@ import com.intellij.psi.PsiClass
 import com.intellij.psi.PsiMethod
 import org.jetbrains.uast.UCallExpression
 
+@Suppress("DetectorIsMissingAnnotations")
 class ThreadPoolDetector : Detector(), SourceCodeScanner {
   override fun getApplicableMethodNames(): List<String> =
     listOf(
@@ -20,7 +21,8 @@ class ThreadPoolDetector : Detector(), SourceCodeScanner {
       "newScheduledThreadPool",
       "newSingleThreadExecutor",
       "newSingleThreadScheduledExecutor",
-      "newWorkStealingPool"
+      "newWorkStealingPool",
+      "factory"
     )
 
   override fun getApplicableConstructorTypes(): List<String> =
@@ -32,14 +34,14 @@ class ThreadPoolDetector : Detector(), SourceCodeScanner {
     )
 
   override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
-    if (!isExecutorMethod(method)) {
+    if (!isExecutorMethod(method) && !isPoolableFactory(method)) {
       return
     }
 
     context.report(
       THREAD_POOL_CREATION,
       context.getCallLocation(node, includeReceiver = false, includeArguments = true),
-      "Creating thread pools is not allowed."
+      "Creating thread pools is not allowed"
     )
   }
 
@@ -51,7 +53,7 @@ class ThreadPoolDetector : Detector(), SourceCodeScanner {
     context.report(
       THREAD_POOL_CREATION,
       context.getCallLocation(node, includeReceiver = false, includeArguments = true),
-      "Creating threads or thread pools is not allowed."
+      "Creating threads or thread pools is not allowed"
     )
   }
 
@@ -62,6 +64,14 @@ class ThreadPoolDetector : Detector(), SourceCodeScanner {
     return false
   }
 
+  private fun isPoolableFactory(method: PsiMethod): Boolean {
+    if (method.name != "factory") return false
+    (method.parent as? PsiClass)?.let {
+      return it.name == "PoolableExecutors"
+    }
+    return false
+  }
+
   companion object {
     private val IMPLEMENTATION =
       Implementation(ThreadPoolDetector::class.java, Scope.JAVA_FILE_SCOPE)
@@ -71,10 +81,12 @@ class ThreadPoolDetector : Detector(), SourceCodeScanner {
     val THREAD_POOL_CREATION =
       Issue.create(
         id = "ThreadPoolCreation",
-        briefDescription = "Creating thread pools is not allowed.",
+        briefDescription = "Creating thread pools is not allowed",
         explanation =
           """
-                    Please use one of the executors provided by firebase-common
+                    Please use one of the executors provided by firebase-common.
+
+                    See: https://github.com/firebase/firebase-android-sdk/blob/master/docs/executors.md
                 """,
         category = Category.CORRECTNESS,
         priority = 6,