Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing null annotation detector #11

Merged
merged 20 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
da7c879
Add missing null annotation detector
mkevins Aug 8, 2023
754ee89
Add missing null annotation detector implementation for methods
mkevins Aug 8, 2023
dae2dac
Add comment labels to groups of extension functions
mkevins Aug 9, 2023
a5fcb44
Fix and add tests for annotations on constructor parameters
mkevins Aug 9, 2023
7184199
Fix implementation for construction parameter issue
mkevins Aug 9, 2023
6aeb4fd
Reformat registry code
mkevins Aug 9, 2023
7295d2a
Remove allowCompileErrors from tests
mkevins Aug 9, 2023
c649e44
Add null annotation test files
mkevins Aug 9, 2023
a12ad11
Add tests to cover cases where null annotations are present
mkevins Aug 9, 2023
ef4e19b
Merge branch 'update-project-apis' into add/missing-null-annotation-d…
mkevins Aug 9, 2023
fee2089
Merge branch 'update-project-apis' into add/missing-null-annotation-d…
mkevins Aug 9, 2023
e29edfe
Remove TODO comment
mkevins Aug 9, 2023
a1d6a7b
Reformat MissingNullAnnotationDetector code
mkevins Aug 9, 2023
c783933
Merge branch 'update-project-apis' into add/missing-null-annotation-d…
mkevins Aug 9, 2023
a612ba2
Merge branch 'update-project-apis' into add/missing-null-annotation-d…
mkevins Aug 9, 2023
0bad707
Use braces for multiline conditional
mkevins Aug 22, 2023
5d107d2
Reformat extension methods
mkevins Aug 22, 2023
5e807b2
Use noop UElementHandler for non-Java files
mkevins Aug 22, 2023
dfaedd1
Ignore anonymous constructor implementations
mkevins Aug 22, 2023
bb6d98b
Use inferred type for createUastHandler override
mkevins Aug 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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<Class<out UElement>> = listOf(
UField::class.java,
UMethod::class.java,
)

override fun createUastHandler(context: JavaContext) = with(context) {
if (!isJava(uastFile?.sourcePsi)) {
return UElementHandler.NONE
mkevins marked this conversation as resolved.
Show resolved Hide resolved
}
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(
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
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",
mkevins marked this conversation as resolved.
Show resolved Hide resolved
)
}
}

/* 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()
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
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
}
Expand Down
Loading