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

Publicly usable lint checks #1427

Merged
merged 3 commits into from Sep 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions firestore/build.gradle.kts
Expand Up @@ -24,6 +24,8 @@ dependencies {

compileOnly(Config.Libs.Arch.paging)

lintChecks(project(":lint"))

androidTestImplementation(Config.Libs.Test.junit)
androidTestImplementation(Config.Libs.Test.runner)
androidTestImplementation(Config.Libs.Test.rules)
Expand Down
2 changes: 1 addition & 1 deletion internal/lint/build.gradle.kts
Expand Up @@ -12,6 +12,6 @@ dependencies {

tasks.withType<Jar>().configureEach {
manifest {
attributes(mapOf("Lint-Registry-v2" to "com.firebaseui.lint.LintIssueRegistry"))
attributes(mapOf("Lint-Registry-v2" to "com.firebaseui.lint.internal.LintIssueRegistry"))
}
}
@@ -0,0 +1,12 @@
package com.firebaseui.lint.internal

import com.android.tools.lint.client.api.IssueRegistry

/**
* Registry for custom FirebaseUI lint checks.
*/
class LintIssueRegistry : IssueRegistry() {
override val issues = listOf(
NonGlobalIdDetector.NON_GLOBAL_ID
)
}
@@ -1,4 +1,4 @@
package com.firebaseui.lint
package com.firebaseui.lint.internal

import com.android.tools.lint.detector.api.Category
import com.android.tools.lint.detector.api.Implementation
Expand Down
18 changes: 0 additions & 18 deletions internal/lint/src/test/java/com/firebaseui/lint/LintTestHelper.kt

This file was deleted.

@@ -1,11 +1,25 @@
package com.firebaseui.lint
package com.firebaseui.lint.internal

import com.android.tools.lint.checks.infrastructure.TestFiles.xml
import com.firebaseui.lint.LintTestHelper.configuredLint
import com.firebaseui.lint.NonGlobalIdDetector.Companion.NON_GLOBAL_ID
import com.android.tools.lint.checks.infrastructure.TestLintTask
import com.firebaseui.lint.internal.NonGlobalIdDetector.Companion.NON_GLOBAL_ID
import org.junit.Test
import java.io.File

class NonGlobalIdDetectorTest {

// Nasty hack to make lint tests pass on Windows. For some reason, lint doesn't
// automatically find the Android SDK in its standard path on Windows. This hack looks
// through the system properties to find the path defined in `local.properties` and then
// sets lint's SDK home to that path if it's found.
private val sdkPath = System.getProperty("java.library.path").split(';').find {
it.contains("SDK", true)
}

fun configuredLint(): TestLintTask = TestLintTask.lint().apply {
sdkHome(File(sdkPath ?: return@apply))
}

@Test
fun `Passes on valid view id`() {
configuredLint()
Expand Down
17 changes: 17 additions & 0 deletions lint/build.gradle.kts
@@ -0,0 +1,17 @@
plugins {
id("kotlin")
}

dependencies {
compileOnly(Config.Libs.Lint.api)
compileOnly(Config.Libs.Kotlin.jvm)

testImplementation(Config.Libs.Lint.api)
testImplementation(Config.Libs.Lint.tests)
}

tasks.withType<Jar>().configureEach {
manifest {
attributes(mapOf("Lint-Registry-v2" to "com.firebaseui.lint.LintIssueRegistry"))
}
}
Expand Up @@ -7,8 +7,7 @@ import com.android.tools.lint.client.api.IssueRegistry
*/
class LintIssueRegistry : IssueRegistry() {
override val issues = listOf(
NonGlobalIdDetector.NON_GLOBAL_ID,
FirestoreRecyclerAdapterLifecycleDetector.ISSUE_MISSING_LISTENING_START_METHOD,
FirestoreRecyclerAdapterLifecycleDetector.ISSUE_MISSING_LISTENING_STOP_METHOD
FirestoreRecyclerAdapterLifecycleDetector.ISSUE_MISSING_LISTENING_START_METHOD,
FirestoreRecyclerAdapterLifecycleDetector.ISSUE_MISSING_LISTENING_STOP_METHOD
)
}
@@ -1,13 +1,26 @@
package com.firebaseui.lint

import com.android.tools.lint.checks.infrastructure.TestFiles.java
import com.android.tools.lint.checks.infrastructure.TestLintTask
import com.firebaseui.lint.FirestoreRecyclerAdapterLifecycleDetector.Companion.ISSUE_MISSING_LISTENING_START_METHOD
import com.firebaseui.lint.FirestoreRecyclerAdapterLifecycleDetector.Companion.ISSUE_MISSING_LISTENING_STOP_METHOD
import com.firebaseui.lint.LintTestHelper.configuredLint
import org.junit.Test
import java.io.File

class FirestoreRecyclerAdapterLifecycleDetectorTest {

// Nasty hack to make lint tests pass on Windows. For some reason, lint doesn't
// automatically find the Android SDK in its standard path on Windows. This hack looks
// through the system properties to find the path defined in `local.properties` and then
// sets lint's SDK home to that path if it's found.
private val sdkPath = System.getProperty("java.library.path").split(';').find {
it.contains("SDK", true)
}

fun configuredLint(): TestLintTask = TestLintTask.lint().apply {
sdkHome(File(sdkPath ?: return@apply))
}

@Test
fun `Checks missing startListening() method call`() {
configuredLint()
Expand Down
5 changes: 5 additions & 0 deletions proguard-tests/build.gradle.kts
Expand Up @@ -4,6 +4,11 @@ android {
}

buildTypes {
named("debug").configure {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why'd we add this back? It adds like 5+ mins to a full debug build. Shouldn't we only run it in CI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh damn I forgot why we removed it. Android Studio gets pretty angry when not all modules have matching configurations and this made it happy.

Let me see if I can appease Android Studio without compromising time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ohhh, yeah, I remember seeing those warnings. Let's use a variant filter then: https://developer.android.com/studio/build/build-variants#filter-variants

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep it looks like that's already working with setIgnore. I pushed a commit to simplify the debug config to the minimum that makes AS happy.

I did a time of some clean builds and the extra config doesn't cost us anything. Will confirm via Travis logs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome, SGTM!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Travis confirmed: no slowdown.

// This empty config is only here to make Android Studio happy.
// This build type is later ignored in the variantFilter section
}

named("release").configure {
// For the purposes of the sample, allow testing of a proguarded release build
// using the debug key
Expand Down
3 changes: 3 additions & 0 deletions settings.gradle.kts
Expand Up @@ -5,5 +5,8 @@ include(
":common", ":firestore", ":database",
":storage",


":lint",

":proguard-tests", ":internal:lint", ":internal:lintchecks"
)