From 2fbe7bd8fc3880cd74b64c45764f192bca3345a4 Mon Sep 17 00:00:00 2001 From: Vladimir Kryachko Date: Wed, 23 Nov 2022 11:26:44 -0500 Subject: [PATCH] Fix warnings in the linter. --- .../firebase/messaging/FcmExecutors.java | 2 ++ tools/lint/src/main/kotlin/CheckRegistry.kt | 9 ++++++ .../src/main/kotlin/DeferredApiDetector.kt | 32 +++++++++---------- .../src/main/kotlin/KotlinInteropDetector.kt | 21 +++++++----- ...stElementHasNoExportedAttributeDetector.kt | 1 + .../kotlin/NonAndroidxNullabilityDetector.kt | 5 +-- .../main/kotlin/ProviderAssignmentDetector.kt | 22 +++++++------ .../src/main/kotlin/ThreadPoolDetector.kt | 24 ++++++++++---- 8 files changed, 74 insertions(+), 42 deletions(-) diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/FcmExecutors.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/FcmExecutors.java index 385af046e99..0f1bf92f062 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/FcmExecutors.java +++ b/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( diff --git a/tools/lint/src/main/kotlin/CheckRegistry.kt b/tools/lint/src/main/kotlin/CheckRegistry.kt index 4283340ecb8..b864caf3729 100644 --- a/tools/lint/src/main/kotlin/CheckRegistry.kt +++ b/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" + ) } diff --git a/tools/lint/src/main/kotlin/DeferredApiDetector.kt b/tools/lint/src/main/kotlin/DeferredApiDetector.kt index a78677e7a28..ac9746f9f16 100644 --- a/tools/lint/src/main/kotlin/DeferredApiDetector.kt +++ b/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 = listOf(ANNOTATION) override fun visitAnnotationUsage( context: JavaContext, - usage: UElement, - type: AnnotationUsageType, - annotation: UAnnotation, - qualifiedName: String, - method: PsiMethod?, - annotations: List, - allMemberAnnotations: List, - allClassAnnotations: List, - allPackageAnnotations: List + 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 dependency." + "${method.name} is only safe to call in the context of a Deferred`` 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, diff --git a/tools/lint/src/main/kotlin/KotlinInteropDetector.kt b/tools/lint/src/main/kotlin/KotlinInteropDetector.kt index 0d2ee22a565..a9d475d8785 100644 --- a/tools/lint/src/main/kotlin/KotlinInteropDetector.kt +++ b/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 diff --git a/tools/lint/src/main/kotlin/ManifestElementHasNoExportedAttributeDetector.kt b/tools/lint/src/main/kotlin/ManifestElementHasNoExportedAttributeDetector.kt index 26538bf110d..71b5b1f5c4d 100644 --- a/tools/lint/src/main/kotlin/ManifestElementHasNoExportedAttributeDetector.kt +++ b/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) { diff --git a/tools/lint/src/main/kotlin/NonAndroidxNullabilityDetector.kt b/tools/lint/src/main/kotlin/NonAndroidxNullabilityDetector.kt index e40139f8566..2d351c7d05c 100644 --- a/tools/lint/src/main/kotlin/NonAndroidxNullabilityDetector.kt +++ b/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 ) } diff --git a/tools/lint/src/main/kotlin/ProviderAssignmentDetector.kt b/tools/lint/src/main/kotlin/ProviderAssignmentDetector.kt index 6c4af3dcb52..2840bd78933 100644 --- a/tools/lint/src/main/kotlin/ProviderAssignmentDetector.kt +++ b/tools/lint/src/main/kotlin/ProviderAssignmentDetector.kt @@ -25,13 +25,14 @@ 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") @@ -39,18 +40,19 @@ class ProviderAssignmentDetector : Detector(), SourceCodeScanner { if (!isProviderGet(method)) { return } - val assignmentExpression = - node.getParentOfType(JavaUAssignmentExpression::class.java, true) - val assignmentTarget = assignmentExpression?.leftOperand as? UReferenceExpression ?: return + val binaryOperation = node.getParentOfType(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", 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, diff --git a/tools/lint/src/main/kotlin/ThreadPoolDetector.kt b/tools/lint/src/main/kotlin/ThreadPoolDetector.kt index c1122a1b15b..3be937f886e 100644 --- a/tools/lint/src/main/kotlin/ThreadPoolDetector.kt +++ b/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 = listOf( @@ -20,7 +21,8 @@ class ThreadPoolDetector : Detector(), SourceCodeScanner { "newScheduledThreadPool", "newSingleThreadExecutor", "newSingleThreadScheduledExecutor", - "newWorkStealingPool" + "newWorkStealingPool", + "factory" ) override fun getApplicableConstructorTypes(): List = @@ -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,