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

WIP: Add lint restriction for Binder #65

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions buildSrc/buildSrc/src/main/kotlin/Deps.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ object Deps {
const val compiler = "com.github.stephanenicolas.toothpick:toothpick-compiler:${Versions.toothpick}"
}

object lint {
const val core = "com.android.tools.lint:lint:${Versions.androidTools}"
const val api = "com.android.tools.lint:lint-api:${Versions.androidTools}"
const val checks = "com.android.tools.lint:lint-checks:${Versions.androidTools}"
const val tests = "com.android.tools.lint:lint-tests:${Versions.androidTools}"
}
const val testutils = "com.android.tools:testutils:${Versions.androidTools}"

const val cicerone = "ru.terrakok.cicerone:cicerone:${Versions.cicerone}"

const val timber = "com.jakewharton.timber:timber:${Versions.timber}"
Expand Down
2 changes: 2 additions & 0 deletions buildSrc/buildSrc/src/main/kotlin/Versions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,6 @@ object Versions {
const val bintray = "1.8.5"

const val ktlint = "0.39.0"

const val androidTools = "27.1.2"
}
27 changes: 27 additions & 0 deletions gemini-lint/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
plugins {
id("java-library")
id("kotlin")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавь сюда еще ktlint плагин

}

java {
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_1_8
}

dependencies {
compileOnly(Deps.lint.api)
compileOnly(Deps.lint.checks)

testImplementation(Deps.kotlinTestJunit)
testImplementation(Deps.lint.core)
testImplementation(Deps.lint.tests)
testImplementation(Deps.testutils)
}

tasks.jar {
manifest {
attributes(
"Lint-Registry-v3" to "com.haroncode.gemini.lint.GenimiIssueRegistry"
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.haroncode.gemini.lint

import com.android.tools.lint.client.api.IssueRegistry
import com.android.tools.lint.detector.api.CURRENT_API
import com.android.tools.lint.detector.api.Issue

class GenimiIssueRegistry : IssueRegistry() {
override val issues = WrongGenimiUsageDetector.issues

override val api = CURRENT_API
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
@file:Suppress("UnstableApiUsage")

package com.haroncode.gemini.lint

import com.android.tools.lint.detector.api.*
import com.intellij.psi.PsiMethod
import org.jetbrains.uast.UCallExpression

class WrongGenimiUsageDetector : Detector(), SourceCodeScanner {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот тут есть подозрение, что используется старое API посмотри как сделано на нашем рабочем проекте (название не называю из-за NDA)

override fun getApplicableMethodNames() = listOf("bind")

override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
val evaluator = context.evaluator

if (evaluator.isMemberInSubClassOf(method, "com.haroncode.gemini.binder.Binder")) {
checkCallMethod(method, node, context)
}
}



private fun checkCallMethod(method: PsiMethod, call: UCallExpression, context: JavaContext) {
var parent = call.uastParent
while (parent != null && parent !is PsiMethod) {
parent = parent.uastParent
}
if (parent !is PsiMethod) return
if (parent.name != "onCreate") {
context.report(ISSUE_ON_CREATE, method, context.getLocation(call), "Calling bind from ${parent.name} instead of onCreate")
}
}

companion object {
val ISSUE_ON_CREATE = Issue.create(
"ShouldBeCalledInOnCreate",
"Bind method for ViewStoreBinding should be called in onCreate method",
"Based on documentation you should call bind in the onCreate method of your's view",
Category.CORRECTNESS,
5,
Severity.ERROR,
Implementation(WrongGenimiUsageDetector::class.java, Scope.JAVA_FILE_SCOPE)
)

val issues = listOf(
ISSUE_ON_CREATE
)
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
com.haroncode.gemini.lint.GenimiIssueRegistry
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package com.haroncode.gemini.lint

import com.android.tools.lint.checks.infrastructure.LintDetectorTest

@Suppress("UnstableApiUsage")
class WrongGenimiUsageDetectorTest : LintDetectorTest() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут у тестов тоже старое апи, желательно поднять версию, так как может тупить

companion object {
private val GEMINI_STUB1 = kotlin(BINDER_STUB)
private val GEMINI_STUB2 = kotlin(BINDER_RULES)
private val OTHER_STUB = kotlin(OTHER_BINDER_STUB)
}

fun testRightUsageWithActivity() {
lint()
.files(
GEMINI_STUB1,
GEMINI_STUB2,
kotlin("""
package ru.test.app

import android.app.Activity
import com.haroncode.gemini.binder.*
import com.haroncode.gemini.binder.rule.*

class Activity1 : Activity() {
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

StoreViewBinding.with(generateSampleRulesFactory<Any>())
.bind(this)
}
}
""".trimIndent())
)
.requireCompileSdk()
.run()
.expectClean()
}

fun testRightUsageWithFragment() {
lint()
.files(
GEMINI_STUB1,
GEMINI_STUB2,
kotlin("""
package ru.test.app

import android.app.Fragment
import com.haroncode.gemini.binder.*
import com.haroncode.gemini.binder.rule.*

class Fragment1 : Fragment() {
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

StoreViewBinding.with(generateSampleRulesFactory<Any>())
.bind(this)
}
}
""".trimIndent())
)
.requireCompileSdk()
.run()
.expectClean()
}

fun testWrongUsageWithFragment_onViewCreated() {
lint()
.files(
GEMINI_STUB1,
GEMINI_STUB2,
kotlin("""
package ru.test.app

import android.app.Fragment
import com.haroncode.gemini.binder.*
import com.haroncode.gemini.binder.rule.*

class Fragment1 : Fragment() {
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)

StoreViewBinding.with(generateSampleRulesFactory<Any>())
.bind(this)
}
}
""".trimIndent())
)
.requireCompileSdk()
.run()
.expect("""
src/ru/test/app/Fragment1.kt:11: Error: Calling bind from onViewCreated instead of onCreate [ShouldBeCalledInOnCreate]
StoreViewBinding.with(generateSampleRulesFactory<Any>())
^
1 errors, 0 warnings
""".trimIndent())
}

fun testWrongUsageWithActivity_onResume() {
lint()
.files(
GEMINI_STUB1,
GEMINI_STUB2,
kotlin("""
package ru.test.app

import android.app.Activity
import com.haroncode.gemini.binder.*
import com.haroncode.gemini.binder.rule.*

class Activity1 : Activity() {
override fun onResume() {
super.onViewCreated(view, savedInstanceState)

StoreViewBinding.with(generateSampleRulesFactory<Any>())
.bind(this)
}
}
""".trimIndent())
)
.requireCompileSdk()
.run()
.expect("""
src/ru/test/app/Activity1.kt:11: Error: Calling bind from onResume instead of onCreate [ShouldBeCalledInOnCreate]
StoreViewBinding.with(generateSampleRulesFactory<Any>())
^
1 errors, 0 warnings
""".trimIndent())
}

fun testDoesNotTriggeredByOtherClasses() {
lint()
.files(
OTHER_STUB,
kotlin("""
package ru.test.app

import android.app.Fragment
import ru.test.lib.*

class Fragment1 : Fragment() {
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)

StoreViewBinding.with().bind(this)
}
}
""".trimIndent())
)
.requireCompileSdk()
.run()
.expectClean()

}

override fun getDetector() = WrongGenimiUsageDetector()

override fun getIssues() = WrongGenimiUsageDetector.issues
}
74 changes: 74 additions & 0 deletions gemini-lint/src/test/java/com/haroncode/gemini/lint/stubs.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package com.haroncode.gemini.lint

val BINDER_STUB = """
package com.haroncode.gemini.binder

import com.haroncode.gemini.binder.rule.*

object StoreViewBinding {
fun <T : SavedStateRegistryOwner> with(
lifecycleStrategy: LifecycleStrategy = StartStopStrategy,
factoryProvider: () -> BindingRulesFactory<T>,
): Binder<T> = SimpleBinder(
factoryProvider = factoryProvider,
lifecycleStrategy = lifecycleStrategy,
)
fun <T : SavedStateRegistryOwner> withRestore(
lifecycleStrategy: LifecycleStrategy = StartStopStrategy,
factoryProvider: () -> BindingRulesFactory<T>,
): Binder<T> = RestoreBinder(
factoryProvider = factoryProvider,
lifecycleStrategy = lifecycleStrategy,
)
fun <T : LifecycleOwner> with(
factory: BindingRulesFactory<T>,
lifecycleStrategy: LifecycleStrategy = StartStopStrategy
): Binder<T> = BinderImpl(
factory = factory,
lifecycleStrategy = lifecycleStrategy
)
}

interface Binder<View> {
fun bind(view: View)
}

internal class BinderImpl<View : LifecycleOwner>(
private val factory: BindingRulesFactory<View>,
private val lifecycleStrategy: LifecycleStrategy
) : Binder<View> {
override fun bind(view: View) { }
}
""".trimIndent()

val BINDER_RULES = """
package com.haroncode.gemini.binder.rule

interface BindingRulesFactory<P : Any> {
fun create(param: P): Collection<BindingRule>
}

interface BindingRule {
suspend fun bind()
}

fun <T : Any> generateSampleRulesFactory() = object : BindingRulesFactory<T> {
override fun create(param: T): Collection<BindingRule> = emptyList()
}
""".trimIndent()

val OTHER_BINDER_STUB = """
package ru.test.lib

object StoreViewBinding {
fun <T : LifecycleOwner> with(): Binder<T> = BinderImpl()
}

interface Binder<View> {
fun bind(view: View)
}

class BinderImpl<View> : Binder<View> {
override fun bind(view: View) {}
}
""".trimIndent()
2 changes: 2 additions & 0 deletions sample/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,6 @@ dependencies {
implementation(Deps.timber)

implementation(project(":gemini-binder"))

lintChecks(project(":gemini-lint"))
}
1 change: 1 addition & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
include(":gemini-lint")
include(
":gemini-core",
":gemini-core-test",
Expand Down