From 52f674a6083e3f14e7e7238b15faef1d69a60ef4 Mon Sep 17 00:00:00 2001 From: Shaishav Gandhi Date: Tue, 11 Dec 2018 15:48:13 -0800 Subject: [PATCH] Add gradle property check for "lenient" flag --- .../autodispose/lint/AutoDisposeDetector.kt | 18 ++++-- .../lint/AutoDisposeDetectorTest.kt | 56 ++++++++++++++++--- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/static-analysis/autodispose-lint/src/main/kotlin/com/uber/autodispose/lint/AutoDisposeDetector.kt b/static-analysis/autodispose-lint/src/main/kotlin/com/uber/autodispose/lint/AutoDisposeDetector.kt index a82c51735..c83669ab6 100644 --- a/static-analysis/autodispose-lint/src/main/kotlin/com/uber/autodispose/lint/AutoDisposeDetector.kt +++ b/static-analysis/autodispose-lint/src/main/kotlin/com/uber/autodispose/lint/AutoDisposeDetector.kt @@ -47,6 +47,7 @@ import java.util.Properties import java.util.EnumSet internal const val CUSTOM_SCOPE_KEY = "autodispose.typesWithScope" +internal const val LENIENT = "autodispose.lenient" /** * Detector which checks if your stream subscriptions are handled by AutoDispose. @@ -104,6 +105,8 @@ class AutoDisposeDetector: Detector(), SourceCodeScanner { // defined by the consumer. private lateinit var appliedScopes: Set + private var lenient: Boolean = false + override fun beforeCheckRootProject(context: Context) { // Add the default Android scopes. val scopes = HashSet(DEFAULT_SCOPES) @@ -119,6 +122,9 @@ class AutoDisposeDetector: Detector(), SourceCodeScanner { .filter { it.isNotBlank() } scopes.addAll(customScopes) } + props.getProperty(LENIENT)?.toBoolean()?.let { + lenient = it + } } appliedScopes = scopes } @@ -207,11 +213,15 @@ class AutoDisposeDetector: Detector(), SourceCodeScanner { if (isReactiveType(evaluator, method) && isInScope(evaluator, node.getContainingUClass()) ) { - val isUnusedReturnValue = isExpressionValueUnused(node) - if (isUnusedReturnValue || !isCapturedTypeAllowed(node.returnType, evaluator)) { - // The subscribe return type isn't handled by consumer or the returned type - // doesn't implement Disposable. + if (!lenient) { context.report(ISSUE, node, context.getLocation(node), LINT_DESCRIPTION) + } else { + val isUnusedReturnValue = isExpressionValueUnused(node) + if (isUnusedReturnValue || !isCapturedTypeAllowed(node.returnType, evaluator)) { + // The subscribe return type isn't handled by consumer or the returned type + // doesn't implement Disposable. + context.report(ISSUE, node, context.getLocation(node), LINT_DESCRIPTION) + } } } } diff --git a/static-analysis/autodispose-lint/src/test/kotlin/com/uber/autodispose/lint/AutoDisposeDetectorTest.kt b/static-analysis/autodispose-lint/src/test/kotlin/com/uber/autodispose/lint/AutoDisposeDetectorTest.kt index 3b9a9ccda..7f0ebf109 100644 --- a/static-analysis/autodispose-lint/src/test/kotlin/com/uber/autodispose/lint/AutoDisposeDetectorTest.kt +++ b/static-analysis/autodispose-lint/src/test/kotlin/com/uber/autodispose/lint/AutoDisposeDetectorTest.kt @@ -16,6 +16,7 @@ package com.uber.autodispose.lint +import com.android.tools.lint.checks.infrastructure.TestFile import com.android.tools.lint.checks.infrastructure.TestFiles.java import com.android.tools.lint.checks.infrastructure.TestFiles.kotlin import com.android.tools.lint.checks.infrastructure.TestFiles.projectProperties @@ -70,6 +71,13 @@ class AutoDisposeDetectorTest { class ClassWithCustomScope {} """).indented() + + private fun lenientPropertiesFile(lenient: Boolean = true): TestFile.PropertyTestFile { + val properties = projectProperties() + properties.property(LENIENT, lenient.toString()) + properties.to(AutoDisposeDetector.PROPERTY_FILE) + return properties + } } @Test fun observableErrorsOutOnOmittingAutoDispose() { @@ -523,7 +531,8 @@ class AutoDisposeDetectorTest { } @Test fun capturedDisposable() { - lint().files(rxJava2(), LIFECYCLE_OWNER, ACTIVITY, kotlin(""" + val propertiesFile = lenientPropertiesFile() + lint().files(rxJava2(), propertiesFile, LIFECYCLE_OWNER, ACTIVITY, kotlin(""" package foo import androidx.appcompat.app.AppCompatActivity import io.reactivex.Observable @@ -540,7 +549,8 @@ class AutoDisposeDetectorTest { } @Test fun nestedDisposable() { - lint().files(rxJava2(), LIFECYCLE_OWNER, ACTIVITY, kotlin(""" + val propertiesFile = lenientPropertiesFile() + lint().files(rxJava2(), propertiesFile, LIFECYCLE_OWNER, ACTIVITY, kotlin(""" package foo import androidx.appcompat.app.AppCompatActivity import io.reactivex.Observable @@ -584,7 +594,8 @@ class AutoDisposeDetectorTest { } @Test fun checkLenientLintWithLambdas() { - lint().files(rxJava2(), LIFECYCLE_OWNER, ACTIVITY, kotlin(""" + val propertiesFile = lenientPropertiesFile() + lint().files(rxJava2(), propertiesFile, LIFECYCLE_OWNER, ACTIVITY, kotlin(""" package foo import androidx.appcompat.app.AppCompatActivity import io.reactivex.Observable @@ -605,7 +616,8 @@ class AutoDisposeDetectorTest { } @Test fun javaCapturedDisposable() { - lint().files(rxJava2(), LIFECYCLE_OWNER, ACTIVITY, java(""" + val propertiesFile = lenientPropertiesFile() + lint().files(rxJava2(), propertiesFile, LIFECYCLE_OWNER, ACTIVITY, java(""" package foo; import androidx.appcompat.app.AppCompatActivity; import io.reactivex.Observable; @@ -622,6 +634,28 @@ class AutoDisposeDetectorTest { .expectClean() } + @Test fun javaCapturedDisposableWithoutLenientProperty() { + val propertiesFile = lenientPropertiesFile(false) + lint().files(rxJava2(), propertiesFile, LIFECYCLE_OWNER, ACTIVITY, java(""" + package foo; + import androidx.appcompat.app.AppCompatActivity; + import io.reactivex.Observable; + import io.reactivex.disposables.Disposable; + + class MyActivity extends AppCompatActivity { + fun doSomething() { + Disposable disposable = Observable.just(1, 2, 3).subscribe(); + } + } + """).indented()) + .issues(AutoDisposeDetector.ISSUE) + .run() + .expect("""src/foo/MyActivity.java:8: Error: Always apply an AutoDispose scope before subscribing within defined scoped elements. [AutoDisposeUsage] + | Disposable disposable = Observable.just(1, 2, 3).subscribe(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |1 errors, 0 warnings""".trimMargin()) + } + @Test fun subscribeWithCapturedNonDisposableFromMethodReference() { lint() .files(rxJava2(), LIFECYCLE_OWNER, FRAGMENT, java(""" @@ -672,8 +706,9 @@ class AutoDisposeDetectorTest { } @Test fun subscribeWithCapturedDisposableFromMethodReference() { + val propertiesFile = lenientPropertiesFile() lint() - .files(rxJava2(), LIFECYCLE_OWNER, FRAGMENT, java(""" + .files(rxJava2(), propertiesFile, LIFECYCLE_OWNER, FRAGMENT, java(""" package foo; import io.reactivex.Observable; import io.reactivex.observers.DisposableObserver; @@ -717,8 +752,9 @@ class AutoDisposeDetectorTest { } @Test fun kotlinSubscribeWithCapturedNonDisposableFromMethodReference() { + val propertiesFile = lenientPropertiesFile() lint() - .files(rxJava2(), LIFECYCLE_OWNER, FRAGMENT, kotlin(""" + .files(rxJava2(), propertiesFile, LIFECYCLE_OWNER, FRAGMENT, kotlin(""" package foo import io.reactivex.Observable import io.reactivex.observers.DisposableObserver @@ -758,13 +794,14 @@ class AutoDisposeDetectorTest { .run() .expect("""src/foo/ExampleClass.kt:12: Error: Always apply an AutoDispose scope before subscribing within defined scoped elements. [AutoDisposeUsage] | val observer: Observer = methodReferencable(Function { observable.subscribeWith(it) }) - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |1 errors, 0 warnings""".trimMargin()) } @Test fun kotlinSubscribeWithCapturedDisposableFromMethodReference() { + val propertiesFile = lenientPropertiesFile() lint() - .files(rxJava2(), LIFECYCLE_OWNER, FRAGMENT, kotlin(""" + .files(rxJava2(), propertiesFile, LIFECYCLE_OWNER, FRAGMENT, kotlin(""" package foo import io.reactivex.Observable import io.reactivex.observers.DisposableObserver @@ -842,8 +879,9 @@ class AutoDisposeDetectorTest { } @Test fun subscribeWithCapturedDisposable() { + val propertiesFile = lenientPropertiesFile() lint() - .files(rxJava2(), LIFECYCLE_OWNER, FRAGMENT, java(""" + .files(rxJava2(), propertiesFile, LIFECYCLE_OWNER, FRAGMENT, java(""" package foo; import io.reactivex.Observable; import io.reactivex.observers.DisposableObserver;