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..d08943a --- /dev/null +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/MissingNullAnnotationDetector.kt @@ -0,0 +1,183 @@ +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.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( + UField::class.java, + UMethod::class.java, + ) + + override fun createUastHandler(context: JavaContext) = with(context) { + if (!isJava(uastFile?.sourcePsi)) { + return UElementHandler.NONE + } + object : UElementHandler() { + override fun visitField(node: UField) { + if (node.requiresNullAnnotation && !node.isNullAnnotated) { + report(node, MISSING_FIELD_ANNOTATION) + } + } + + override fun visitMethod(node: UMethod) { + // Ignore anonymous constructor implementations + if (node.isAnonymousConstructor) { + return + } + node.uastParameters.forEach { visitParameter(node, it) } + + if (node.requiresNullAnnotation && !node.isNullAnnotated) { + report(node, MISSING_METHOD_RETURN_TYPE_ANNOTATION) + } + } + + 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) + } + } + } + } + } + + 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.", + ) + 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.", + ) + + val acceptableNullAnnotations = listOf( + "androidx.annotation.NonNull", + "androidx.annotation.Nullable", + ) + } +} + +/* UVariable Extensions */ +private val UVariable.isPrimitive + get() = type is PsiPrimitiveType +private val UVariable.isEnum + get() = this is UEnumConstant +private val UVariable.isInjected + get() = annotations.any { annotation -> + annotation.qualifiedName?.endsWith("Inject") ?: false + } +private val UVariable.isConstant + get() = isStatic && isFinal +private val UVariable.isInitializedFinalField + get() = isFinal && uastInitializer != null +private val UVariable.requiresNullAnnotation + get() = !(isPrimitive || isEnum || isConstant || isInitializedFinalField || isInjected) + +/* UMethod Extensions */ +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 + get() = uAnnotations.any { annotation -> + MissingNullAnnotationDetector.acceptableNullAnnotations.any { nullAnnotation -> + annotation.qualifiedName == nullAnnotation + } + } + +/* Issue.Companion Extensions */ +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, + ) +) + +/* JavaContext Extensions */ +private fun JavaContext.report(node: UElement, issue: Issue) = report( + issue, + node, + getLocation(node), + "Missing null annotation", + node.fixes, +) + +/* UElement Extensions */ +private val UElement.fixes + get() = 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 52a4951..4410c63 100644 --- a/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt +++ b/WordPressLint/src/main/java/org/wordpress/android/lint/WordPressIssueRegistry.kt @@ -3,22 +3,24 @@ 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, - ) + 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 } 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..1bfab47 --- /dev/null +++ b/WordPressLint/src/test/java/org/wordpress/android/lint/MissingNullAnnotationDetectorTest.kt @@ -0,0 +1,274 @@ +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 { + @Test + fun `it should inform when null annotation is missing on field`() { + lint().files(LintDetectorTest.java(""" + package test; + + class ExampleClass { + String mExampleField = "example"; + } + """).indented()) + .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 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(""" + package test; + + class ExampleClass { + @Inject String mExampleField = "example"; + } + """).indented()) + .issues(MissingNullAnnotationDetector.MISSING_FIELD_ANNOTATION) + .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()) + .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 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(""" + package test; + + class ExampleClass { + String getMessage(String name) { + return name + " example"; + } + } + """).indented()) + .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 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(""" + package test; + + class ExampleClass { + ExampleClass(String name) {} + } + """).indented()) + .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 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; + + class ExampleClass { + @Inject ExampleClass(String name) {} + } + """).indented()) + .issues(MissingNullAnnotationDetector.MISSING_METHOD_PARAMETER_ANNOTATION) + .run() + .expectClean() + } +} 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() + ) +}