From da7c879d56a20f3385023dc4b72fc8bf0e9011f8 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Tue, 8 Aug 2023 12:39:48 +1000 Subject: [PATCH 01/16] Add missing null annotation detector This adds a lint rule to inform when we are missing null annotations in Java code. It currently only informs for fields, and requires Android null annotations (`@NonNull`, and `@Nullable`). --- .../lint/MissingNullAnnotationDetector.kt | 122 ++++++++++++++++++ .../android/lint/WordPressIssueRegistry.kt | 6 +- .../lint/MissingNullAnnotationDetectorTest.kt | 45 +++++++ 3 files changed, 168 insertions(+), 5 deletions(-) create mode 100644 WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt create mode 100644 WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt new file mode 100644 index 0000000..4eb68b3 --- /dev/null +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -0,0 +1,122 @@ +package org.wordpress.android.lint + +import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.Category.Companion.CORRECTNESS +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.Companion.JAVA_FILE_SCOPE +import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.SourceCodeScanner +import com.android.tools.lint.detector.api.isJava +import com.intellij.psi.PsiPrimitiveType +import org.jetbrains.uast.UAnnotated +import org.jetbrains.uast.UElement +import org.jetbrains.uast.UEnumConstant +import org.jetbrains.uast.UField +import org.jetbrains.uast.UMethod + +class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { + override fun getApplicableUastTypes(): List> = listOf( + UField::class.java, +// UMethod::class.java, + ) + + override fun createUastHandler(context: JavaContext): UElementHandler? = + if (isJava(context.uastFile?.sourcePsi)) + object: UElementHandler() { + override fun visitField(node: UField) { + if (!node.isAutoSuppressed && !node.isNullAnnotated) { + context.report( + MISSING_FIELD_ANNOTATION, + node, + context.getLocation(node), + "Missing null annotation", + node.fixes, + ) + } + } + } + else + null + + companion object { + val MISSING_FIELD_ANNOTATION = Issue.create( + id = "MissingNullAnnotationOnField", + briefDescription = "Nullable/NonNull annotation missing on field", + explanation = "Checks for missing `@NonNull/@Nullable` annotations for fields.", + category = CORRECTNESS, + priority = 5, + severity = Severity.INFORMATIONAL, + implementation = Implementation( + MissingNullAnnotationDetector::class.java, + JAVA_FILE_SCOPE, + ) + ) + + @Suppress("ForbiddenComment") + // TODO: Create a LintFix to normalize our usage to Android's annotations. This can be + // useful for running Nullability analysis. See: + // https://developer.android.com/studio/write/annotations#nullability-analysis + private val acceptableNullAnnotations = listOf( + "androidx.annotation.NonNull", + "androidx.annotation.Nullable", + ) + + private val UField.isPrimitive get() = this.type is PsiPrimitiveType + private val UField.isEnum get() = this is UEnumConstant + private val UField.isConstant get() = this.isStatic && this.isFinal + private val UField.isInitializedFinalField get() = this.isFinal && this.uastInitializer != null + + /** We ignore missing null annotations for cases where this evaluates to true. */ + private val UField.isAutoSuppressed get() = + this.isPrimitive + || this.isEnum + || this.isConstant + || this.isInitializedFinalField + || this.annotations.any { annotation -> + annotation.qualifiedName?.endsWith("Inject") ?: false + } + + + private val UAnnotated.isNullAnnotated get() = this.uAnnotations.any { annotation -> + acceptableNullAnnotations.any { nullAnnotation -> + annotation.qualifiedName == nullAnnotation + } + } + + private val UElement.fixes get() = this.asSourceString().let { sourceString -> + val nullableReplacement = "@Nullable $sourceString" + val nonNullReplacement = "@NonNull $sourceString" + + LintFix.create().group() + .add( + LintFix.create() + .name("Annotate as @Nullable") + .replace() + .text(sourceString) + .shortenNames() + .reformat(true) + .with(nullableReplacement) + .build() + ) + .add( + LintFix.create() + .name("Annotate as @NonNull") + .replace() + .text(sourceString) + .shortenNames() + .reformat(true) + .with(nonNullReplacement) + .build() + ) + .build() + } + } +} + + + + diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt index 52a4951..1e04de3 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt @@ -3,16 +3,12 @@ package org.wordpress.android.lint 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 org.wordpress.android.lint.WordPressAndroidImportInViewModelDetector.Companion.ISSUE_ANDROID_IMPORT_IN_VIEWMODEL class WordPressIssueRegistry : IssueRegistry() { override val api = CURRENT_API override val minApi = MIN_API override val issues get() = listOf( - WordPressRtlCodeDetector.SET_PADDING, - WordPressRtlCodeDetector.SET_MARGIN, - WordPressRtlCodeDetector.GET_PADDING, - ISSUE_ANDROID_IMPORT_IN_VIEWMODEL, + MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION, ) override val vendor = Vendor( vendorName = "WordPress Android", diff --git a/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt new file mode 100644 index 0000000..ce54690 --- /dev/null +++ b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt @@ -0,0 +1,45 @@ +package org.wordpress.android.lint + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.checks.infrastructure.TestLintTask.lint +import org.junit.Test + + +class MissingNullAnnotationDetectorTest { + @Test + fun `it should inform when null annotation is missing on field`() { + lint().files(LintDetectorTest.java(""" + package test; + + class ExampleClass { + String mExampleField = "example"; + } + """).indented()) + .allowCompilationErrors() + .issues(MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION) + .run() + .expect(""" + src/test/ExampleClass.java:4: Information: Missing null annotation [MissingNullAnnotationOnField] + String mExampleField = "example"; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 0 errors, 0 warnings + """ + .trimIndent() + ) + + } + @Test + fun `it ignores injections`() { + lint().files(LintDetectorTest.java(""" + package test; + + class ExampleClass { + @Inject String mExampleField = "example"; + } + """).indented()) + .allowCompilationErrors() + .issues(MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION) + .run() + .expectClean() + } +} From 754ee89f0d784235a11f5b301275c133ab454594 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Tue, 8 Aug 2023 17:34:21 +1000 Subject: [PATCH 02/16] Add missing null annotation detector implementation for methods This implements missing null annotation detection for method parameters and method return types. Injected parameters are ignored for this rule. --- .../lint/MissingNullAnnotationDetector.kt | 183 +++++++++++------- .../android/lint/WordPressIssueRegistry.kt | 2 + .../lint/MissingNullAnnotationDetectorTest.kt | 66 ++++++- 3 files changed, 176 insertions(+), 75 deletions(-) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt index 4eb68b3..2637344 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -13,110 +13,145 @@ import com.android.tools.lint.detector.api.SourceCodeScanner import com.android.tools.lint.detector.api.isJava import com.intellij.psi.PsiPrimitiveType import org.jetbrains.uast.UAnnotated +import org.jetbrains.uast.UAnnotationMethod import org.jetbrains.uast.UElement import org.jetbrains.uast.UEnumConstant import org.jetbrains.uast.UField import org.jetbrains.uast.UMethod +import org.jetbrains.uast.UVariable class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { override fun getApplicableUastTypes(): List> = listOf( UField::class.java, -// UMethod::class.java, + UMethod::class.java, ) - override fun createUastHandler(context: JavaContext): UElementHandler? = - if (isJava(context.uastFile?.sourcePsi)) - object: UElementHandler() { - override fun visitField(node: UField) { - if (!node.isAutoSuppressed && !node.isNullAnnotated) { - context.report( - MISSING_FIELD_ANNOTATION, - node, - context.getLocation(node), - "Missing null annotation", - node.fixes, - ) - } - } + override fun createUastHandler(context: JavaContext): UElementHandler? = with(context) { + if (!isJava(uastFile?.sourcePsi)) { + return null + } + object: UElementHandler() { + override fun visitField(node: UField) { + if (node.requiresNullAnnotation && !node.isNullAnnotated) { + report(node, MISSING_FIELD_ANNOTATION) } - else - null + } + override fun visitMethod(node: UMethod) { + if (node.requiresNullAnnotation && !node.isNullAnnotated) { + report(node, MISSING_METHOD_RETURN_TYPE_ANNOTATION) + } + + node.uastParameters.forEach { parameter -> + if (parameter.requiresNullAnnotation && !parameter.isNullAnnotated) { + report(parameter, MISSING_METHOD_PARAMETER_ANNOTATION) + } + } + } + } + } companion object { val MISSING_FIELD_ANNOTATION = Issue.create( id = "MissingNullAnnotationOnField", briefDescription = "Nullable/NonNull annotation missing on field", explanation = "Checks for missing `@NonNull/@Nullable` annotations for fields.", - category = CORRECTNESS, - priority = 5, - severity = Severity.INFORMATIONAL, - implementation = Implementation( - MissingNullAnnotationDetector::class.java, - JAVA_FILE_SCOPE, - ) + ) + val MISSING_METHOD_RETURN_TYPE_ANNOTATION = Issue.create( + id = "MissingNullAnnotationOnMethodReturnType", + briefDescription = "Nullable/NonNull annotation missing on method return type", + explanation = "Checks for missing `@NonNull/@Nullable` annotations on method return types.", + ) + val MISSING_METHOD_PARAMETER_ANNOTATION = Issue.create( + id = "MissingNullAnnotationOnMethodParameter", + briefDescription = "Nullable/NonNull annotation missing on method parameter", + explanation = "Checks for missing `@NonNull/@Nullable` annotations on method parameters.", ) @Suppress("ForbiddenComment") // TODO: Create a LintFix to normalize our usage to Android's annotations. This can be // useful for running Nullability analysis. See: // https://developer.android.com/studio/write/annotations#nullability-analysis - private val acceptableNullAnnotations = listOf( + val acceptableNullAnnotations = listOf( "androidx.annotation.NonNull", "androidx.annotation.Nullable", ) + } +} - private val UField.isPrimitive get() = this.type is PsiPrimitiveType - private val UField.isEnum get() = this is UEnumConstant - private val UField.isConstant get() = this.isStatic && this.isFinal - private val UField.isInitializedFinalField get() = this.isFinal && this.uastInitializer != null - - /** We ignore missing null annotations for cases where this evaluates to true. */ - private val UField.isAutoSuppressed get() = - this.isPrimitive - || this.isEnum - || this.isConstant - || this.isInitializedFinalField - || this.annotations.any { annotation -> - annotation.qualifiedName?.endsWith("Inject") ?: false - } - - - private val UAnnotated.isNullAnnotated get() = this.uAnnotations.any { annotation -> - acceptableNullAnnotations.any { nullAnnotation -> - annotation.qualifiedName == nullAnnotation - } - } - - private val UElement.fixes get() = this.asSourceString().let { sourceString -> - val nullableReplacement = "@Nullable $sourceString" - val nonNullReplacement = "@NonNull $sourceString" +private val UVariable.isPrimitive get() = this.type is PsiPrimitiveType +private val UVariable.isEnum get() = this is UEnumConstant +private val UVariable.isInjected get() = this.annotations.any { annotation -> + annotation.qualifiedName?.endsWith("Inject") ?: false +} +private val UVariable.isConstant get() = this.isStatic && this.isFinal +private val UVariable.isInitializedFinalField get() = this.isFinal + && this.uastInitializer != null +private val UVariable.requiresNullAnnotation get() = + !this.isPrimitive + && !this.isEnum + && !this.isConstant + && !this.isInitializedFinalField + && !this.isInjected +private val UMethod.isPrimitive get() = this.returnType is PsiPrimitiveType +private val UMethod.requiresNullAnnotation get() = + this !is UAnnotationMethod + && !this.isPrimitive + && !this.isConstructor - LintFix.create().group() - .add( - LintFix.create() - .name("Annotate as @Nullable") - .replace() - .text(sourceString) - .shortenNames() - .reformat(true) - .with(nullableReplacement) - .build() - ) - .add( - LintFix.create() - .name("Annotate as @NonNull") - .replace() - .text(sourceString) - .shortenNames() - .reformat(true) - .with(nonNullReplacement) - .build() - ) - .build() - } +private val UAnnotated.isNullAnnotated get() = this.uAnnotations.any { annotation -> + MissingNullAnnotationDetector.acceptableNullAnnotations.any { nullAnnotation -> + annotation.qualifiedName == nullAnnotation } } +private fun Issue.Companion.create( + id: String, + briefDescription: String, + explanation: String, +) = create( + id = id, + briefDescription = briefDescription, + explanation = explanation, + category = CORRECTNESS, + priority = 5, + severity = Severity.INFORMATIONAL, + implementation = Implementation( + MissingNullAnnotationDetector::class.java, + JAVA_FILE_SCOPE, + ) +) +private fun JavaContext.report(node: UElement, issue: Issue) = report( + issue, + node, + getLocation(node), + "Missing null annotation", + node.fixes, +) +private val UElement.fixes get() = this.asSourceString().let { sourceString -> + val nullableReplacement = "@Nullable $sourceString" + val nonNullReplacement = "@NonNull $sourceString" - + LintFix.create().group() + .add( + LintFix.create() + .name("Annotate as @Nullable") + .replace() + .text(sourceString) + .shortenNames() + .reformat(true) + .with(nullableReplacement) + .build() + ) + .add( + LintFix.create() + .name("Annotate as @NonNull") + .replace() + .text(sourceString) + .shortenNames() + .reformat(true) + .with(nonNullReplacement) + .build() + ) + .build() +} \ No newline at end of file diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt index 1e04de3..8016f10 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt @@ -9,6 +9,8 @@ class WordPressIssueRegistry : IssueRegistry() { override val minApi = MIN_API override val issues get() = listOf( MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION, + MissingNullAnnotationDetector.MISSING_METHOD_RETURN_TYPE_ANNOTATION, + MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION, ) override val vendor = Vendor( vendorName = "WordPress Android", diff --git a/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt index ce54690..bbb8a14 100644 --- a/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt +++ b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt @@ -29,7 +29,7 @@ class MissingNullAnnotationDetectorTest { } @Test - fun `it ignores injections`() { + fun `it ignores injected fields`() { lint().files(LintDetectorTest.java(""" package test; @@ -42,4 +42,68 @@ class MissingNullAnnotationDetectorTest { .run() .expectClean() } + + @Test + fun `it should inform when null annotation is missing on a method return type`() { + lint().files(LintDetectorTest.java(""" + package test; + + class ExampleClass { + String getMessage() { + return "example"; + } + } + """).indented()) + .allowCompilationErrors() + .issues(MissingNullAnnotationDetector.MISSING_METHOD_RETURN_TYPE_ANNOTATION) + .run() + .expect(""" + src/test/ExampleClass.java:4: Information: Missing null annotation [MissingNullAnnotationOnMethodReturnType] + String getMessage() { + ~~~~~~~~~~ + 0 errors, 0 warnings + """ + .trimIndent() + ) + } + + @Test + fun `it should inform when null annotation is missing on a method parameter`() { + lint().files(LintDetectorTest.java(""" + package test; + + class ExampleClass { + String getMessage(String name) { + return name + " example"; + } + } + """).indented()) + .allowCompilationErrors() + .issues(MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION) + .run() + .expect(""" + src/test/ExampleClass.java:4: Information: Missing null annotation [MissingNullAnnotationOnMethodParameter] + String getMessage(String name) { + ~~~~~~~~~~~ + 0 errors, 0 warnings + """ + .trimIndent() + ) + } + @Test + fun `it ignores injected parameters`() { + lint().files(LintDetectorTest.java(""" + package test; + + class ExampleClass { + String getMessage(@Inject String name) { + return name + " example"; + } + } + """).indented()) + .allowCompilationErrors() + .issues(MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION) + .run() + .expectClean() + } } From dae2dacae711082d64730345b825d23fed8cc270 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 9 Aug 2023 10:10:11 +1000 Subject: [PATCH 03/16] Add comment labels to groups of extension functions --- .../android/lint/MissingNullAnnotationDetector.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt index 2637344..90e6734 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -78,6 +78,7 @@ class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { } } +/* UVariable Extensions */ private val UVariable.isPrimitive get() = this.type is PsiPrimitiveType private val UVariable.isEnum get() = this is UEnumConstant private val UVariable.isInjected get() = this.annotations.any { annotation -> @@ -92,18 +93,22 @@ private val UVariable.requiresNullAnnotation get() = && !this.isConstant && !this.isInitializedFinalField && !this.isInjected + +/* UMethod Extensions */ private val UMethod.isPrimitive get() = this.returnType is PsiPrimitiveType private val UMethod.requiresNullAnnotation get() = this !is UAnnotationMethod && !this.isPrimitive && !this.isConstructor +/* UAnnotated Extensions */ private val UAnnotated.isNullAnnotated get() = this.uAnnotations.any { annotation -> MissingNullAnnotationDetector.acceptableNullAnnotations.any { nullAnnotation -> annotation.qualifiedName == nullAnnotation } } +/* Issue.Companion Extensions */ private fun Issue.Companion.create( id: String, briefDescription: String, @@ -121,6 +126,7 @@ private fun Issue.Companion.create( ) ) +/* JavaContext Extensions */ private fun JavaContext.report(node: UElement, issue: Issue) = report( issue, node, @@ -128,6 +134,8 @@ private fun JavaContext.report(node: UElement, issue: Issue) = report( "Missing null annotation", node.fixes, ) + +/* UElement Extensions */ private val UElement.fixes get() = this.asSourceString().let { sourceString -> val nullableReplacement = "@Nullable $sourceString" val nonNullReplacement = "@NonNull $sourceString" From a5fcb442afc54362dd39ef1ae3a623893e9e60f6 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 9 Aug 2023 10:17:00 +1000 Subject: [PATCH 04/16] Fix and add tests for annotations on constructor parameters --- .../lint/MissingNullAnnotationDetectorTest.kt | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt index bbb8a14..715225b 100644 --- a/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt +++ b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt @@ -91,14 +91,33 @@ class MissingNullAnnotationDetectorTest { ) } @Test - fun `it ignores injected parameters`() { + fun `it should inform when null annotation is missing on a construction parameter`() { lint().files(LintDetectorTest.java(""" package test; class ExampleClass { - String getMessage(@Inject String name) { - return name + " example"; - } + ExampleClass(String name) {} + } + """).indented()) + .allowCompilationErrors() + .issues(MissingNullAnnotationDetector.MISSING_CONSTRUCTOR_PARAMETER_ANNOTATION) + .run() + .expect(""" + src/test/ExampleClass.java:4: Information: Missing null annotation [MissingNullAnnotationOnConstructorParameter] + ExampleClass(String name) {} + ~~~~~~~~~~~ + 0 errors, 0 warnings + """ + .trimIndent() + ) + } + @Test + fun `it ignores injected constructors`() { + lint().files(LintDetectorTest.java(""" + package test; + + class ExampleClass { + @Inject ExampleClass(String name) {} } """).indented()) .allowCompilationErrors() From 7184199981de98f6716dd887166e326dccd7ae78 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 9 Aug 2023 10:17:45 +1000 Subject: [PATCH 05/16] Fix implementation for construction parameter issue --- .../lint/MissingNullAnnotationDetector.kt | 25 +++++++++++++------ .../android/lint/WordPressIssueRegistry.kt | 3 ++- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt index 90e6734..7cf977e 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -18,6 +18,7 @@ import org.jetbrains.uast.UElement import org.jetbrains.uast.UEnumConstant import org.jetbrains.uast.UField import org.jetbrains.uast.UMethod +import org.jetbrains.uast.UParameter import org.jetbrains.uast.UVariable class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { @@ -38,14 +39,19 @@ class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { } override fun visitMethod(node: UMethod) { + node.uastParameters.forEach { visitParameter(node, it) } + if (node.requiresNullAnnotation && !node.isNullAnnotated) { report(node, MISSING_METHOD_RETURN_TYPE_ANNOTATION) } + } - node.uastParameters.forEach { parameter -> - if (parameter.requiresNullAnnotation && !parameter.isNullAnnotated) { + private fun visitParameter(node: UMethod, parameter: UParameter) { + if (parameter.requiresNullAnnotation && !parameter.isNullAnnotated) { + if (node.isConstructor) + report(parameter, MISSING_CONSTRUCTOR_PARAMETER_ANNOTATION) + else report(parameter, MISSING_METHOD_PARAMETER_ANNOTATION) - } } } } @@ -56,16 +62,21 @@ class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { briefDescription = "Nullable/NonNull annotation missing on field", explanation = "Checks for missing `@NonNull/@Nullable` annotations for fields.", ) - val MISSING_METHOD_RETURN_TYPE_ANNOTATION = Issue.create( - id = "MissingNullAnnotationOnMethodReturnType", - briefDescription = "Nullable/NonNull annotation missing on method return type", - explanation = "Checks for missing `@NonNull/@Nullable` annotations on method return types.", + val MISSING_CONSTRUCTOR_PARAMETER_ANNOTATION = Issue.create( + id = "MissingNullAnnotationOnConstructorParameter", + briefDescription = "Nullable/NonNull annotation missing on constructor parameter", + explanation = "Checks for missing `@NonNull/@Nullable` annotations on constructor parameters.", ) val MISSING_METHOD_PARAMETER_ANNOTATION = Issue.create( id = "MissingNullAnnotationOnMethodParameter", briefDescription = "Nullable/NonNull annotation missing on method parameter", explanation = "Checks for missing `@NonNull/@Nullable` annotations on method parameters.", ) + val MISSING_METHOD_RETURN_TYPE_ANNOTATION = Issue.create( + id = "MissingNullAnnotationOnMethodReturnType", + briefDescription = "Nullable/NonNull annotation missing on method return type", + explanation = "Checks for missing `@NonNull/@Nullable` annotations on method return types.", + ) @Suppress("ForbiddenComment") // TODO: Create a LintFix to normalize our usage to Android's annotations. This can be diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt index 8016f10..e095964 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt @@ -9,8 +9,9 @@ class WordPressIssueRegistry : IssueRegistry() { override val minApi = MIN_API override val issues get() = listOf( MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION, - MissingNullAnnotationDetector.MISSING_METHOD_RETURN_TYPE_ANNOTATION, + MissingNullAnnotationDetector.MISSING_CONSTRUCTOR_PARAMETER_ANNOTATION, MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION, + MissingNullAnnotationDetector.MISSING_METHOD_RETURN_TYPE_ANNOTATION, ) override val vendor = Vendor( vendorName = "WordPress Android", From 6aeb4fd440c73d74fe96f61a87cbcc722741747d Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 9 Aug 2023 10:20:17 +1000 Subject: [PATCH 06/16] Reformat registry code --- .../android/lint/WordPressIssueRegistry.kt | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt index e095964..4410c63 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt @@ -7,17 +7,20 @@ import com.android.tools.lint.detector.api.CURRENT_API class WordPressIssueRegistry : IssueRegistry() { override val api = CURRENT_API override val minApi = MIN_API - override val issues get() = listOf( - MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION, - MissingNullAnnotationDetector.MISSING_CONSTRUCTOR_PARAMETER_ANNOTATION, - MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION, - MissingNullAnnotationDetector.MISSING_METHOD_RETURN_TYPE_ANNOTATION, - ) + override val issues + get() = listOf( + MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION, + MissingNullAnnotationDetector.MISSING_CONSTRUCTOR_PARAMETER_ANNOTATION, + MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION, + MissingNullAnnotationDetector.MISSING_METHOD_RETURN_TYPE_ANNOTATION, + ) + override val vendor = Vendor( vendorName = "WordPress Android", feedbackUrl = "https://github.com/wordpress-mobile/WordPress-Lint-Android/issues", identifier = "org.wordpress.android.lint", ) + companion object { const val MIN_API = 10 } From 7295d2a315670cd0b1c27ed93ba9e6002f3da252 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 9 Aug 2023 14:11:05 +1000 Subject: [PATCH 07/16] Remove allowCompileErrors from tests --- .../android/lint/MissingNullAnnotationDetectorTest.kt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt index 715225b..cc13ee0 100644 --- a/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt +++ b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt @@ -15,7 +15,6 @@ class MissingNullAnnotationDetectorTest { String mExampleField = "example"; } """).indented()) - .allowCompilationErrors() .issues(MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION) .run() .expect(""" @@ -37,7 +36,6 @@ class MissingNullAnnotationDetectorTest { @Inject String mExampleField = "example"; } """).indented()) - .allowCompilationErrors() .issues(MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION) .run() .expectClean() @@ -54,7 +52,6 @@ class MissingNullAnnotationDetectorTest { } } """).indented()) - .allowCompilationErrors() .issues(MissingNullAnnotationDetector.MISSING_METHOD_RETURN_TYPE_ANNOTATION) .run() .expect(""" @@ -78,7 +75,6 @@ class MissingNullAnnotationDetectorTest { } } """).indented()) - .allowCompilationErrors() .issues(MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION) .run() .expect(""" @@ -99,7 +95,6 @@ class MissingNullAnnotationDetectorTest { ExampleClass(String name) {} } """).indented()) - .allowCompilationErrors() .issues(MissingNullAnnotationDetector.MISSING_CONSTRUCTOR_PARAMETER_ANNOTATION) .run() .expect(""" @@ -120,7 +115,6 @@ class MissingNullAnnotationDetectorTest { @Inject ExampleClass(String name) {} } """).indented()) - .allowCompilationErrors() .issues(MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION) .run() .expectClean() From c649e44d49121a2aed72bd5dcc01923c09a5659f Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 9 Aug 2023 15:41:21 +1000 Subject: [PATCH 08/16] Add null annotation test files --- .../java/org/wordpress/android/lint/Utils.kt | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 WordPressLint/src/test/java/org/wordpress/android/lint/Utils.kt diff --git a/WordPressLint/src/test/java/org/wordpress/android/lint/Utils.kt b/WordPressLint/src/test/java/org/wordpress/android/lint/Utils.kt new file mode 100644 index 0000000..9103749 --- /dev/null +++ b/WordPressLint/src/test/java/org/wordpress/android/lint/Utils.kt @@ -0,0 +1,125 @@ +package org.wordpress.android.lint + +import com.android.tools.lint.checks.infrastructure.TestFile +import com.android.tools.lint.checks.infrastructure.TestFiles + +/* The null annotations are copied here for usage in testing. This is necessary because without it, + the qualifiedName of the UAnnotation node does not report the fully annotated name. + */ + +object Utils { + val nullableClass: TestFile + get() = TestFiles.kotlin( + "src/androidx/annotation/Nullable.kt", + """ +/* + * Copyright (C) 2013 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 androidx.annotation + +import java.lang.annotation.ElementType.ANNOTATION_TYPE +import java.lang.annotation.ElementType.FIELD +import java.lang.annotation.ElementType.LOCAL_VARIABLE +import java.lang.annotation.ElementType.METHOD +import java.lang.annotation.ElementType.PACKAGE +import java.lang.annotation.ElementType.PARAMETER + +/** + * Denotes that a parameter, field or method return value can be null. + * + * When decorating a method call parameter, this denotes that the parameter can + * legitimately be null and the method will gracefully deal with it. Typically + * used on optional parameters. + * + * When decorating a method, this denotes the method might legitimately return + * null. + * + * This is a marker annotation and it has no specific attributes. + */ +@MustBeDocumented +@Retention(AnnotationRetention.BINARY) +@Target( + AnnotationTarget.FUNCTION, + AnnotationTarget.PROPERTY_GETTER, + AnnotationTarget.PROPERTY_SETTER, + AnnotationTarget.VALUE_PARAMETER, + AnnotationTarget.FIELD, + AnnotationTarget.LOCAL_VARIABLE, + AnnotationTarget.ANNOTATION_CLASS, + AnnotationTarget.FILE +) +// Needed due to Kotlin's lack of PACKAGE annotation target +// https://youtrack.jetbrains.com/issue/KT-45921 +@Suppress("DEPRECATED_JAVA_ANNOTATION") +@java.lang.annotation.Target(METHOD, PARAMETER, FIELD, LOCAL_VARIABLE, ANNOTATION_TYPE, PACKAGE) +public annotation class Nullable + """.trimIndent() + ) + + val nonNullClass: TestFile + get() = TestFiles.kotlin( + "src/androidx/annotation/NonNull.kt", + """ +/* + * Copyright (C) 2013 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 androidx.annotation + +import java.lang.annotation.ElementType.ANNOTATION_TYPE +import java.lang.annotation.ElementType.FIELD +import java.lang.annotation.ElementType.LOCAL_VARIABLE +import java.lang.annotation.ElementType.METHOD +import java.lang.annotation.ElementType.PACKAGE +import java.lang.annotation.ElementType.PARAMETER + +/** + * Denotes that a parameter, field or method return value can never be null. + * + * This is a marker annotation and it has no specific attributes. + */ +@MustBeDocumented +@Retention(AnnotationRetention.BINARY) +@Target( + AnnotationTarget.FUNCTION, + AnnotationTarget.PROPERTY_GETTER, + AnnotationTarget.PROPERTY_SETTER, + AnnotationTarget.VALUE_PARAMETER, + AnnotationTarget.FIELD, + AnnotationTarget.LOCAL_VARIABLE, + AnnotationTarget.ANNOTATION_CLASS, + AnnotationTarget.FILE +) +// Needed due to Kotlin's lack of PACKAGE annotation target +// https://youtrack.jetbrains.com/issue/KT-45921 +@Suppress("DEPRECATED_JAVA_ANNOTATION") +@java.lang.annotation.Target( + METHOD, PARAMETER, FIELD, LOCAL_VARIABLE, ANNOTATION_TYPE, PACKAGE +) +public annotation class NonNull + """.trimIndent() + ) +} From a12ad1161372975632a40f6d240d2581cccdb702 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 9 Aug 2023 15:41:44 +1000 Subject: [PATCH 09/16] Add tests to cover cases where null annotations are present --- .../lint/MissingNullAnnotationDetectorTest.kt | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt index cc13ee0..1bfab47 100644 --- a/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt +++ b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt @@ -3,6 +3,8 @@ package org.wordpress.android.lint import com.android.tools.lint.checks.infrastructure.LintDetectorTest import com.android.tools.lint.checks.infrastructure.TestLintTask.lint import org.junit.Test +import org.wordpress.android.lint.Utils.nonNullClass +import org.wordpress.android.lint.Utils.nullableClass class MissingNullAnnotationDetectorTest { @@ -27,6 +29,42 @@ class MissingNullAnnotationDetectorTest { ) } + + @Test + fun `it should not inform when @NotNull annotation is present on field`() { + lint().files( + nonNullClass, + LintDetectorTest.java(""" + package test; + + import androidx.annotation.NonNull; + + class ExampleClass { + @NonNull String mExampleField = "example"; + } + """).indented()) + .issues(MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION) + .run() + .expectClean() + } + + @Test + fun `it should not inform when @Nullable annotation is present on field`() { + lint().files( + nullableClass, + LintDetectorTest.java(""" + package test; + + import androidx.annotation.Nullable; + + class ExampleClass { + @Nullable String mExampleField = "example"; + } + """).indented()) + .issues(MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION) + .run() + .expectClean() + } @Test fun `it ignores injected fields`() { lint().files(LintDetectorTest.java(""" @@ -64,6 +102,46 @@ class MissingNullAnnotationDetectorTest { ) } + @Test + fun `it should not inform when @NonNull annotation is present on a method return type`() { + lint().files( + nonNullClass, + LintDetectorTest.java(""" + package test; + + import androidx.annotation.NonNull; + + class ExampleClass { + @NonNull String getMessage() { + return "example"; + } + } + """).indented()) + .issues(MissingNullAnnotationDetector.MISSING_METHOD_RETURN_TYPE_ANNOTATION) + .run() + .expectClean() + } + + @Test + fun `it should not inform when @Nullable annotation is present on a method return type`() { + lint().files( + nullableClass, + LintDetectorTest.java(""" + package test; + + import androidx.annotation.Nullable; + + class ExampleClass { + @Nullable String getMessage() { + return "example"; + } + } + """).indented()) + .issues(MissingNullAnnotationDetector.MISSING_METHOD_RETURN_TYPE_ANNOTATION) + .run() + .expectClean() + } + @Test fun `it should inform when null annotation is missing on a method parameter`() { lint().files(LintDetectorTest.java(""" @@ -86,6 +164,46 @@ class MissingNullAnnotationDetectorTest { .trimIndent() ) } + + @Test + fun `it should not inform when @NonNull annotation is present on a method parameter`() { + lint().files( + nonNullClass, + LintDetectorTest.java(""" + package test; + + import androidx.annotation.NonNull; + + class ExampleClass { + String getMessage(@NonNull String name) { + return name + " example"; + } + } + """).indented()) + .issues(MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION) + .run() + .expectClean() + } + + @Test + fun `it should not inform when @Nullable annotation is present on a method parameter`() { + lint().files( + nullableClass, + LintDetectorTest.java(""" + package test; + + import androidx.annotation.Nullable; + + class ExampleClass { + String getMessage(@Nullable String name) { + return name + " example"; + } + } + """).indented()) + .issues(MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION) + .run() + .expectClean() + } @Test fun `it should inform when null annotation is missing on a construction parameter`() { lint().files(LintDetectorTest.java(""" @@ -107,6 +225,40 @@ class MissingNullAnnotationDetectorTest { ) } @Test + fun `it should not inform when @NonNull annotation is present on a construction parameter`() { + lint().files( + nonNullClass, + LintDetectorTest.java(""" + package test; + + import androidx.annotation.NonNull; + + class ExampleClass { + ExampleClass(@NonNull String name) {} + } + """).indented()) + .issues(MissingNullAnnotationDetector.MISSING_CONSTRUCTOR_PARAMETER_ANNOTATION) + .run() + .expectClean() + } + @Test + fun `it should inform when @Nullable annotation is present on a construction parameter`() { + lint().files( + nullableClass, + LintDetectorTest.java(""" + package test; + + import androidx.annotation.Nullable; + + class ExampleClass { + ExampleClass(@Nullable String name) {} + } + """).indented()) + .issues(MissingNullAnnotationDetector.MISSING_CONSTRUCTOR_PARAMETER_ANNOTATION) + .run() + .expectClean() + } + @Test fun `it ignores injected constructors`() { lint().files(LintDetectorTest.java(""" package test; From e29edfe1863cdf6ebbf65c3b98155de8fcef9f14 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 9 Aug 2023 16:30:08 +1000 Subject: [PATCH 10/16] Remove TODO comment This should be opened as an issue in the repo instead. --- .../wordpress/android/lint/MissingNullAnnotationDetector.kt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt index 7cf977e..32b6322 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -78,10 +78,6 @@ class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { explanation = "Checks for missing `@NonNull/@Nullable` annotations on method return types.", ) - @Suppress("ForbiddenComment") - // TODO: Create a LintFix to normalize our usage to Android's annotations. This can be - // useful for running Nullability analysis. See: - // https://developer.android.com/studio/write/annotations#nullability-analysis val acceptableNullAnnotations = listOf( "androidx.annotation.NonNull", "androidx.annotation.Nullable", From a1d6a7becf055c8f63022e99be360c29c72e427f Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 9 Aug 2023 16:33:32 +1000 Subject: [PATCH 11/16] Reformat MissingNullAnnotationDetector code --- .../lint/MissingNullAnnotationDetector.kt | 101 ++++++++++-------- 1 file changed, 54 insertions(+), 47 deletions(-) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt index 32b6322..14699f4 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -21,7 +21,7 @@ import org.jetbrains.uast.UMethod import org.jetbrains.uast.UParameter import org.jetbrains.uast.UVariable -class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { +class MissingNullAnnotationDetector : Detector(), SourceCodeScanner { override fun getApplicableUastTypes(): List> = listOf( UField::class.java, UMethod::class.java, @@ -31,7 +31,7 @@ class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { if (!isJava(uastFile?.sourcePsi)) { return null } - object: UElementHandler() { + object : UElementHandler() { override fun visitField(node: UField) { if (node.requiresNullAnnotation && !node.isNullAnnotated) { report(node, MISSING_FIELD_ANNOTATION) @@ -56,6 +56,7 @@ class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { } } } + companion object { val MISSING_FIELD_ANNOTATION = Issue.create( id = "MissingNullAnnotationOnField", @@ -88,32 +89,37 @@ class MissingNullAnnotationDetector: Detector(), SourceCodeScanner { /* UVariable Extensions */ private val UVariable.isPrimitive get() = this.type is PsiPrimitiveType private val UVariable.isEnum get() = this is UEnumConstant -private val UVariable.isInjected get() = this.annotations.any { annotation -> - annotation.qualifiedName?.endsWith("Inject") ?: false -} +private val UVariable.isInjected + get() = this.annotations.any { annotation -> + annotation.qualifiedName?.endsWith("Inject") ?: false + } private val UVariable.isConstant get() = this.isStatic && this.isFinal -private val UVariable.isInitializedFinalField get() = this.isFinal - && this.uastInitializer != null -private val UVariable.requiresNullAnnotation get() = - !this.isPrimitive - && !this.isEnum - && !this.isConstant - && !this.isInitializedFinalField - && !this.isInjected +private val UVariable.isInitializedFinalField + get() = this.isFinal + && this.uastInitializer != null +private val UVariable.requiresNullAnnotation + get() = + !this.isPrimitive + && !this.isEnum + && !this.isConstant + && !this.isInitializedFinalField + && !this.isInjected /* UMethod Extensions */ private val UMethod.isPrimitive get() = this.returnType is PsiPrimitiveType -private val UMethod.requiresNullAnnotation get() = - this !is UAnnotationMethod - && !this.isPrimitive - && !this.isConstructor +private val UMethod.requiresNullAnnotation + get() = + this !is UAnnotationMethod + && !this.isPrimitive + && !this.isConstructor /* UAnnotated Extensions */ -private val UAnnotated.isNullAnnotated get() = this.uAnnotations.any { annotation -> - MissingNullAnnotationDetector.acceptableNullAnnotations.any { nullAnnotation -> - annotation.qualifiedName == nullAnnotation +private val UAnnotated.isNullAnnotated + get() = this.uAnnotations.any { annotation -> + MissingNullAnnotationDetector.acceptableNullAnnotations.any { nullAnnotation -> + annotation.qualifiedName == nullAnnotation + } } -} /* Issue.Companion Extensions */ private fun Issue.Companion.create( @@ -143,30 +149,31 @@ private fun JavaContext.report(node: UElement, issue: Issue) = report( ) /* UElement Extensions */ -private val UElement.fixes get() = this.asSourceString().let { sourceString -> - val nullableReplacement = "@Nullable $sourceString" - val nonNullReplacement = "@NonNull $sourceString" +private val UElement.fixes + get() = this.asSourceString().let { sourceString -> + val nullableReplacement = "@Nullable $sourceString" + val nonNullReplacement = "@NonNull $sourceString" - LintFix.create().group() - .add( - LintFix.create() - .name("Annotate as @Nullable") - .replace() - .text(sourceString) - .shortenNames() - .reformat(true) - .with(nullableReplacement) - .build() - ) - .add( - LintFix.create() - .name("Annotate as @NonNull") - .replace() - .text(sourceString) - .shortenNames() - .reformat(true) - .with(nonNullReplacement) - .build() - ) - .build() -} \ No newline at end of file + LintFix.create().group() + .add( + LintFix.create() + .name("Annotate as @Nullable") + .replace() + .text(sourceString) + .shortenNames() + .reformat(true) + .with(nullableReplacement) + .build() + ) + .add( + LintFix.create() + .name("Annotate as @NonNull") + .replace() + .text(sourceString) + .shortenNames() + .reformat(true) + .with(nonNullReplacement) + .build() + ) + .build() + } \ No newline at end of file From 0bad70743f66680d143d1060b273192a369ae5d7 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Tue, 22 Aug 2023 16:14:08 +1000 Subject: [PATCH 12/16] Use braces for multiline conditional --- .../wordpress/android/lint/MissingNullAnnotationDetector.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt index 14699f4..36201f1 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -48,10 +48,11 @@ class MissingNullAnnotationDetector : Detector(), SourceCodeScanner { private fun visitParameter(node: UMethod, parameter: UParameter) { if (parameter.requiresNullAnnotation && !parameter.isNullAnnotated) { - if (node.isConstructor) + if (node.isConstructor) { report(parameter, MISSING_CONSTRUCTOR_PARAMETER_ANNOTATION) - else + } else { report(parameter, MISSING_METHOD_PARAMETER_ANNOTATION) + } } } } From 5d107d21dbc4e84f9295d9a7044361d94d4fc2ee Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Tue, 22 Aug 2023 16:15:38 +1000 Subject: [PATCH 13/16] Reformat extension methods --- .../lint/MissingNullAnnotationDetector.kt | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt index 36201f1..6ec987e 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -88,35 +88,30 @@ class MissingNullAnnotationDetector : Detector(), SourceCodeScanner { } /* UVariable Extensions */ -private val UVariable.isPrimitive get() = this.type is PsiPrimitiveType -private val UVariable.isEnum get() = this is UEnumConstant +private val UVariable.isPrimitive + get() = type is PsiPrimitiveType +private val UVariable.isEnum + get() = this is UEnumConstant private val UVariable.isInjected - get() = this.annotations.any { annotation -> + get() = annotations.any { annotation -> annotation.qualifiedName?.endsWith("Inject") ?: false } -private val UVariable.isConstant get() = this.isStatic && this.isFinal +private val UVariable.isConstant + get() = isStatic && isFinal private val UVariable.isInitializedFinalField - get() = this.isFinal - && this.uastInitializer != null + get() = isFinal && uastInitializer != null private val UVariable.requiresNullAnnotation - get() = - !this.isPrimitive - && !this.isEnum - && !this.isConstant - && !this.isInitializedFinalField - && !this.isInjected + get() = !(isPrimitive || isEnum || isConstant || isInitializedFinalField || isInjected) /* UMethod Extensions */ -private val UMethod.isPrimitive get() = this.returnType is PsiPrimitiveType +private val UMethod.isPrimitive + get() = returnType is PsiPrimitiveType private val UMethod.requiresNullAnnotation - get() = - this !is UAnnotationMethod - && !this.isPrimitive - && !this.isConstructor + get() = this !is UAnnotationMethod && !isPrimitive && !isConstructor /* UAnnotated Extensions */ private val UAnnotated.isNullAnnotated - get() = this.uAnnotations.any { annotation -> + get() = uAnnotations.any { annotation -> MissingNullAnnotationDetector.acceptableNullAnnotations.any { nullAnnotation -> annotation.qualifiedName == nullAnnotation } @@ -151,7 +146,7 @@ private fun JavaContext.report(node: UElement, issue: Issue) = report( /* UElement Extensions */ private val UElement.fixes - get() = this.asSourceString().let { sourceString -> + get() = asSourceString().let { sourceString -> val nullableReplacement = "@Nullable $sourceString" val nonNullReplacement = "@NonNull $sourceString" From 5e807b2471b82bc2ecf898aa32ddd9572722085d Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Tue, 22 Aug 2023 16:16:08 +1000 Subject: [PATCH 14/16] Use noop UElementHandler for non-Java files --- .../org/wordpress/android/lint/MissingNullAnnotationDetector.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt index 6ec987e..264b073 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -29,7 +29,7 @@ class MissingNullAnnotationDetector : Detector(), SourceCodeScanner { override fun createUastHandler(context: JavaContext): UElementHandler? = with(context) { if (!isJava(uastFile?.sourcePsi)) { - return null + return UElementHandler.NONE } object : UElementHandler() { override fun visitField(node: UField) { From dfaedd16b2130d4ad3bb203aa41a64c4a82e5f15 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Tue, 22 Aug 2023 16:17:46 +1000 Subject: [PATCH 15/16] Ignore anonymous constructor implementations --- .../android/lint/MissingNullAnnotationDetector.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt index 264b073..1e0b767 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -14,12 +14,14 @@ import com.android.tools.lint.detector.api.isJava import com.intellij.psi.PsiPrimitiveType import org.jetbrains.uast.UAnnotated import org.jetbrains.uast.UAnnotationMethod +import org.jetbrains.uast.UAnonymousClass import org.jetbrains.uast.UElement import org.jetbrains.uast.UEnumConstant import org.jetbrains.uast.UField import org.jetbrains.uast.UMethod import org.jetbrains.uast.UParameter import org.jetbrains.uast.UVariable +import org.jetbrains.uast.getContainingUClass class MissingNullAnnotationDetector : Detector(), SourceCodeScanner { override fun getApplicableUastTypes(): List> = listOf( @@ -39,6 +41,10 @@ class MissingNullAnnotationDetector : Detector(), SourceCodeScanner { } override fun visitMethod(node: UMethod) { + // Ignore anonymous constructor implementations + if (node.isAnonymousConstructor) { + return + } node.uastParameters.forEach { visitParameter(node, it) } if (node.requiresNullAnnotation && !node.isNullAnnotated) { @@ -108,6 +114,8 @@ private val UMethod.isPrimitive get() = returnType is PsiPrimitiveType private val UMethod.requiresNullAnnotation get() = this !is UAnnotationMethod && !isPrimitive && !isConstructor +private val UMethod.isAnonymousConstructor + get() = isConstructor && getContainingUClass()?.let { it is UAnonymousClass } == true /* UAnnotated Extensions */ private val UAnnotated.isNullAnnotated From bb6d98be145aa9876508d1c53bebf1d0e0032380 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 23 Aug 2023 15:19:11 +1000 Subject: [PATCH 16/16] Use inferred type for createUastHandler override --- .../org/wordpress/android/lint/MissingNullAnnotationDetector.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt index 1e0b767..d08943a 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -29,7 +29,7 @@ class MissingNullAnnotationDetector : Detector(), SourceCodeScanner { UMethod::class.java, ) - override fun createUastHandler(context: JavaContext): UElementHandler? = with(context) { + override fun createUastHandler(context: JavaContext) = with(context) { if (!isJava(uastFile?.sourcePsi)) { return UElementHandler.NONE }