-
Notifications
You must be signed in to change notification settings - Fork 97
Feature: Lint Checks #158
Feature: Lint Checks #158
Changes from 9 commits
81221da
b6bfdad
616ad0b
770dfd6
040e117
aebadbe
0166387
d88c24c
2b8e3f0
6fe43a9
3d43e9b
9b34699
7b5ac3e
da0b916
fa159cb
dfafaa0
a0ce173
d339edf
fd26e60
38892fa
d641968
620feca
ce5da87
ea76784
bf9f5c2
139f3f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| .idea | ||
| build/ | ||
| .gradle | ||
| gradle | ||
| gradlew | ||
| gradlew.bat | ||
| .idea/workspace.xml |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| # ThirtyInch - Lint | ||
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| plugins { | ||
| id "org.jetbrains.kotlin.jvm" | ||
| id "jacoco" | ||
| } | ||
|
|
||
| repositories { | ||
| jcenter() | ||
| } | ||
|
|
||
| configurations { | ||
| lintChecks | ||
| } | ||
|
|
||
| dependencies { | ||
| implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8" | ||
|
|
||
| compileOnly "com.android.tools.lint:lint-api:$lintVersion" | ||
| compileOnly "com.android.tools.lint:lint-checks:$lintVersion" | ||
|
|
||
| testImplementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8" | ||
|
||
| testImplementation 'junit:junit:4.12' | ||
| testImplementation "org.assertj:assertj-core:$assertjVersion" | ||
|
|
||
| testImplementation "com.android.tools.lint:lint:$lintVersion" | ||
| testImplementation "com.android.tools.lint:lint-tests:$lintVersion" | ||
|
|
||
| lintChecks files(jar) | ||
| } | ||
|
|
||
| jar { | ||
| manifest { | ||
| attributes("Manifest-Version": 1.0) | ||
| attributes("Lint-Registry": "net.grandcentrix.thirtyinch.lint.TiLintRegistry") | ||
| // The TI checks are build with the new 3.0 APIs (including UAST) so we should also register the v2 lint registry. | ||
| attributes("Lint-Registry-v2": "net.grandcentrix.thirtyinch.lint.TiLintRegistry") | ||
| } | ||
| } | ||
|
|
||
| sourceCompatibility = 1.6 | ||
|
||
|
|
||
| defaultTasks 'assemble' | ||
|
||
| compileKotlin { | ||
| kotlinOptions { | ||
| jvmTarget = "1.8" | ||
| } | ||
| } | ||
|
|
||
| compileTestKotlin { | ||
| kotlinOptions { | ||
| jvmTarget = "1.8" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| package net.grandcentrix.thirtyinch.lint | ||
|
|
||
| import com.android.tools.lint.detector.api.Category | ||
| 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.Scope | ||
| import com.android.tools.lint.detector.api.Severity | ||
|
|
||
| private val CATEGORY_TI = Category.create("ThirtyInch", 5) | ||
|
|
||
| sealed class TiIssue( | ||
| val id: String, | ||
| val briefDescription: String, | ||
| val category: Category, | ||
| val priority: Int, | ||
| val severity: Severity | ||
| ) { | ||
|
|
||
| object MissingView : TiIssue( | ||
| id = "MissingTiViewImplementation", | ||
| briefDescription = "TiView Implementation missing in class", | ||
| category = CATEGORY_TI, | ||
| priority = 8, | ||
| severity = Severity.ERROR | ||
| ) | ||
|
|
||
| fun asLintIssue(detectorCls: Class<out Detector>, description: String = briefDescription): Issue = | ||
| Issue.create( | ||
| id, | ||
| briefDescription, | ||
| description, | ||
| category, | ||
| priority, | ||
| severity, | ||
| Implementation( | ||
| detectorCls, | ||
| Scope.JAVA_FILE_SCOPE | ||
| ) | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package net.grandcentrix.thirtyinch.lint | ||
|
|
||
| import com.android.tools.lint.client.api.IssueRegistry | ||
| import com.android.tools.lint.detector.api.Issue | ||
| import net.grandcentrix.thirtyinch.lint.detector.MissingViewInCompositeDetector | ||
| import net.grandcentrix.thirtyinch.lint.detector.MissingViewInThirtyInchDetector | ||
|
|
||
| class TiLintRegistry : IssueRegistry() { | ||
| override val issues: List<Issue> | ||
| get() = listOf( | ||
| MissingViewInThirtyInchDetector.ISSUE.apply { | ||
| setEnabledByDefault(true) | ||
| }, | ||
| MissingViewInCompositeDetector.ISSUE.apply { | ||
| setEnabledByDefault(true) | ||
| } | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package net.grandcentrix.thirtyinch.lint.detector | ||
|
|
||
| import com.android.tools.lint.detector.api.Detector | ||
| import com.android.tools.lint.detector.api.Issue | ||
| import com.android.tools.lint.detector.api.JavaContext | ||
| import com.android.tools.lint.detector.api.TextFormat | ||
| import com.intellij.psi.PsiClass | ||
| import com.intellij.psi.PsiClassType | ||
| import com.intellij.psi.PsiType | ||
| import com.intellij.psi.util.PsiUtil | ||
| import org.jetbrains.uast.UClass | ||
| import org.jetbrains.uast.getUastContext | ||
|
|
||
| // Base class for Lint checks centered around the notion of "TiView not implemented" | ||
| abstract class BaseMissingViewDetector : Detector(), Detector.UastScanner { | ||
|
|
||
| /** | ||
| * The Issue that the detector is connected to, reported on illegal state detection | ||
| */ | ||
| abstract val issue: Issue | ||
|
|
||
| /** | ||
| * The list of super-classes to detect. | ||
| * We're forcing sub-classed Detectors to implement this by means of redeclaration | ||
| */ | ||
| abstract override fun applicableSuperClasses(): List<String> | ||
|
|
||
| /** | ||
| * Tries to extract the PsiType of the TiView sub-class that is relevant for the given declaration. | ||
| * The relevant super-class (from applicableSuperClasses()) & its resolved variant are given as well. | ||
| */ | ||
| abstract fun tryFindViewInterface(context: JavaContext, declaration: UClass, extendedType: PsiClassType, | ||
| resolvedType: PsiClass): PsiType? | ||
|
|
||
| /** | ||
| * Whether or not to allow the absence of an "implements TiView" clause on the given declaration. | ||
| * The View interface is given as well to allow for further introspection into the setup of the class at hand. | ||
| * When false is returned here, Lint will report the Issue connected to this Detector on the given declaration. | ||
| */ | ||
| abstract fun allowMissingViewInterface(context: JavaContext, declaration: UClass, viewInterface: PsiType): Boolean | ||
|
|
||
| final override fun visitClass(context: JavaContext, declaration: UClass) { | ||
| if (!context.isEnabled(issue)) { | ||
| return | ||
| } | ||
| // Don't trigger on abstract classes | ||
| if (PsiUtil.isAbstractClass(declaration.psi)) { | ||
| return | ||
| } | ||
| // Extract the MVP View type from the declaration | ||
| tryFindViewInterface(context, declaration)?.let { viewInterface -> | ||
| // Check if the class implements that interface as well | ||
| if (!tryFindViewImplementation(context, declaration, viewInterface)) { | ||
| // Interface not implemented; check if alternate condition applies | ||
| if (!allowMissingViewInterface(context, declaration, viewInterface)) { | ||
| // Invalid state: Report issue for this class | ||
| declaration.nameIdentifier?.run { | ||
| context.report( | ||
| issue, | ||
| context.getLocation(this.originalElement), | ||
| issue.getBriefDescription(TextFormat.TEXT)) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun tryFindViewInterface(context: JavaContext, declaration: UClass): PsiType? { | ||
| for (extendedType in declaration.extendsListTypes) { | ||
| extendedType.resolveGenerics().element?.let { resolvedType -> | ||
| val qualifiedName = resolvedType.qualifiedName | ||
| if (applicableSuperClasses().contains(qualifiedName)) { | ||
| // This detector is interested in this class; delegate to it | ||
| return tryFindViewInterface(context, declaration, extendedType, resolvedType) | ||
| } | ||
| // Crawl up the type hierarchy to catch declarations in super classes | ||
| val uastContext = declaration.getUastContext() | ||
| return tryFindViewInterface(context, uastContext.getClass(resolvedType)) | ||
| } | ||
| } | ||
| return null | ||
| } | ||
|
|
||
| private fun tryFindViewImplementation(context: JavaContext, declaration: UClass, | ||
| viewInterface: PsiType): Boolean { | ||
| for (implementedType in declaration.implementsListTypes) { | ||
| if (implementedType == viewInterface) { | ||
| return true | ||
| } | ||
| implementedType.resolve()?.let { resolvedType -> | ||
| val uastContext = declaration.getUastContext() | ||
| return tryFindViewImplementation(context, uastContext.getClass(resolvedType), viewInterface) | ||
| } | ||
| } | ||
| return false | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| package net.grandcentrix.thirtyinch.lint.detector | ||
|
|
||
| import com.android.tools.lint.detector.api.Issue | ||
| import com.android.tools.lint.detector.api.JavaContext | ||
| import com.intellij.psi.PsiClass | ||
| import com.intellij.psi.PsiClassType | ||
| import com.intellij.psi.PsiJavaCodeReferenceElement | ||
| import com.intellij.psi.PsiType | ||
| import net.grandcentrix.thirtyinch.lint.TiIssue.MissingView | ||
| import org.jetbrains.uast.UBlockExpression | ||
| import org.jetbrains.uast.UCallExpression | ||
| import org.jetbrains.uast.UClass | ||
| import org.jetbrains.uast.UExpression | ||
| import org.jetbrains.uast.getUastContext | ||
|
|
||
| private const val ADD_PLUGIN_METHOD = "addPlugin" | ||
| private const val TI_ACTIVITY_PLUGIN_NAME = "TiActivityPlugin" | ||
| private const val TI_FRAGMENT_PLUGIN_NAME = "TiFragmentPlugin" | ||
| private val CA_CLASS_NAMES = listOf( | ||
| "com.pascalwelsch.compositeandroid.activity.CompositeActivity", | ||
| "com.pascalwelsch.compositeandroid.fragment.CompositeFragment" | ||
| ) | ||
|
|
||
| class MissingViewInCompositeDetector : BaseMissingViewDetector() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't know if it make sense because we wanted to drop the support vor CompositeAndroid anyway, right @passsy?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, as long it is still present we should support it. |
||
| companion object { | ||
| val ISSUE = MissingView.asLintIssue( | ||
| MissingViewInCompositeDetector::class.java, | ||
| "When using ThirtyInch, a class extending CompositeActivity or CompositeFragment " + | ||
| "has to implement the TiView interface associated with it in its signature, " + | ||
| "if it applies the respective plugin as well." | ||
| ) | ||
| } | ||
|
|
||
| override fun applicableSuperClasses() = CA_CLASS_NAMES | ||
|
|
||
| override val issue: Issue; get() = MissingViewInThirtyInchDetector.ISSUE | ||
|
|
||
| override fun tryFindViewInterface(context: JavaContext, declaration: UClass, extendedType: PsiClassType, | ||
| resolvedType: PsiClass): PsiType? { | ||
| // Expect TiPlugin to be applied in the extended CA class | ||
| // Found default constructor | ||
| val defaultConstructor = declaration.constructors.firstOrNull { it.typeParameters.isEmpty() } | ||
|
|
||
| defaultConstructor?.let { | ||
| val uastContext = declaration.getUastContext() | ||
| val body = uastContext.getMethodBody(defaultConstructor) | ||
| return tryFindViewFromCompositeConstructor(context, declaration, body) | ||
| } | ||
| return null | ||
| } | ||
|
|
||
| private fun tryFindViewFromCompositeConstructor(context: JavaContext, declaration: UClass, | ||
| expression: UExpression?): PsiType? { | ||
| if (expression == null) { | ||
| return null | ||
| } | ||
| when (expression) { | ||
| is UBlockExpression -> { | ||
| // Unwrap block statements; the first resolvable result is returned | ||
| expression.expressions | ||
| .mapNotNull { tryFindViewFromCompositeConstructor(context, declaration, it) } | ||
| .forEach { return it } | ||
| } | ||
| is UCallExpression -> { | ||
| // Inspect call sites | ||
| if (ADD_PLUGIN_METHOD == expression.methodName && expression.valueArgumentCount == 1) { | ||
| // Expect a plugin to be used as the only argument to this method | ||
| val argument = expression.valueArguments[0] | ||
| if (argument is UCallExpression) { | ||
| val argReference = argument.classReference ?: return null | ||
| val resolvedName = argReference.resolvedName | ||
| if (TI_ACTIVITY_PLUGIN_NAME == resolvedName || TI_FRAGMENT_PLUGIN_NAME == resolvedName) { | ||
| // Matching names. Finally, find the type parameters passed to the plugin | ||
| val psiReference = argReference.psi as PsiJavaCodeReferenceElement? ?: return null | ||
| val parameterTypes = psiReference.typeParameters | ||
| if (parameterTypes.size != 2) { | ||
| return null | ||
| } | ||
| return parameterTypes[1] | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return null | ||
| } | ||
|
|
||
| override fun allowMissingViewInterface(context: JavaContext, declaration: UClass, viewInterface: PsiType) = false | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also calculate this by ourselfs, right? 🤔
Just extract the AGP version and "add" the values to 23.0.0 ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can calculate this ourself. But I would prefer to change that still manually: