-
Notifications
You must be signed in to change notification settings - Fork 97
Added Lint Check for missing TiView implementation #111
Changes from 8 commits
8d47fca
abf377a
28c6fe6
57c075d
44b3f23
57697a6
1f6526e
edf8c57
decba0c
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 @@ | ||
| /build |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| buildscript { | ||
| repositories { | ||
| jcenter() | ||
| } | ||
| dependencies { | ||
| classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion" | ||
| } | ||
| } | ||
|
|
||
| apply plugin: 'kotlin' | ||
| apply plugin: 'jacoco' | ||
|
|
||
| targetCompatibility = JavaVersion.VERSION_1_7 | ||
| sourceCompatibility = JavaVersion.VERSION_1_7 | ||
|
|
||
| configurations { | ||
| lintChecks | ||
| } | ||
|
|
||
| jar { | ||
| manifest { | ||
| attributes('Lint-Registry': 'net.grandcentrix.thirtyinch.IssueRegistry') | ||
| } | ||
| } | ||
|
|
||
| repositories { | ||
| maven { url "https://dl.bintray.com/android/android-tools" } | ||
|
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. I don't like such "special cases".
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. Yeah, I agree that it's unfortunate that we have to include the additional repository, just so we can access Lint-specific dependencies. A comment and, potentially, a link to an issue over at b.android.com would be helpful here! |
||
| } | ||
|
|
||
| dependencies { | ||
| compile "org.jetbrains.kotlin:kotlin-stdlib:$kotlinVersion" | ||
| compile "com.android.tools.lint:lint-api:$lintVersion" | ||
| compile "com.android.tools.lint:lint-checks:$lintVersion" | ||
|
|
||
| testCompile "com.android.tools.lint:lint:$lintVersion" | ||
| testCompile "com.android.tools.lint:lint-tests:$lintVersion" | ||
| testCompile "org.assertj:assertj-core:$assertjVersion" | ||
|
|
||
| lintChecks files(jar) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| package net.grandcentrix.thirtyinch | ||
|
|
||
| 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 | ||
| */ | ||
| override abstract 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 | ||
|
|
||
| override final 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 | ||
| context.report( | ||
| issue, | ||
| context.getLocation(declaration.nameIdentifier), | ||
| 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,9 @@ | ||
| package net.grandcentrix.thirtyinch | ||
|
|
||
| class IssueRegistry : com.android.tools.lint.client.api.IssueRegistry() { | ||
|
|
||
| override fun getIssues() = listOf( | ||
| MissingViewInThirtyInchDetector.ISSUE, | ||
| MissingViewInCompositeDetector.ISSUE | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package net.grandcentrix.thirtyinch | ||
|
|
||
| import com.android.tools.lint.detector.api.* | ||
| import java.util.* | ||
|
|
||
| enum class Issues( | ||
|
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. I don't like enum. Especially if we can just create a
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. The |
||
| val id: String, | ||
| val briefDescription: String, | ||
| val category: Category, | ||
| val priority: Int, | ||
| val severity: Severity) { | ||
|
|
||
| MISSING_VIEW( | ||
| id = "MissingTiViewImplementation", | ||
| briefDescription = "TiView Implementation missing in class", | ||
| category = Category.CORRECTNESS, | ||
| priority = 8, | ||
| severity = Severity.ERROR); | ||
|
|
||
| fun create(detectorCls: Class<out Detector>, description: String = briefDescription): Issue = | ||
| Issue.create( | ||
| id, | ||
| briefDescription, | ||
| description, | ||
| category, | ||
| priority, | ||
| severity, | ||
| Implementation( | ||
| detectorCls, | ||
| EnumSet.of(Scope.JAVA_FILE, Scope.TEST_SOURCES))) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| package net.grandcentrix.thirtyinch | ||
|
|
||
| import com.android.annotations.VisibleForTesting | ||
| 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 org.jetbrains.uast.* | ||
|
||
|
|
||
| private val ADD_PLUGIN_METHOD = "addPlugin" | ||
| private val TI_ACTIVITY_PLUGIN_NAME = "TiActivityPlugin" | ||
| private 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() { | ||
|
|
||
| companion object { | ||
| @VisibleForTesting | ||
| val ISSUE: Issue = Issues.MISSING_VIEW.create( | ||
| 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() = 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 | ||
| .filter { it.typeParameters.isEmpty() } | ||
| .firstOrNull() | ||
|
|
||
| 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 | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package net.grandcentrix.thirtyinch | ||
|
|
||
| import com.android.annotations.VisibleForTesting | ||
| 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.PsiType | ||
| import org.jetbrains.uast.UClass | ||
|
|
||
| private val TI_VIEW_FQ = "net.grandcentrix.thirtyinch.TiView" | ||
| private val PROVIDE_VIEW_METHOD = "provideView" | ||
| private val TI_CLASS_NAMES = listOf( | ||
| "net.grandcentrix.thirtyinch.TiActivity", | ||
| "net.grandcentrix.thirtyinch.TiFragment") | ||
|
|
||
| class MissingViewInThirtyInchDetector : BaseMissingViewDetector() { | ||
|
|
||
| companion object { | ||
| @VisibleForTesting | ||
| val ISSUE: Issue = Issues.MISSING_VIEW.create( | ||
| MissingViewInThirtyInchDetector::class.java, | ||
| "When using ThirtyInch, a class extending TiActivity, TiFragment or CompositeActivity " + | ||
| "has to implement the TiView interface associated with it in its signature, " + | ||
| "or implement `provideView()` instead to override this default behaviour.") | ||
| } | ||
|
|
||
| override fun applicableSuperClasses() = TI_CLASS_NAMES | ||
|
|
||
| override val issue: Issue | ||
| get() = ISSUE | ||
|
|
||
| override fun tryFindViewInterface(context: JavaContext, declaration: UClass, extendedType: PsiClassType, resolvedType: PsiClass): PsiType? { | ||
| // Expect <P extends TiPresenter, V extends TiView> signature in the extended Ti class | ||
| val parameters = extendedType.parameters | ||
| val parameterTypes = resolvedType.typeParameters | ||
| if (parameters.size != 2 || parameterTypes.size != 2) { | ||
| return null | ||
| } | ||
|
|
||
| // Check that the second type parameter is actually a TiView | ||
| val parameterType = parameterTypes[1] | ||
| val parameter = parameters[1] | ||
| return parameterType.extendsListTypes | ||
| .map { it.resolveGenerics().element } | ||
| .filter { TI_VIEW_FQ == it?.qualifiedName } | ||
| .map { parameter } | ||
| .firstOrNull() | ||
| } | ||
|
|
||
| override fun allowMissingViewInterface(context: JavaContext, declaration: UClass, viewInterface: PsiType): Boolean { | ||
| // Interface not implemented; check if provideView() is overridden instead | ||
| return declaration.findMethodsByName(PROVIDE_VIEW_METHOD, true) | ||
| .any { viewInterface == it.returnType } | ||
| } | ||
| } |
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.
Is the alpha version required for UAST? Isn't there a stable one?
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.
The latest stable release of the Lint libraries (25.3.0) doesn't include UAST yet, no. I suppose it'll be out once AS 3.0 is stable, though.