From 85ecc865b367bfecc46c18b3b3c7ee5655075efd Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Tue, 2 Jan 2018 10:59:21 +0530 Subject: [PATCH 01/26] Error prone check added to check unbinded subscriptions Error prone flag added to support components with custom lifecycle Removed unused dependencies --- gradle/dependencies.gradle | 7 + settings.gradle | 1 + .../build.gradle | 70 +++++++ .../gradle.properties | 19 ++ .../RxJavaMissingAutoDisposeErrorChecker.java | 197 ++++++++++++++++++ .../prone/checker/ComponentWithLifeCycle.java | 19 ++ ...avaMissingAutoDisposeErrorCheckerTest.java | 63 ++++++ .../MissingAutoDisposeErrorNegativeCases.java | 123 +++++++++++ .../MissingAutoDisposeErrorPositiveCases.java | 101 +++++++++ ...MissingAutoDisposeErrorPositiveCases2.java | 51 +++++ 10 files changed, 651 insertions(+) create mode 100644 tooling/autodispose-rx-error-prone-checker/build.gradle create mode 100644 tooling/autodispose-rx-error-prone-checker/gradle.properties create mode 100644 tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java create mode 100644 tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/ComponentWithLifeCycle.java create mode 100644 tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorCheckerTest.java create mode 100644 tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java create mode 100644 tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java create mode 100644 tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 4af5a9d4a..e2eeadd48 100755 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -24,6 +24,10 @@ def versions = [ support: '26.1.0' ] +def apt = [ + autoService : "com.google.auto.service:auto-service:1.0-rc3", +] + def build = [ buildToolsVersion: '26.0.2', compileSdkVersion: 26, @@ -32,7 +36,9 @@ def build = [ targetSdkVersion: 26, checkerFramework: 'org.checkerframework:dataflow:2.3.0', + errorProneCheckApi: "com.google.errorprone:error_prone_check_api:${versions.errorProne}", errorProne: "com.google.errorprone:error_prone_core:${versions.errorProne}", + errorProneTestHelpers: "com.google.errorprone:error_prone_test_helpers:${versions.errorProne}", nullAway: 'com.uber.nullaway:nullaway:0.2.0', repositories: [ @@ -84,6 +90,7 @@ def test = [ ] ext.deps = [ + "apt": apt, "build": build, "kotlin": kotlin, "misc": misc, diff --git a/settings.gradle b/settings.gradle index a74b395c8..6da674c06 100755 --- a/settings.gradle +++ b/settings.gradle @@ -26,4 +26,5 @@ include ':autodispose-kotlin' include 'autodispose-rxlifecycle' include ':sample' include ':test-utils' +include ':tooling:autodispose-rx-error-prone-checker' diff --git a/tooling/autodispose-rx-error-prone-checker/build.gradle b/tooling/autodispose-rx-error-prone-checker/build.gradle new file mode 100644 index 000000000..fc4b8bd40 --- /dev/null +++ b/tooling/autodispose-rx-error-prone-checker/build.gradle @@ -0,0 +1,70 @@ +plugins { + id "net.ltgt.errorprone" version "0.0.13" + id "com.github.johnrengelman.shadow" version "2.0.1" + id "com.github.johnrengelman.plugin-shadow" version "2.0.0" + id "java" +} + +// we use this config to get the path of the JDK 9 javac jar, to +// stick it in the bootclasspath when running tests +configurations.maybeCreate("epJavac") + +sourceCompatibility = "1.8" +targetCompatibility = "1.8" + +dependencies { + compileOnly deps.apt.autoService + testCompile deps.rx.java + + compileOnly deps.build.errorProneCheckApi + + errorprone deps.build.errorProne + + testCompile deps.test.junit + testCompile(deps.build.errorProneTestHelpers) { + exclude group: "junit", module: "junit" + } + testCompile project(':autodispose') + + epJavac deps.build.errorProneCheckApi +} + +shadowJar { + classifier = null +} +build.dependsOn shadowJar + +javadoc { + failOnError = false +} + +compileJava { + options.compilerArgs += ["-Xlint:unchecked", "-Werror"] +} + +test { + maxHeapSize = "1024m" + jvmArgs "-Xbootclasspath/p:${configurations.epJavac.asPath}" +} + +apply from: rootProject.file("gradle/gradle-mvn-push.gradle") + +def configurePomForShadow(pom) { + pom.scopeMappings.mappings.remove(project.configurations.compile) + pom.scopeMappings.mappings.remove(project.configurations.runtime) + pom.scopeMappings.addMapping(MavenPlugin.COMPILE_PRIORITY, project.configurations.shadow, Conf2ScopeMappingContainer.COMPILE) +} + +install { + repositories.mavenInstaller { + configurePomForShadow(pom) + } +} +install.dependsOn shadowJar + +uploadArchives { + repositories.mavenDeployer { + configurePomForShadow(pom) + } +} +uploadArchives.dependsOn shadowJar diff --git a/tooling/autodispose-rx-error-prone-checker/gradle.properties b/tooling/autodispose-rx-error-prone-checker/gradle.properties new file mode 100644 index 000000000..8ff8ad0c1 --- /dev/null +++ b/tooling/autodispose-rx-error-prone-checker/gradle.properties @@ -0,0 +1,19 @@ +# +# Copyright (C) 2017. Uber Technologies +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +POM_NAME=AutoDispose Rx Error Prone Checker (RxLifecycle Interop) +POM_ARTIFACT_ID=autodispose-rx-error-prone-checker +POM_PACKAGING=jar diff --git a/tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java b/tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java new file mode 100644 index 000000000..7cedd323d --- /dev/null +++ b/tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java @@ -0,0 +1,197 @@ +/* + * Copyright (C) 2017. Uber Technologies + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.uber.autodispose.rx.error.prone.checker; + +import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; +import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.method.MethodMatchers; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.tools.javac.code.Type; +import java.util.List; +import java.util.Optional; + +import static com.google.errorprone.BugPattern.Category.JDK; +import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; +import static com.google.errorprone.matchers.Matchers.instanceMethod; + +/** + * Checker for subscriptions not binding to lifecycle in components with lifecycle. + * Use -XepOpt:AutoDisposeLeakCheck to add support for custom components with lifecycle. + */ +@AutoService(BugChecker.class) +@BugPattern( + name = "RxJavaMissingAutoDisposeErrorChecker", + summary = "Always apply an Autodispose scope before subscribing", + category = JDK, + severity = ERROR +) +public final class RxJavaMissingAutoDisposeErrorChecker extends BugChecker + implements MethodInvocationTreeMatcher { + + private static final String AS = "as"; + private static final ImmutableList AS_CALL_MATCHERS; + private static final ImmutableList TO_CALL_MATCHERS; + private static final ImmutableList SUBSCRIBE_MATCHERS; + private static final String SUBSCRIBE = "subscribe"; + private static final String TO = "to"; + + private final ImmutableList classesWithLifecycle; + + public RxJavaMissingAutoDisposeErrorChecker(ErrorProneFlags flags) { + Optional> inputClasses = flags.getList("AutoDisposeLeakCheck"); + ImmutableList.Builder builder = new ImmutableList.Builder<>(); + classesWithLifecycle = builder + .add("android.app.Activity") + .add("android.app.Fragment") + .add("com.uber.autodispose.LifecycleScopeProvider") + .addAll(inputClasses.orElse(ImmutableList.of())) + .build(); + } + + static { + ImmutableList.Builder builder = new ImmutableList.Builder<>(); + builder + .add(instanceMethod().onDescendantOf("io.reactivex.Single").named(TO)) + .add(instanceMethod().onDescendantOf("io.reactivex.Observable").named(TO)) + .add(instanceMethod().onDescendantOf("io.reactivex.Completable").named(TO)) + .add(instanceMethod().onDescendantOf("io.reactivex.Flowable").named(TO)) + .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(TO)); + TO_CALL_MATCHERS = builder.build(); + + ImmutableList.Builder builder2 + = new ImmutableList.Builder<>(); + builder2 + .add(instanceMethod().onDescendantOf("io.reactivex.Single").named(AS)) + .add(instanceMethod().onDescendantOf("io.reactivex.Observable").named(AS)) + .add(instanceMethod().onDescendantOf("io.reactivex.Completable").named(AS)) + .add(instanceMethod().onDescendantOf("io.reactivex.Flowable").named(AS)) + .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(AS)); + AS_CALL_MATCHERS = builder2.build(); + + ImmutableList.Builder builder3 = + new ImmutableList.Builder<>(); + builder3 + .add(instanceMethod().onDescendantOf("io.reactivex.Single").named(SUBSCRIBE)) + .add(instanceMethod().onDescendantOf("io.reactivex.Observable").named(SUBSCRIBE)) + .add(instanceMethod().onDescendantOf("io.reactivex.Completable").named(SUBSCRIBE)) + .add(instanceMethod().onDescendantOf("io.reactivex.Flowable").named(SUBSCRIBE)) + .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(SUBSCRIBE)); + SUBSCRIBE_MATCHERS = builder3.build(); + } + + private static final Matcher METHOD_NAME_MATCHERS = + new Matcher() { + @Override + public boolean matches(ExpressionTree tree, VisitorState state) { + if (!(tree instanceof MethodInvocationTree)) { + return false; + } + MethodInvocationTree invTree = (MethodInvocationTree) tree; + + final MemberSelectTree memberTree = (MemberSelectTree) invTree.getMethodSelect(); + if (!memberTree.getIdentifier().contentEquals(TO)) { + return false; + } + + for (MethodMatchers.MethodNameMatcher nameMatcher : TO_CALL_MATCHERS) { + if (nameMatcher.matches(invTree, state)) { + ExpressionTree arg = invTree.getArguments().get(0); + final Type scoper = state.getTypeFromString("com.uber.autodispose.Scoper"); + return ASTHelpers.isSubtype(ASTHelpers.getType(arg), scoper, state); + } + } + + for (MethodMatchers.MethodNameMatcher nameMatcher : AS_CALL_MATCHERS) { + if (nameMatcher.matches(invTree, state)) { + ExpressionTree arg = invTree.getArguments().get(0); + final Type scoper = state + .getTypeFromString("com.uber.autodispose.AutoDisposeConverter"); + return ASTHelpers.isSubtype(ASTHelpers.getType(arg), scoper, state); + } + } + return false; + } + }; + + private static Matcher matcher(List classesWithLifecycle) { + return new Matcher() { + @Override + public boolean matches(MethodInvocationTree tree, VisitorState state) { + + boolean matchFound = false; + try { + final MemberSelectTree memberTree = (MemberSelectTree) tree.getMethodSelect(); + if (!memberTree.getIdentifier().contentEquals(SUBSCRIBE)) { + return false; + } + + for (MethodMatchers.MethodNameMatcher nameMatcher : SUBSCRIBE_MATCHERS) { + if (!nameMatcher.matches(tree, state)) { + continue; + } else { + matchFound = true; + break; + } + } + if (!matchFound) { + return false; + } + + ClassTree enclosingClass = + ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class); + Type.ClassType enclosingClassType = ASTHelpers.getType(enclosingClass); + + for (String s : classesWithLifecycle) { + Type lifecycleType = state.getTypeFromString(s); + if (ASTHelpers.isSubtype(enclosingClassType, lifecycleType, state)) { + return !METHOD_NAME_MATCHERS.matches(memberTree.getExpression(), state); + } + } + return false; + } catch (ClassCastException e) { + return false; + } + } + }; + } + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (matcher(classesWithLifecycle).matches(tree, state)) { + return buildDescription(tree).build(); + } else { + return Description.NO_MATCH; + } + } + + @Override + public String linkUrl() { + return "https://github.com/uber/RIBs" + + "/blob/memory_leaks_module/android/demos/memory-leaks/README.md"; + } +} diff --git a/tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/ComponentWithLifeCycle.java b/tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/ComponentWithLifeCycle.java new file mode 100644 index 000000000..f5adec392 --- /dev/null +++ b/tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/ComponentWithLifeCycle.java @@ -0,0 +1,19 @@ +/* + * Copyright (C) 2017. Uber Technologies + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.uber.autodispose.rx.error.prone.checker; + +public class ComponentWithLifeCycle { } diff --git a/tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorCheckerTest.java b/tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorCheckerTest.java new file mode 100644 index 000000000..ade216ad8 --- /dev/null +++ b/tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorCheckerTest.java @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2017. Uber Technologies + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.uber.autodispose.rx.error.prone.checker; + +import com.google.errorprone.CompilationTestHelper; +import java.util.Arrays; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +@SuppressWarnings("CheckTestExtendsBaseClass") +public class RxJavaMissingAutoDisposeErrorCheckerTest { + + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private CompilationTestHelper compilationHelper; + + @Before + public void setup() { + compilationHelper = + CompilationTestHelper + .newInstance(RxJavaMissingAutoDisposeErrorChecker.class, getClass()); + compilationHelper.setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:AutoDisposeLeakCheck" + + "=com.uber.autodispose.rx.error.prone.checker.ComponentWithLifeCycle")); + } + + @Test + public void test_autodisposePositiveCases() { + compilationHelper.addSourceFile("MissingAutoDisposeErrorPositiveCases.java").doTest(); + } + + @Test + public void test_autodisposePositiveCases2() { + compilationHelper.addSourceFile("MissingAutoDisposeErrorPositiveCases2.java").doTest(); + } + + @Test + public void test_autodisposeNegativeCases() { + compilationHelper.addSourceFile("MissingAutoDisposeErrorNegativeCases.java").doTest(); + } +} diff --git a/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java b/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java new file mode 100644 index 000000000..e030e5a4e --- /dev/null +++ b/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java @@ -0,0 +1,123 @@ +/* + * Copyright (C) 2017. Uber Technologies + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.uber.errorprone.checker.rx.testdata; + +import com.uber.autodispose.AutoDispose; +import com.uber.autodispose.AutoDisposeConverter; +import com.uber.autodispose.CompletableScoper; +import com.uber.autodispose.FlowableScoper; +import com.uber.autodispose.LifecycleEndedException; +import com.uber.autodispose.MaybeScoper; +import com.uber.autodispose.ObservableScoper; +import com.uber.autodispose.SingleScoper; +import com.uber.autodispose.LifecycleScopeProvider; +import com.uber.autodispose.TestLifecycleScopeProvider; + +import io.reactivex.Completable; +import io.reactivex.Flowable; +import io.reactivex.Maybe; +import io.reactivex.Observable; +import io.reactivex.Single; +import io.reactivex.annotations.CheckReturnValue; +import io.reactivex.functions.Function; +import io.reactivex.subjects.BehaviorSubject; +import javax.annotation.Nullable; + +/** + * Cases that use autodispose and should not fail the MissingAutodisposeError check. + */ +public class MissingAutoDisposeErrorNegativeCases + implements LifecycleScopeProvider { + + private final BehaviorSubject lifecycleSubject = + BehaviorSubject.create(); + + /** + * @return a sequence of lifecycle events. + */ + @CheckReturnValue public Observable lifecycle() { + return lifecycleSubject.hide(); + } + + /** + * @return a sequence of lifecycle events. It's recommended to back this with a static instance to + * avoid unnecessary object allocation. + */ + @CheckReturnValue + public Function correspondingEvents() { + return new Function() { + @Override public TestLifecycleScopeProvider.TestLifecycle apply( + TestLifecycleScopeProvider.TestLifecycle testLifecycle) { + switch (testLifecycle) { + case STARTED: + return TestLifecycleScopeProvider.TestLifecycle.STOPPED; + case STOPPED: + throw new LifecycleEndedException(); + default: + throw new IllegalStateException("Unknown lifecycle event."); + } + } + }; + } + + /** + * @return the last seen lifecycle event, or {@code null} if none. + */ + @Nullable public TestLifecycleScopeProvider.TestLifecycle peekLifecycle() { + return lifecycleSubject.getValue(); + } + + public void observable_subscribeWithTo() { + Observable.just(1).to(new ObservableScoper(this)).subscribe(); + } + + public void single_subscribeWithTo() { + Single.just(true).to(new SingleScoper(this)).subscribe(); + } + + public void completable_subscribeWithTo() { + Completable.complete().to(new CompletableScoper(this)).subscribe(); + } + + public void maybe_subscribeWithTo() { + Maybe.just(1).to(new MaybeScoper(this)).subscribe(); + } + + public void flowable_subscribeWithTo() { + Flowable.just(1).to(new FlowableScoper(this)).subscribe(); + } + + public void observable_subscribeWithAs() { + Observable.just(1).as(AutoDispose.autoDisposable(this)).subscribe(); + } + + public void single_subscribeWithAs() { + Single.just(true).as(AutoDispose.autoDisposable(this)).subscribe(); + } + + public void completable_subscribeWithAs() { + Completable.complete().as(AutoDispose.autoDisposable(this)).subscribe(); + } + + public void maybe_subscribeWithAs() { + Maybe.just(1).as(AutoDispose.autoDisposable(this)).subscribe(); + } + + public void flowable_subscribeWithAs() { + Flowable.just(1).as(AutoDispose.autoDisposable(this)).subscribe(); + } +} diff --git a/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java b/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java new file mode 100644 index 000000000..119b9bcd6 --- /dev/null +++ b/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java @@ -0,0 +1,101 @@ +/* + * Copyright (C) 2017. Uber Technologies + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.uber.autodispose.rx.error.prone.checker; + +import com.uber.autodispose.LifecycleEndedException; +import com.uber.autodispose.LifecycleScopeProvider; +import com.uber.autodispose.TestLifecycleScopeProvider; + +import io.reactivex.Completable; +import io.reactivex.Flowable; +import io.reactivex.Maybe; +import io.reactivex.Observable; +import io.reactivex.Single; +import io.reactivex.annotations.CheckReturnValue; +import io.reactivex.functions.Function; +import io.reactivex.subjects.BehaviorSubject; +import javax.annotation.Nullable; + +/** + * Cases that don't use autodispose and should fail the MissingAutodisposeError check. + */ +public class MissingAutoDisposeErrorPositiveCases + implements LifecycleScopeProvider { + + private final BehaviorSubject lifecycleSubject = + BehaviorSubject.create(); + + /** + * @return a sequence of lifecycle events. + */ + @CheckReturnValue public Observable lifecycle() { + return lifecycleSubject.hide(); + } + + /** + * @return a sequence of lifecycle events. It's recommended to back this with a static instance to + * avoid unnecessary object allocation. + */ + @CheckReturnValue + public Function correspondingEvents() { + return new Function() { + @Override public TestLifecycleScopeProvider.TestLifecycle apply( + TestLifecycleScopeProvider.TestLifecycle testLifecycle) { + switch (testLifecycle) { + case STARTED: + return TestLifecycleScopeProvider.TestLifecycle.STOPPED; + case STOPPED: + throw new LifecycleEndedException(); + default: + throw new IllegalStateException("Unknown lifecycle event."); + } + } + }; + } + + /** + * @return the last seen lifecycle event, or {@code null} if none. + */ + @Nullable public TestLifecycleScopeProvider.TestLifecycle peekLifecycle() { + return lifecycleSubject.getValue(); + } + + public void observable_subscribeWithoutAutodispose() { + // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + Observable.empty().subscribe(); + } + + public void single_subscribeWithoutAutodispose() { + // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + Single.just(true).subscribe(); + } + + public void completable_subscribeWithoutAutodispose() { + // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + Completable.complete().subscribe(); + } + + public void maybe_subscribeWithoutAutodispose() { + // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + Maybe.empty().subscribe(); + } + + public void flowable_subscribeWithoutAutodispose() { + // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + Flowable.empty().subscribe(); + } +} diff --git a/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java b/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java new file mode 100644 index 000000000..477fd2180 --- /dev/null +++ b/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2017. Uber Technologies + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.uber.autodispose.rx.error.prone.checker; + +import com.uber.autodispose.ObservableScoper; +import io.reactivex.Completable; +import io.reactivex.Flowable; +import io.reactivex.Maybe; +import io.reactivex.Observable; +import io.reactivex.Single; + +public class MissingAutoDisposeErrorPositiveCases2 extends ComponentWithLifeCycle { + public void observable_subscribeWithoutAutodispose() { + // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + Observable.empty().subscribe(); + } + + public void single_subscribeWithoutAutodispose() { + // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + Single.just(true).subscribe(); + } + + public void completable_subscribeWithoutAutodispose() { + // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + Completable.complete().subscribe(); + } + + public void maybe_subscribeWithoutAutodispose() { + // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + Maybe.empty().subscribe(); + } + + public void flowable_subscribeWithoutAutodispose() { + // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + Flowable.empty().subscribe(); + } +} From 456ed87bf710d674e6d14f246ce7ef0269ae40fc Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Fri, 5 Jan 2018 19:24:32 +0530 Subject: [PATCH 02/26] Removed shadow plugin and other unused configs from build.gradle --- gradle/dependencies.gradle | 4 +- .../build.gradle | 42 +------------------ .../RxJavaMissingAutoDisposeErrorChecker.java | 4 ++ 3 files changed, 8 insertions(+), 42 deletions(-) diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index e2eeadd48..d9c29e333 100755 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -25,7 +25,7 @@ def versions = [ ] def apt = [ - autoService : "com.google.auto.service:auto-service:1.0-rc3", + autoService : "com.google.auto.service:auto-service:1.0-rc4", ] def build = [ @@ -36,8 +36,8 @@ def build = [ targetSdkVersion: 26, checkerFramework: 'org.checkerframework:dataflow:2.3.0', - errorProneCheckApi: "com.google.errorprone:error_prone_check_api:${versions.errorProne}", errorProne: "com.google.errorprone:error_prone_core:${versions.errorProne}", + errorProneCheckApi: "com.google.errorprone:error_prone_check_api:${versions.errorProne}", errorProneTestHelpers: "com.google.errorprone:error_prone_test_helpers:${versions.errorProne}", nullAway: 'com.uber.nullaway:nullaway:0.2.0', diff --git a/tooling/autodispose-rx-error-prone-checker/build.gradle b/tooling/autodispose-rx-error-prone-checker/build.gradle index fc4b8bd40..92796c5e3 100644 --- a/tooling/autodispose-rx-error-prone-checker/build.gradle +++ b/tooling/autodispose-rx-error-prone-checker/build.gradle @@ -1,7 +1,5 @@ plugins { id "net.ltgt.errorprone" version "0.0.13" - id "com.github.johnrengelman.shadow" version "2.0.1" - id "com.github.johnrengelman.plugin-shadow" version "2.0.0" id "java" } @@ -9,8 +7,8 @@ plugins { // stick it in the bootclasspath when running tests configurations.maybeCreate("epJavac") -sourceCompatibility = "1.8" -targetCompatibility = "1.8" +sourceCompatibility = 1.8 +targetCompatibility = 1.8 dependencies { compileOnly deps.apt.autoService @@ -18,8 +16,6 @@ dependencies { compileOnly deps.build.errorProneCheckApi - errorprone deps.build.errorProne - testCompile deps.test.junit testCompile(deps.build.errorProneTestHelpers) { exclude group: "junit", module: "junit" @@ -29,42 +25,8 @@ dependencies { epJavac deps.build.errorProneCheckApi } -shadowJar { - classifier = null -} -build.dependsOn shadowJar - -javadoc { - failOnError = false -} - -compileJava { - options.compilerArgs += ["-Xlint:unchecked", "-Werror"] -} - test { - maxHeapSize = "1024m" jvmArgs "-Xbootclasspath/p:${configurations.epJavac.asPath}" } apply from: rootProject.file("gradle/gradle-mvn-push.gradle") - -def configurePomForShadow(pom) { - pom.scopeMappings.mappings.remove(project.configurations.compile) - pom.scopeMappings.mappings.remove(project.configurations.runtime) - pom.scopeMappings.addMapping(MavenPlugin.COMPILE_PRIORITY, project.configurations.shadow, Conf2ScopeMappingContainer.COMPILE) -} - -install { - repositories.mavenInstaller { - configurePomForShadow(pom) - } -} -install.dependsOn shadowJar - -uploadArchives { - repositories.mavenDeployer { - configurePomForShadow(pom) - } -} -uploadArchives.dependsOn shadowJar diff --git a/tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java b/tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java index 7cedd323d..fa5e806f6 100644 --- a/tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java +++ b/tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java @@ -62,6 +62,10 @@ public final class RxJavaMissingAutoDisposeErrorChecker extends BugChecker private final ImmutableList classesWithLifecycle; + public RxJavaMissingAutoDisposeErrorChecker() { + this(ErrorProneFlags.empty()); + } + public RxJavaMissingAutoDisposeErrorChecker(ErrorProneFlags flags) { Optional> inputClasses = flags.getList("AutoDisposeLeakCheck"); ImmutableList.Builder builder = new ImmutableList.Builder<>(); From db11fb259b35d8225eb935931170bb4c993fded1 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Fri, 5 Jan 2018 19:47:12 +0530 Subject: [PATCH 03/26] Renamed package and module name to remove rx --- settings.gradle | 2 +- .../build.gradle | 0 .../gradle.properties | 4 ++-- .../prone/checker/MissingAutoDisposeErrorChecker.java} | 10 +++++----- .../error/prone/checker/ComponentWithLifeCycle.java | 2 +- .../checker/MissingAutoDisposeErrorCheckerTest.java} | 8 ++++---- .../checker/MissingAutoDisposeErrorNegativeCases.java | 2 +- .../checker/MissingAutoDisposeErrorPositiveCases.java | 2 +- .../checker/MissingAutoDisposeErrorPositiveCases2.java | 2 +- 9 files changed, 16 insertions(+), 16 deletions(-) rename tooling/{autodispose-rx-error-prone-checker => autodispose-error-prone-checker}/build.gradle (100%) rename tooling/{autodispose-rx-error-prone-checker => autodispose-error-prone-checker}/gradle.properties (83%) rename tooling/{autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java => autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java} (96%) rename tooling/{autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx => autodispose-error-prone-checker/src/test/java/com/uber/autodispose}/error/prone/checker/ComponentWithLifeCycle.java (92%) rename tooling/{autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorCheckerTest.java => autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorCheckerTest.java} (86%) rename tooling/{autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx => autodispose-error-prone-checker/src/test/resources/com/uber/autodispose}/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java (98%) rename tooling/{autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx => autodispose-error-prone-checker/src/test/resources/com/uber/autodispose}/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java (98%) rename tooling/{autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx => autodispose-error-prone-checker/src/test/resources/com/uber/autodispose}/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java (97%) diff --git a/settings.gradle b/settings.gradle index 6da674c06..303b396f6 100755 --- a/settings.gradle +++ b/settings.gradle @@ -26,5 +26,5 @@ include ':autodispose-kotlin' include 'autodispose-rxlifecycle' include ':sample' include ':test-utils' -include ':tooling:autodispose-rx-error-prone-checker' +include ':tooling:autodispose-error-prone-checker' diff --git a/tooling/autodispose-rx-error-prone-checker/build.gradle b/tooling/autodispose-error-prone-checker/build.gradle similarity index 100% rename from tooling/autodispose-rx-error-prone-checker/build.gradle rename to tooling/autodispose-error-prone-checker/build.gradle diff --git a/tooling/autodispose-rx-error-prone-checker/gradle.properties b/tooling/autodispose-error-prone-checker/gradle.properties similarity index 83% rename from tooling/autodispose-rx-error-prone-checker/gradle.properties rename to tooling/autodispose-error-prone-checker/gradle.properties index 8ff8ad0c1..e254e1781 100644 --- a/tooling/autodispose-rx-error-prone-checker/gradle.properties +++ b/tooling/autodispose-error-prone-checker/gradle.properties @@ -14,6 +14,6 @@ # limitations under the License. # -POM_NAME=AutoDispose Rx Error Prone Checker (RxLifecycle Interop) -POM_ARTIFACT_ID=autodispose-rx-error-prone-checker +POM_NAME=AutoDispose Error-Prone Checker +POM_ARTIFACT_ID=autodispose-error-prone-checker POM_PACKAGING=jar diff --git a/tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java similarity index 96% rename from tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java rename to tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java index fa5e806f6..5a7fb6684 100644 --- a/tooling/autodispose-rx-error-prone-checker/src/main/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.uber.autodispose.rx.error.prone.checker; +package com.uber.autodispose.error.prone.checker; import com.google.auto.service.AutoService; import com.google.common.collect.ImmutableList; @@ -45,12 +45,12 @@ */ @AutoService(BugChecker.class) @BugPattern( - name = "RxJavaMissingAutoDisposeErrorChecker", + name = "MissingAutoDisposeErrorChecker", summary = "Always apply an Autodispose scope before subscribing", category = JDK, severity = ERROR ) -public final class RxJavaMissingAutoDisposeErrorChecker extends BugChecker +public final class MissingAutoDisposeErrorChecker extends BugChecker implements MethodInvocationTreeMatcher { private static final String AS = "as"; @@ -62,11 +62,11 @@ public final class RxJavaMissingAutoDisposeErrorChecker extends BugChecker private final ImmutableList classesWithLifecycle; - public RxJavaMissingAutoDisposeErrorChecker() { + public MissingAutoDisposeErrorChecker() { this(ErrorProneFlags.empty()); } - public RxJavaMissingAutoDisposeErrorChecker(ErrorProneFlags flags) { + public MissingAutoDisposeErrorChecker(ErrorProneFlags flags) { Optional> inputClasses = flags.getList("AutoDisposeLeakCheck"); ImmutableList.Builder builder = new ImmutableList.Builder<>(); classesWithLifecycle = builder diff --git a/tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/ComponentWithLifeCycle.java b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/ComponentWithLifeCycle.java similarity index 92% rename from tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/ComponentWithLifeCycle.java rename to tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/ComponentWithLifeCycle.java index f5adec392..880fdd9b5 100644 --- a/tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/ComponentWithLifeCycle.java +++ b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/ComponentWithLifeCycle.java @@ -14,6 +14,6 @@ * limitations under the License. */ -package com.uber.autodispose.rx.error.prone.checker; +package com.uber.autodispose.error.prone.checker; public class ComponentWithLifeCycle { } diff --git a/tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorCheckerTest.java b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorCheckerTest.java similarity index 86% rename from tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorCheckerTest.java rename to tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorCheckerTest.java index ade216ad8..d4a4375dd 100644 --- a/tooling/autodispose-rx-error-prone-checker/src/test/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorCheckerTest.java +++ b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorCheckerTest.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.uber.autodispose.rx.error.prone.checker; +package com.uber.autodispose.error.prone.checker; import com.google.errorprone.CompilationTestHelper; import java.util.Arrays; @@ -27,7 +27,7 @@ @RunWith(JUnit4.class) @SuppressWarnings("CheckTestExtendsBaseClass") -public class RxJavaMissingAutoDisposeErrorCheckerTest { +public class MissingAutoDisposeErrorCheckerTest { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -37,13 +37,13 @@ public class RxJavaMissingAutoDisposeErrorCheckerTest { public void setup() { compilationHelper = CompilationTestHelper - .newInstance(RxJavaMissingAutoDisposeErrorChecker.class, getClass()); + .newInstance(MissingAutoDisposeErrorChecker.class, getClass()); compilationHelper.setArgs( Arrays.asList( "-d", temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:AutoDisposeLeakCheck" - + "=com.uber.autodispose.rx.error.prone.checker.ComponentWithLifeCycle")); + + "=com.uber.autodispose.error.prone.checker.ComponentWithLifeCycle")); } @Test diff --git a/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java similarity index 98% rename from tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java rename to tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java index e030e5a4e..6494b4a00 100644 --- a/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java +++ b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.uber.errorprone.checker.rx.testdata; +package com.uber.autodispose.error.prone.checker; import com.uber.autodispose.AutoDispose; import com.uber.autodispose.AutoDisposeConverter; diff --git a/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java similarity index 98% rename from tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java rename to tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java index 119b9bcd6..8ef346f87 100644 --- a/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java +++ b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.uber.autodispose.rx.error.prone.checker; +package com.uber.autodispose.error.prone.checker; import com.uber.autodispose.LifecycleEndedException; import com.uber.autodispose.LifecycleScopeProvider; diff --git a/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java similarity index 97% rename from tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java rename to tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java index 477fd2180..dcdf5b5b4 100644 --- a/tooling/autodispose-rx-error-prone-checker/src/test/resources/com/uber/autodispose/rx/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java +++ b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.uber.autodispose.rx.error.prone.checker; +package com.uber.autodispose.error.prone.checker; import com.uber.autodispose.ObservableScoper; import io.reactivex.Completable; From 872e71e2854b2b3ea53b78edffc063a8710508c6 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Fri, 5 Jan 2018 20:00:50 +0530 Subject: [PATCH 04/26] Removed to based api and inlined the builders --- .../MissingAutoDisposeErrorChecker.java | 37 ++++--------------- .../MissingAutoDisposeErrorNegativeCases.java | 20 ---------- 2 files changed, 7 insertions(+), 50 deletions(-) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java index 5a7fb6684..6823e3095 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java @@ -55,10 +55,8 @@ public final class MissingAutoDisposeErrorChecker extends BugChecker private static final String AS = "as"; private static final ImmutableList AS_CALL_MATCHERS; - private static final ImmutableList TO_CALL_MATCHERS; private static final ImmutableList SUBSCRIBE_MATCHERS; private static final String SUBSCRIBE = "subscribe"; - private static final String TO = "to"; private final ImmutableList classesWithLifecycle; @@ -78,34 +76,21 @@ public MissingAutoDisposeErrorChecker(ErrorProneFlags flags) { } static { - ImmutableList.Builder builder = new ImmutableList.Builder<>(); - builder - .add(instanceMethod().onDescendantOf("io.reactivex.Single").named(TO)) - .add(instanceMethod().onDescendantOf("io.reactivex.Observable").named(TO)) - .add(instanceMethod().onDescendantOf("io.reactivex.Completable").named(TO)) - .add(instanceMethod().onDescendantOf("io.reactivex.Flowable").named(TO)) - .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(TO)); - TO_CALL_MATCHERS = builder.build(); - - ImmutableList.Builder builder2 - = new ImmutableList.Builder<>(); - builder2 + AS_CALL_MATCHERS = new ImmutableList.Builder() .add(instanceMethod().onDescendantOf("io.reactivex.Single").named(AS)) .add(instanceMethod().onDescendantOf("io.reactivex.Observable").named(AS)) .add(instanceMethod().onDescendantOf("io.reactivex.Completable").named(AS)) .add(instanceMethod().onDescendantOf("io.reactivex.Flowable").named(AS)) - .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(AS)); - AS_CALL_MATCHERS = builder2.build(); + .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(AS)) + .build(); - ImmutableList.Builder builder3 = - new ImmutableList.Builder<>(); - builder3 + SUBSCRIBE_MATCHERS = new ImmutableList.Builder() .add(instanceMethod().onDescendantOf("io.reactivex.Single").named(SUBSCRIBE)) .add(instanceMethod().onDescendantOf("io.reactivex.Observable").named(SUBSCRIBE)) .add(instanceMethod().onDescendantOf("io.reactivex.Completable").named(SUBSCRIBE)) .add(instanceMethod().onDescendantOf("io.reactivex.Flowable").named(SUBSCRIBE)) - .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(SUBSCRIBE)); - SUBSCRIBE_MATCHERS = builder3.build(); + .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(SUBSCRIBE)) + .build(); } private static final Matcher METHOD_NAME_MATCHERS = @@ -118,18 +103,10 @@ public boolean matches(ExpressionTree tree, VisitorState state) { MethodInvocationTree invTree = (MethodInvocationTree) tree; final MemberSelectTree memberTree = (MemberSelectTree) invTree.getMethodSelect(); - if (!memberTree.getIdentifier().contentEquals(TO)) { + if (!memberTree.getIdentifier().contentEquals(AS)) { return false; } - for (MethodMatchers.MethodNameMatcher nameMatcher : TO_CALL_MATCHERS) { - if (nameMatcher.matches(invTree, state)) { - ExpressionTree arg = invTree.getArguments().get(0); - final Type scoper = state.getTypeFromString("com.uber.autodispose.Scoper"); - return ASTHelpers.isSubtype(ASTHelpers.getType(arg), scoper, state); - } - } - for (MethodMatchers.MethodNameMatcher nameMatcher : AS_CALL_MATCHERS) { if (nameMatcher.matches(invTree, state)) { ExpressionTree arg = invTree.getArguments().get(0); diff --git a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java index 6494b4a00..7e04931c4 100644 --- a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java +++ b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java @@ -81,26 +81,6 @@ public Function(this)).subscribe(); - } - - public void single_subscribeWithTo() { - Single.just(true).to(new SingleScoper(this)).subscribe(); - } - - public void completable_subscribeWithTo() { - Completable.complete().to(new CompletableScoper(this)).subscribe(); - } - - public void maybe_subscribeWithTo() { - Maybe.just(1).to(new MaybeScoper(this)).subscribe(); - } - - public void flowable_subscribeWithTo() { - Flowable.just(1).to(new FlowableScoper(this)).subscribe(); - } - public void observable_subscribeWithAs() { Observable.just(1).as(AutoDispose.autoDisposable(this)).subscribe(); } From f876a672c8a5689b6441c0c8c7983b77efa67a8c Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Fri, 5 Jan 2018 23:53:11 +0530 Subject: [PATCH 05/26] Fixes in the checker - caching matcher --- .../MissingAutoDisposeErrorChecker.java | 62 +++++++++---------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java index 6823e3095..62ef5fed3 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java @@ -35,7 +35,6 @@ import java.util.List; import java.util.Optional; -import static com.google.errorprone.BugPattern.Category.JDK; import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; import static com.google.errorprone.matchers.Matchers.instanceMethod; @@ -47,7 +46,7 @@ @BugPattern( name = "MissingAutoDisposeErrorChecker", summary = "Always apply an Autodispose scope before subscribing", - category = JDK, + tags = {BugPattern.StandardTags.CONCURRENCY}, severity = ERROR ) public final class MissingAutoDisposeErrorChecker extends BugChecker @@ -58,7 +57,7 @@ public final class MissingAutoDisposeErrorChecker extends BugChecker private static final ImmutableList SUBSCRIBE_MATCHERS; private static final String SUBSCRIBE = "subscribe"; - private final ImmutableList classesWithLifecycle; + private final Matcher matcher; public MissingAutoDisposeErrorChecker() { this(ErrorProneFlags.empty()); @@ -66,13 +65,13 @@ public MissingAutoDisposeErrorChecker() { public MissingAutoDisposeErrorChecker(ErrorProneFlags flags) { Optional> inputClasses = flags.getList("AutoDisposeLeakCheck"); - ImmutableList.Builder builder = new ImmutableList.Builder<>(); - classesWithLifecycle = builder + ImmutableList classesWithLifecycle = new ImmutableList.Builder() .add("android.app.Activity") .add("android.app.Fragment") .add("com.uber.autodispose.LifecycleScopeProvider") .addAll(inputClasses.orElse(ImmutableList.of())) .build(); + matcher = matcher(classesWithLifecycle); } static { @@ -93,6 +92,9 @@ public MissingAutoDisposeErrorChecker(ErrorProneFlags flags) { .build(); } + /** + * Matcher to find the as operator in the observable chain. + */ private static final Matcher METHOD_NAME_MATCHERS = new Matcher() { @Override @@ -125,45 +127,41 @@ private static Matcher matcher(List classesWithLif public boolean matches(MethodInvocationTree tree, VisitorState state) { boolean matchFound = false; - try { - final MemberSelectTree memberTree = (MemberSelectTree) tree.getMethodSelect(); - if (!memberTree.getIdentifier().contentEquals(SUBSCRIBE)) { - return false; - } + final MemberSelectTree memberTree = (MemberSelectTree) tree.getMethodSelect(); + if (!memberTree.getIdentifier().contentEquals(SUBSCRIBE)) { + return false; + } - for (MethodMatchers.MethodNameMatcher nameMatcher : SUBSCRIBE_MATCHERS) { - if (!nameMatcher.matches(tree, state)) { - continue; - } else { - matchFound = true; - break; - } - } - if (!matchFound) { - return false; + for (MethodMatchers.MethodNameMatcher nameMatcher : SUBSCRIBE_MATCHERS) { + if (!nameMatcher.matches(tree, state)) { + continue; + } else { + matchFound = true; + break; } + } + if (!matchFound) { + return false; + } - ClassTree enclosingClass = - ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class); - Type.ClassType enclosingClassType = ASTHelpers.getType(enclosingClass); + ClassTree enclosingClass = + ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class); + Type.ClassType enclosingClassType = ASTHelpers.getType(enclosingClass); - for (String s : classesWithLifecycle) { - Type lifecycleType = state.getTypeFromString(s); - if (ASTHelpers.isSubtype(enclosingClassType, lifecycleType, state)) { - return !METHOD_NAME_MATCHERS.matches(memberTree.getExpression(), state); - } + for (String s : classesWithLifecycle) { + Type lifecycleType = state.getTypeFromString(s); + if (ASTHelpers.isSubtype(enclosingClassType, lifecycleType, state)) { + return !METHOD_NAME_MATCHERS.matches(memberTree.getExpression(), state); } - return false; - } catch (ClassCastException e) { - return false; } + return false; } }; } @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { - if (matcher(classesWithLifecycle).matches(tree, state)) { + if (matcher.matches(tree, state)) { return buildDescription(tree).build(); } else { return Description.NO_MATCH; From 93df2cda542e456ee19d07e6237076071887c839 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sat, 6 Jan 2018 08:36:12 +0530 Subject: [PATCH 06/26] Updated link url to point to a shortcut to wiki/readmen on autodispose --- .../error/prone/checker/MissingAutoDisposeErrorChecker.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java index 62ef5fed3..571123f70 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java @@ -170,7 +170,6 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState @Override public String linkUrl() { - return "https://github.com/uber/RIBs" - + "/blob/memory_leaks_module/android/demos/memory-leaks/README.md"; + return "http://t.uber.com/autodispose-leak-checker"; } } From 47c35ce452fe3965ac92bf3240679f65dc5ffddc Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 7 Jan 2018 09:12:32 +0530 Subject: [PATCH 07/26] Renamed test to AutoDisposeLeakCheck --- ...orChecker.java => AutoDisposeLeakChecker.java} | 8 ++++---- ...rTest.java => AutoDisposeLeakCheckerTest.java} | 15 +++++++-------- ...sposeLeakCheckerCustomClassPositiveCases.java} | 2 +- ...poseLeakCheckerDefaultClassPositiveCases.java} | 2 +- ...a => AutoDisposeLeakCheckerNegativeCases.java} | 2 +- 5 files changed, 14 insertions(+), 15 deletions(-) rename tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/{MissingAutoDisposeErrorChecker.java => AutoDisposeLeakChecker.java} (96%) rename tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/{MissingAutoDisposeErrorCheckerTest.java => AutoDisposeLeakCheckerTest.java} (73%) rename tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/{MissingAutoDisposeErrorPositiveCases2.java => AutoDisposeLeakCheckerCustomClassPositiveCases.java} (94%) rename tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/{MissingAutoDisposeErrorPositiveCases.java => AutoDisposeLeakCheckerDefaultClassPositiveCases.java} (98%) rename tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/{MissingAutoDisposeErrorNegativeCases.java => AutoDisposeLeakCheckerNegativeCases.java} (98%) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java similarity index 96% rename from tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java rename to tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index 571123f70..07acd61eb 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -44,12 +44,12 @@ */ @AutoService(BugChecker.class) @BugPattern( - name = "MissingAutoDisposeErrorChecker", + name = "AutoDisposeLeakChecker", summary = "Always apply an Autodispose scope before subscribing", tags = {BugPattern.StandardTags.CONCURRENCY}, severity = ERROR ) -public final class MissingAutoDisposeErrorChecker extends BugChecker +public final class AutoDisposeLeakChecker extends BugChecker implements MethodInvocationTreeMatcher { private static final String AS = "as"; @@ -59,11 +59,11 @@ public final class MissingAutoDisposeErrorChecker extends BugChecker private final Matcher matcher; - public MissingAutoDisposeErrorChecker() { + public AutoDisposeLeakChecker() { this(ErrorProneFlags.empty()); } - public MissingAutoDisposeErrorChecker(ErrorProneFlags flags) { + public AutoDisposeLeakChecker(ErrorProneFlags flags) { Optional> inputClasses = flags.getList("AutoDisposeLeakCheck"); ImmutableList classesWithLifecycle = new ImmutableList.Builder() .add("android.app.Activity") diff --git a/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorCheckerTest.java b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java similarity index 73% rename from tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorCheckerTest.java rename to tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java index d4a4375dd..49d4bff60 100644 --- a/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorCheckerTest.java +++ b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java @@ -26,8 +26,7 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -@SuppressWarnings("CheckTestExtendsBaseClass") -public class MissingAutoDisposeErrorCheckerTest { +public class AutoDisposeLeakCheckerTest { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -37,7 +36,7 @@ public class MissingAutoDisposeErrorCheckerTest { public void setup() { compilationHelper = CompilationTestHelper - .newInstance(MissingAutoDisposeErrorChecker.class, getClass()); + .newInstance(AutoDisposeLeakChecker.class, getClass()); compilationHelper.setArgs( Arrays.asList( "-d", @@ -47,17 +46,17 @@ public void setup() { } @Test - public void test_autodisposePositiveCases() { - compilationHelper.addSourceFile("MissingAutoDisposeErrorPositiveCases.java").doTest(); + public void test_autodisposePositiveCasesWithDefaultClass() { + compilationHelper.addSourceFile("AutoDisposeLeakCheckerDefaultClassPositiveCases.java").doTest(); } @Test - public void test_autodisposePositiveCases2() { - compilationHelper.addSourceFile("MissingAutoDisposeErrorPositiveCases2.java").doTest(); + public void test_autodisposePositiveCaseswithCustomClass() { + compilationHelper.addSourceFile("AutoDisposeLeakCheckerCustomClassPositiveCases.java").doTest(); } @Test public void test_autodisposeNegativeCases() { - compilationHelper.addSourceFile("MissingAutoDisposeErrorNegativeCases.java").doTest(); + compilationHelper.addSourceFile("AutoDisposeLeakCheckerNegativeCases.java").doTest(); } } diff --git a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java similarity index 94% rename from tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java rename to tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java index dcdf5b5b4..54eee6a10 100644 --- a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases2.java +++ b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java @@ -23,7 +23,7 @@ import io.reactivex.Observable; import io.reactivex.Single; -public class MissingAutoDisposeErrorPositiveCases2 extends ComponentWithLifeCycle { +public class AutoDisposeLeakCheckerCustomClassPositiveCases extends ComponentWithLifeCycle { public void observable_subscribeWithoutAutodispose() { // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing Observable.empty().subscribe(); diff --git a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java similarity index 98% rename from tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java rename to tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java index 8ef346f87..c809a829e 100644 --- a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorPositiveCases.java +++ b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java @@ -33,7 +33,7 @@ /** * Cases that don't use autodispose and should fail the MissingAutodisposeError check. */ -public class MissingAutoDisposeErrorPositiveCases +public class AutoDisposeLeakCheckerDefaultClassPositiveCases implements LifecycleScopeProvider { private final BehaviorSubject lifecycleSubject = diff --git a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java similarity index 98% rename from tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java rename to tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java index 7e04931c4..604bac65a 100644 --- a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorNegativeCases.java +++ b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java @@ -40,7 +40,7 @@ /** * Cases that use autodispose and should not fail the MissingAutodisposeError check. */ -public class MissingAutoDisposeErrorNegativeCases +public class AutoDisposeLeakCheckerNegativeCases implements LifecycleScopeProvider { private final BehaviorSubject lifecycleSubject = From cf07e466c34ba52189a68b0bf97751f5a08dad37 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 7 Jan 2018 09:23:17 +0530 Subject: [PATCH 08/26] Added logic to unset default classes --- .../prone/checker/AutoDisposeLeakChecker.java | 7 +++++-- .../checker/AutoDisposeLeakCheckerTest.java | 16 ++++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index 07acd61eb..44b021886 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -65,12 +65,15 @@ public AutoDisposeLeakChecker() { public AutoDisposeLeakChecker(ErrorProneFlags flags) { Optional> inputClasses = flags.getList("AutoDisposeLeakCheck"); - ImmutableList classesWithLifecycle = new ImmutableList.Builder() + ImmutableList defaultClassesWithLifecycle = new ImmutableList.Builder() .add("android.app.Activity") .add("android.app.Fragment") .add("com.uber.autodispose.LifecycleScopeProvider") - .addAll(inputClasses.orElse(ImmutableList.of())) + .add("android.support.v4.app.Fragment") + .add("android.arch.lifecycle.LifecycleOwner") + .add("com.uber.autodispose.ScopeProvider") .build(); + ImmutableList classesWithLifecycle = inputClasses.orElse(defaultClassesWithLifecycle); matcher = matcher(classesWithLifecycle); } diff --git a/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java index 49d4bff60..37ec8e712 100644 --- a/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java +++ b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java @@ -37,21 +37,25 @@ public void setup() { compilationHelper = CompilationTestHelper .newInstance(AutoDisposeLeakChecker.class, getClass()); - compilationHelper.setArgs( - Arrays.asList( - "-d", - temporaryFolder.getRoot().getAbsolutePath(), - "-XepOpt:AutoDisposeLeakCheck" - + "=com.uber.autodispose.error.prone.checker.ComponentWithLifeCycle")); } @Test public void test_autodisposePositiveCasesWithDefaultClass() { + compilationHelper.setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath())); compilationHelper.addSourceFile("AutoDisposeLeakCheckerDefaultClassPositiveCases.java").doTest(); } @Test public void test_autodisposePositiveCaseswithCustomClass() { + compilationHelper.setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:AutoDisposeLeakCheck" + + "=com.uber.autodispose.error.prone.checker.ComponentWithLifeCycle")); compilationHelper.addSourceFile("AutoDisposeLeakCheckerCustomClassPositiveCases.java").doTest(); } From 2547c2fbb20ad46adb32b4cb7d2da8b605dcf89e Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 7 Jan 2018 09:23:57 +0530 Subject: [PATCH 09/26] Updated link url --- .../autodispose/error/prone/checker/AutoDisposeLeakChecker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index 44b021886..9fea7601d 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -173,6 +173,6 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState @Override public String linkUrl() { - return "http://t.uber.com/autodispose-leak-checker"; + return "https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker"; } } From 702b9291de122752de71964e9d3311ca0f64b3b6 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 7 Jan 2018 10:50:59 +0530 Subject: [PATCH 10/26] Used streams and lambdas to simplify code --- .../prone/checker/AutoDisposeLeakChecker.java | 80 ++++++++++--------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index 9fea7601d..cdc0503bb 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -34,6 +34,9 @@ import com.sun.tools.javac.code.Type; import java.util.List; import java.util.Optional; +import java.util.function.Function; +import java.util.function.Predicate; +import javax.swing.text.html.Option; import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; import static com.google.errorprone.matchers.Matchers.instanceMethod; @@ -112,53 +115,54 @@ public boolean matches(ExpressionTree tree, VisitorState state) { return false; } - for (MethodMatchers.MethodNameMatcher nameMatcher : AS_CALL_MATCHERS) { - if (nameMatcher.matches(invTree, state)) { - ExpressionTree arg = invTree.getArguments().get(0); - final Type scoper = state - .getTypeFromString("com.uber.autodispose.AutoDisposeConverter"); - return ASTHelpers.isSubtype(ASTHelpers.getType(arg), scoper, state); - } - } - return false; + return AS_CALL_MATCHERS + .stream() + .filter(methodNameMatcher -> methodNameMatcher.matches(invTree, state)) + .map(methodNameMatcher -> { + ExpressionTree arg = invTree.getArguments().get(0); + final Type scoper = state + .getTypeFromString("com.uber.autodispose.AutoDisposeConverter"); + return ASTHelpers.isSubtype(ASTHelpers.getType(arg), scoper, state); + }) + .filter(aBoolean -> aBoolean) + .findFirst() + .orElse(false); } }; private static Matcher matcher(List classesWithLifecycle) { - return new Matcher() { - @Override - public boolean matches(MethodInvocationTree tree, VisitorState state) { - - boolean matchFound = false; - final MemberSelectTree memberTree = (MemberSelectTree) tree.getMethodSelect(); - if (!memberTree.getIdentifier().contentEquals(SUBSCRIBE)) { - return false; - } - - for (MethodMatchers.MethodNameMatcher nameMatcher : SUBSCRIBE_MATCHERS) { - if (!nameMatcher.matches(tree, state)) { - continue; - } else { - matchFound = true; - break; - } - } - if (!matchFound) { - return false; - } + return (Matcher) (tree, state) -> { - ClassTree enclosingClass = - ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class); - Type.ClassType enclosingClassType = ASTHelpers.getType(enclosingClass); + boolean matchFound = false; + final MemberSelectTree memberTree = (MemberSelectTree) tree.getMethodSelect(); + if (!memberTree.getIdentifier().contentEquals(SUBSCRIBE)) { + return false; + } - for (String s : classesWithLifecycle) { - Type lifecycleType = state.getTypeFromString(s); - if (ASTHelpers.isSubtype(enclosingClassType, lifecycleType, state)) { - return !METHOD_NAME_MATCHERS.matches(memberTree.getExpression(), state); - } + for (MethodMatchers.MethodNameMatcher nameMatcher : SUBSCRIBE_MATCHERS) { + if (nameMatcher.matches(tree, state)) { + matchFound = true; + break; } + } + if (!matchFound) { return false; } + + ClassTree enclosingClass = + ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class); + Type.ClassType enclosingClassType = ASTHelpers.getType(enclosingClass); + + return classesWithLifecycle + .stream() + .map(s -> { + Type lifecycleType = state.getTypeFromString(s); + return ASTHelpers.isSubtype(enclosingClassType, lifecycleType, state) + && !METHOD_NAME_MATCHERS.matches(memberTree.getExpression(), state); + }) + .filter(aBoolean -> aBoolean) + .findFirst() + .orElse(false); }; } From e1703ff3b25e4207dbc21054d8d2e1d31b4e0a09 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 7 Jan 2018 11:04:16 +0530 Subject: [PATCH 11/26] Fixed javadoc and build errors --- .../error/prone/checker/AutoDisposeLeakChecker.java | 9 +++++---- .../prone/checker/AutoDisposeLeakCheckerTest.java | 12 +++++++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index cdc0503bb..ee047ecee 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -34,16 +34,17 @@ import com.sun.tools.javac.code.Type; import java.util.List; import java.util.Optional; -import java.util.function.Function; -import java.util.function.Predicate; -import javax.swing.text.html.Option; import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; import static com.google.errorprone.matchers.Matchers.instanceMethod; /** * Checker for subscriptions not binding to lifecycle in components with lifecycle. - * Use -XepOpt:AutoDisposeLeakCheck to add support for custom components with lifecycle. + * Use -XepOpt:AutoDisposeLeakCheck flag to add support for custom components with lifecycle. + * The sample configuration for Conductor: + * 

+ *   -XepOpt:AutoDisposeLeakCheck=com.bluelinelabs.conductor.Controller,android.app.Activity
+ *
*/ @AutoService(BugChecker.class) @BugPattern( diff --git a/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java index 37ec8e712..ab1e3c286 100644 --- a/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java +++ b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java @@ -45,7 +45,9 @@ public void test_autodisposePositiveCasesWithDefaultClass() { Arrays.asList( "-d", temporaryFolder.getRoot().getAbsolutePath())); - compilationHelper.addSourceFile("AutoDisposeLeakCheckerDefaultClassPositiveCases.java").doTest(); + compilationHelper + .addSourceFile("AutoDisposeLeakCheckerDefaultClassPositiveCases.java") + .doTest(); } @Test @@ -56,11 +58,15 @@ public void test_autodisposePositiveCaseswithCustomClass() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:AutoDisposeLeakCheck" + "=com.uber.autodispose.error.prone.checker.ComponentWithLifeCycle")); - compilationHelper.addSourceFile("AutoDisposeLeakCheckerCustomClassPositiveCases.java").doTest(); + compilationHelper + .addSourceFile("AutoDisposeLeakCheckerCustomClassPositiveCases.java") + .doTest(); } @Test public void test_autodisposeNegativeCases() { - compilationHelper.addSourceFile("AutoDisposeLeakCheckerNegativeCases.java").doTest(); + compilationHelper + .addSourceFile("AutoDisposeLeakCheckerNegativeCases.java") + .doTest(); } } From dd16a1444ff17a601ff51b4109c79272305fe837 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sat, 20 Jan 2018 14:09:39 +0530 Subject: [PATCH 12/26] Fixed a bug when the invocation is not of the correct type --- .../prone/checker/AutoDisposeLeakChecker.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index ee047ecee..c2d960ab5 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -111,7 +111,15 @@ public boolean matches(ExpressionTree tree, VisitorState state) { } MethodInvocationTree invTree = (MethodInvocationTree) tree; - final MemberSelectTree memberTree = (MemberSelectTree) invTree.getMethodSelect(); + ExpressionTree methodSelectTree = invTree.getMethodSelect(); + + // MemberSelectTree is used only for member access expression. + // This is not true for scenarios such as calling super constructor. + if (!(methodSelectTree instanceof MemberSelectTree)) { + return false; + } + + final MemberSelectTree memberTree = (MemberSelectTree) methodSelectTree; if (!memberTree.getIdentifier().contentEquals(AS)) { return false; } @@ -135,6 +143,14 @@ private static Matcher matcher(List classesWithLif return (Matcher) (tree, state) -> { boolean matchFound = false; + ExpressionTree methodSelectTree = tree.getMethodSelect(); + + // MemberSelectTree is used only for member access expression. + // This is not true for scenarios such as calling super constructor. + if (!(methodSelectTree instanceof MemberSelectTree)) { + return false; + } + final MemberSelectTree memberTree = (MemberSelectTree) tree.getMethodSelect(); if (!memberTree.getIdentifier().contentEquals(SUBSCRIBE)) { return false; From 2e8bf18dd19bd961dc150ce4197ce56542686a88 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sat, 20 Jan 2018 14:52:27 +0530 Subject: [PATCH 13/26] Rebased master and fixed naming conventions --- .../prone/checker/AutoDisposeLeakChecker.java | 4 ++- ...seLeakCheckerCustomClassPositiveCases.java | 30 ++++++++++++------ ...eLeakCheckerDefaultClassPositiveCases.java | 31 ++++++++++++------- .../AutoDisposeLeakCheckerNegativeCases.java | 21 +++++++++---- 4 files changed, 58 insertions(+), 28 deletions(-) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index c2d960ab5..46bdcc266 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -49,7 +49,7 @@ @AutoService(BugChecker.class) @BugPattern( name = "AutoDisposeLeakChecker", - summary = "Always apply an Autodispose scope before subscribing", + summary = "Always apply an AutoDispose scope before subscribing", tags = {BugPattern.StandardTags.CONCURRENCY}, severity = ERROR ) @@ -88,6 +88,7 @@ public AutoDisposeLeakChecker(ErrorProneFlags flags) { .add(instanceMethod().onDescendantOf("io.reactivex.Completable").named(AS)) .add(instanceMethod().onDescendantOf("io.reactivex.Flowable").named(AS)) .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(AS)) + .add(instanceMethod().onDescendantOf("io.reactivex.parallel.ParallelFlowable").named(AS)) .build(); SUBSCRIBE_MATCHERS = new ImmutableList.Builder() @@ -96,6 +97,7 @@ public AutoDisposeLeakChecker(ErrorProneFlags flags) { .add(instanceMethod().onDescendantOf("io.reactivex.Completable").named(SUBSCRIBE)) .add(instanceMethod().onDescendantOf("io.reactivex.Flowable").named(SUBSCRIBE)) .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(SUBSCRIBE)) + .add(instanceMethod().onDescendantOf("io.reactivex.parallel.ParallelFlowable").named(SUBSCRIBE)) .build(); } diff --git a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java index 54eee6a10..71b59a434 100644 --- a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java +++ b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java @@ -16,36 +16,46 @@ package com.uber.autodispose.error.prone.checker; +import com.uber.autodispose.AutoDispose; import com.uber.autodispose.ObservableScoper; import io.reactivex.Completable; import io.reactivex.Flowable; import io.reactivex.Maybe; import io.reactivex.Observable; import io.reactivex.Single; +import org.reactivestreams.Subscriber; public class AutoDisposeLeakCheckerCustomClassPositiveCases extends ComponentWithLifeCycle { - public void observable_subscribeWithoutAutodispose() { - // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + public void observable_subscribeWithoutAutoDispose() { + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing Observable.empty().subscribe(); } - public void single_subscribeWithoutAutodispose() { - // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + public void single_subscribeWithoutAutoDispose() { + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing Single.just(true).subscribe(); } - public void completable_subscribeWithoutAutodispose() { - // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + public void completable_subscribeWithoutAutoDispose() { + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing Completable.complete().subscribe(); } - public void maybe_subscribeWithoutAutodispose() { - // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + public void maybe_subscribeWithoutAutoDispose() { + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing Maybe.empty().subscribe(); } - public void flowable_subscribeWithoutAutodispose() { - // BUG: Diagnostic contains: Always apply an Autodispose scope before subscribing + public void flowable_subscribeWithoutAutoDispose() { + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing Flowable.empty().subscribe(); } + + public void parallelFlowable_subscribeWithoutAutoDispose() { + Subscriber[] subscribers = new Subscriber[] {}; + Flowable.just(1, 2) + .parallel(2) + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing + .subscribe(subscribers); + } } diff --git a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java index c809a829e..c2aab60a1 100644 --- a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java +++ b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java @@ -29,9 +29,10 @@ import io.reactivex.functions.Function; import io.reactivex.subjects.BehaviorSubject; import javax.annotation.Nullable; +import org.reactivestreams.Subscriber; /** - * Cases that don't use autodispose and should fail the MissingAutodisposeError check. + * Cases that don't use autodispose and should fail the {@link AutoDisposeLeakChecker} check. */ public class AutoDisposeLeakCheckerDefaultClassPositiveCases implements LifecycleScopeProvider { @@ -74,28 +75,36 @@ public Function[] subscribers = new Subscriber[] {}; + Flowable.just(1, 2) + .parallel(2) + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing + .subscribe(subscribers); + } } diff --git a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java index 604bac65a..e0a6fc24e 100644 --- a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java +++ b/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java @@ -36,9 +36,10 @@ import io.reactivex.functions.Function; import io.reactivex.subjects.BehaviorSubject; import javax.annotation.Nullable; +import org.reactivestreams.Subscriber; /** - * Cases that use autodispose and should not fail the MissingAutodisposeError check. + * Cases that use {@link AutoDispose} and should not fail the {@link AutoDisposeLeakChecker} check. */ public class AutoDisposeLeakCheckerNegativeCases implements LifecycleScopeProvider { @@ -81,23 +82,31 @@ public FunctionautoDisposable(this)).subscribe(); } - public void single_subscribeWithAs() { + public void single_subscribeWithAutoDispose() { Single.just(true).as(AutoDispose.autoDisposable(this)).subscribe(); } - public void completable_subscribeWithAs() { + public void completable_subscribeWithAutoDispose() { Completable.complete().as(AutoDispose.autoDisposable(this)).subscribe(); } - public void maybe_subscribeWithAs() { + public void maybe_subscribeWithAutoDispose() { Maybe.just(1).as(AutoDispose.autoDisposable(this)).subscribe(); } - public void flowable_subscribeWithAs() { + public void flowable_subscribeWithAutoDispose() { Flowable.just(1).as(AutoDispose.autoDisposable(this)).subscribe(); } + + public void parallelFlowable_subscribeWithAutoDispose() { + Subscriber[] subscribers = new Subscriber[] {}; + Flowable.just(1, 2) + .parallel(2) + .as(AutoDispose.autoDisposable(this)) + .subscribe(subscribers); + } } From a9bf40217b0916c4bfa759f85ef8dd86c92e63fd Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sat, 20 Jan 2018 15:01:29 +0530 Subject: [PATCH 14/26] Changed plugin type to java library and tested it on sample project --- .../autodispose-error-prone-checker/build.gradle | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tooling/autodispose-error-prone-checker/build.gradle b/tooling/autodispose-error-prone-checker/build.gradle index 92796c5e3..94f98c693 100644 --- a/tooling/autodispose-error-prone-checker/build.gradle +++ b/tooling/autodispose-error-prone-checker/build.gradle @@ -1,6 +1,6 @@ plugins { id "net.ltgt.errorprone" version "0.0.13" - id "java" + id "java-library" } // we use this config to get the path of the JDK 9 javac jar, to @@ -11,16 +11,16 @@ sourceCompatibility = 1.8 targetCompatibility = 1.8 dependencies { - compileOnly deps.apt.autoService - testCompile deps.rx.java + implementation deps.apt.autoService + testImplementation deps.rx.java - compileOnly deps.build.errorProneCheckApi + implementation deps.build.errorProneCheckApi - testCompile deps.test.junit - testCompile(deps.build.errorProneTestHelpers) { + testImplementation deps.test.junit + testImplementation(deps.build.errorProneTestHelpers) { exclude group: "junit", module: "junit" } - testCompile project(':autodispose') + testImplementation project(':autodispose') epJavac deps.build.errorProneCheckApi } From a90e3d6bd72578181761b655a8c32217b55aad9b Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sat, 20 Jan 2018 15:26:06 +0530 Subject: [PATCH 15/26] Fixed checkstyle and removed redundant params from tesT --- .../error/prone/checker/AutoDisposeLeakChecker.java | 4 +++- .../prone/checker/AutoDisposeLeakCheckerTest.java | 13 +++---------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index 46bdcc266..a780edada 100644 --- a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -97,7 +97,9 @@ public AutoDisposeLeakChecker(ErrorProneFlags flags) { .add(instanceMethod().onDescendantOf("io.reactivex.Completable").named(SUBSCRIBE)) .add(instanceMethod().onDescendantOf("io.reactivex.Flowable").named(SUBSCRIBE)) .add(instanceMethod().onDescendantOf("io.reactivex.Maybe").named(SUBSCRIBE)) - .add(instanceMethod().onDescendantOf("io.reactivex.parallel.ParallelFlowable").named(SUBSCRIBE)) + .add(instanceMethod() + .onDescendantOf("io.reactivex.parallel.ParallelFlowable") + .named(SUBSCRIBE)) .build(); } diff --git a/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java index ab1e3c286..d04f65b56 100644 --- a/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java +++ b/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java @@ -17,7 +17,7 @@ package com.uber.autodispose.error.prone.checker; import com.google.errorprone.CompilationTestHelper; -import java.util.Arrays; +import java.util.Collections; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -41,10 +41,6 @@ public void setup() { @Test public void test_autodisposePositiveCasesWithDefaultClass() { - compilationHelper.setArgs( - Arrays.asList( - "-d", - temporaryFolder.getRoot().getAbsolutePath())); compilationHelper .addSourceFile("AutoDisposeLeakCheckerDefaultClassPositiveCases.java") .doTest(); @@ -53,11 +49,8 @@ public void test_autodisposePositiveCasesWithDefaultClass() { @Test public void test_autodisposePositiveCaseswithCustomClass() { compilationHelper.setArgs( - Arrays.asList( - "-d", - temporaryFolder.getRoot().getAbsolutePath(), - "-XepOpt:AutoDisposeLeakCheck" - + "=com.uber.autodispose.error.prone.checker.ComponentWithLifeCycle")); + Collections.singletonList("-XepOpt:AutoDisposeLeakCheck" + + "=com.uber.autodispose.error.prone.checker.ComponentWithLifeCycle")); compilationHelper .addSourceFile("AutoDisposeLeakCheckerCustomClassPositiveCases.java") .doTest(); From d4b3f226fc64b7488f48e0984cd693d60c4d88ba Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sat, 20 Jan 2018 15:33:03 +0530 Subject: [PATCH 16/26] Updated readme to point to the wiki for error prone check. This section can also contains lint related details in the future --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index ea6680126..00bf52b44 100755 --- a/README.md +++ b/README.md @@ -285,6 +285,12 @@ Javadocs and KDocs for the most recent release can be found here: https://uber.g Snapshots of the development version are available in [Sonatype's snapshots repository][snapshots]. +Static code analysis +------- +There is an optional error-prone checker you can use to enforce use of AutoDispose. +Integration steps and more details +can be found on the [wiki](https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker) + License ------- From 9fd6dde0c0fe446dcde9a36da0d70605f84b5037 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 21 Jan 2018 07:14:52 +0530 Subject: [PATCH 17/26] Reverted to compileOnly and clubbed all testImplementation together --- tooling/autodispose-error-prone-checker/build.gradle | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tooling/autodispose-error-prone-checker/build.gradle b/tooling/autodispose-error-prone-checker/build.gradle index 94f98c693..bbec49884 100644 --- a/tooling/autodispose-error-prone-checker/build.gradle +++ b/tooling/autodispose-error-prone-checker/build.gradle @@ -11,15 +11,14 @@ sourceCompatibility = 1.8 targetCompatibility = 1.8 dependencies { - implementation deps.apt.autoService - testImplementation deps.rx.java - - implementation deps.build.errorProneCheckApi + compileOnly deps.apt.autoService + compileOnly deps.build.errorProneCheckApi - testImplementation deps.test.junit testImplementation(deps.build.errorProneTestHelpers) { exclude group: "junit", module: "junit" } + testImplementation deps.rx.java + testImplementation deps.test.junit testImplementation project(':autodispose') epJavac deps.build.errorProneCheckApi From 5de25a52604028f03ad5590652e0f79997a8f443 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 21 Jan 2018 07:17:55 +0530 Subject: [PATCH 18/26] Renamed tooling to static-analysis --- settings.gradle | 2 +- .../autodispose-error-prone-checker/build.gradle | 0 .../autodispose-error-prone-checker/gradle.properties | 0 .../autodispose/error/prone/checker/AutoDisposeLeakChecker.java | 0 .../error/prone/checker/AutoDisposeLeakCheckerTest.java | 0 .../autodispose/error/prone/checker/ComponentWithLifeCycle.java | 0 .../checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java | 0 .../AutoDisposeLeakCheckerDefaultClassPositiveCases.java | 0 .../prone/checker/AutoDisposeLeakCheckerNegativeCases.java | 0 9 files changed, 1 insertion(+), 1 deletion(-) rename {tooling => static-analysis}/autodispose-error-prone-checker/build.gradle (100%) rename {tooling => static-analysis}/autodispose-error-prone-checker/gradle.properties (100%) rename {tooling => static-analysis}/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java (100%) rename {tooling => static-analysis}/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java (100%) rename {tooling => static-analysis}/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/ComponentWithLifeCycle.java (100%) rename {tooling => static-analysis}/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java (100%) rename {tooling => static-analysis}/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java (100%) rename {tooling => static-analysis}/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java (100%) diff --git a/settings.gradle b/settings.gradle index 303b396f6..f18299270 100755 --- a/settings.gradle +++ b/settings.gradle @@ -26,5 +26,5 @@ include ':autodispose-kotlin' include 'autodispose-rxlifecycle' include ':sample' include ':test-utils' -include ':tooling:autodispose-error-prone-checker' +include ':static-analysis:autodispose-error-prone-checker' diff --git a/tooling/autodispose-error-prone-checker/build.gradle b/static-analysis/autodispose-error-prone-checker/build.gradle similarity index 100% rename from tooling/autodispose-error-prone-checker/build.gradle rename to static-analysis/autodispose-error-prone-checker/build.gradle diff --git a/tooling/autodispose-error-prone-checker/gradle.properties b/static-analysis/autodispose-error-prone-checker/gradle.properties similarity index 100% rename from tooling/autodispose-error-prone-checker/gradle.properties rename to static-analysis/autodispose-error-prone-checker/gradle.properties diff --git a/tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java similarity index 100% rename from tooling/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java rename to static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java diff --git a/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java b/static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java similarity index 100% rename from tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java rename to static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java diff --git a/tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/ComponentWithLifeCycle.java b/static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/ComponentWithLifeCycle.java similarity index 100% rename from tooling/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/ComponentWithLifeCycle.java rename to static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/ComponentWithLifeCycle.java diff --git a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java similarity index 100% rename from tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java rename to static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java diff --git a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java similarity index 100% rename from tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java rename to static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java diff --git a/tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java similarity index 100% rename from tooling/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java rename to static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java From c51f4db404be41f6df92cd01f5408a3d3f0e8747 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 21 Jan 2018 07:19:13 +0530 Subject: [PATCH 19/26] Fixed readme --- README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 00bf52b44..3dbcbf351 100755 --- a/README.md +++ b/README.md @@ -220,6 +220,12 @@ This pattern is *sort of* possible in RxJava 1, but only on `Subscriber` (via `o `CompletableObserver` (which matches RxJava 2's API). We are aggressively migrating our internal code to RxJava 2, and do not plan to try to backport this to RxJava 1. +Static analysis +------- +There is an optional error-prone checker you can use to enforce use of AutoDispose. +Integration steps and more details +can be found on the [wiki](https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker) + Download -------- @@ -285,12 +291,6 @@ Javadocs and KDocs for the most recent release can be found here: https://uber.g Snapshots of the development version are available in [Sonatype's snapshots repository][snapshots]. -Static code analysis -------- -There is an optional error-prone checker you can use to enforce use of AutoDispose. -Integration steps and more details -can be found on the [wiki](https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker) - License ------- From aca6ccf916721f195a136b6ffb3daef9512cf4a6 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 21 Jan 2018 07:36:05 +0530 Subject: [PATCH 20/26] Converted for loop for subscriber matcher to lambda and use method reference for filtering --- .../prone/checker/AutoDisposeLeakChecker.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index a780edada..954c9a168 100644 --- a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -137,7 +137,7 @@ public boolean matches(ExpressionTree tree, VisitorState state) { .getTypeFromString("com.uber.autodispose.AutoDisposeConverter"); return ASTHelpers.isSubtype(ASTHelpers.getType(arg), scoper, state); }) - .filter(aBoolean -> aBoolean) + .filter(Boolean::booleanValue) .findFirst() .orElse(false); } @@ -160,12 +160,13 @@ private static Matcher matcher(List classesWithLif return false; } - for (MethodMatchers.MethodNameMatcher nameMatcher : SUBSCRIBE_MATCHERS) { - if (nameMatcher.matches(tree, state)) { - matchFound = true; - break; - } - } + matchFound = SUBSCRIBE_MATCHERS + .stream() + .map(methodNameMatcher -> methodNameMatcher.matches(tree, state)) + .filter(Boolean::booleanValue) + .findFirst() + .orElse(false); + if (!matchFound) { return false; } @@ -176,12 +177,12 @@ private static Matcher matcher(List classesWithLif return classesWithLifecycle .stream() - .map(s -> { - Type lifecycleType = state.getTypeFromString(s); + .map(classWithLifecycle -> { + Type lifecycleType = state.getTypeFromString(classWithLifecycle); return ASTHelpers.isSubtype(enclosingClassType, lifecycleType, state) && !METHOD_NAME_MATCHERS.matches(memberTree.getExpression(), state); }) - .filter(aBoolean -> aBoolean) + .filter(Boolean::booleanValue) .findFirst() .orElse(false); }; From 2a9ba377463336b786b8228cdb0f67e6027851ab Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 21 Jan 2018 09:22:18 +0530 Subject: [PATCH 21/26] Newline in readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 3dbcbf351..e41995853 100755 --- a/README.md +++ b/README.md @@ -222,6 +222,7 @@ This pattern is *sort of* possible in RxJava 1, but only on `Subscriber` (via `o Static analysis ------- + There is an optional error-prone checker you can use to enforce use of AutoDispose. Integration steps and more details can be found on the [wiki](https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker) From 8308c39ff0786788795da9d35d828efc6e0cc92e Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 21 Jan 2018 09:34:22 +0530 Subject: [PATCH 22/26] Added comments to explain filtering --- .../error/prone/checker/AutoDisposeLeakChecker.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index 954c9a168..c2d35306d 100644 --- a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -137,7 +137,8 @@ public boolean matches(ExpressionTree tree, VisitorState state) { .getTypeFromString("com.uber.autodispose.AutoDisposeConverter"); return ASTHelpers.isSubtype(ASTHelpers.getType(arg), scoper, state); }) - .filter(Boolean::booleanValue) + .filter(Boolean::booleanValue) // Filtering the method invocation with name as + // and has an argument of type AutoDisposeConverter. .findFirst() .orElse(false); } @@ -163,7 +164,7 @@ private static Matcher matcher(List classesWithLif matchFound = SUBSCRIBE_MATCHERS .stream() .map(methodNameMatcher -> methodNameMatcher.matches(tree, state)) - .filter(Boolean::booleanValue) + .filter(Boolean::booleanValue) // Filtering the method invocation with name subscribe .findFirst() .orElse(false); @@ -182,7 +183,8 @@ private static Matcher matcher(List classesWithLif return ASTHelpers.isSubtype(enclosingClassType, lifecycleType, state) && !METHOD_NAME_MATCHERS.matches(memberTree.getExpression(), state); }) - .filter(Boolean::booleanValue) + .filter(Boolean::booleanValue) // Filtering the method invocation + // which is a subtype of one of the classes with lifecycle and name as .findFirst() .orElse(false); }; From 6fd81f337f882a2a7c129ec0991b12b8c5db466d Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 21 Jan 2018 09:40:02 +0530 Subject: [PATCH 23/26] Fixed comments --- .../error/prone/checker/AutoDisposeLeakChecker.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index c2d35306d..83ac57e1f 100644 --- a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -137,8 +137,9 @@ public boolean matches(ExpressionTree tree, VisitorState state) { .getTypeFromString("com.uber.autodispose.AutoDisposeConverter"); return ASTHelpers.isSubtype(ASTHelpers.getType(arg), scoper, state); }) - .filter(Boolean::booleanValue) // Filtering the method invocation with name as + // Filtering the method invocation with name as // and has an argument of type AutoDisposeConverter. + .filter(Boolean::booleanValue) .findFirst() .orElse(false); } @@ -183,8 +184,9 @@ private static Matcher matcher(List classesWithLif return ASTHelpers.isSubtype(enclosingClassType, lifecycleType, state) && !METHOD_NAME_MATCHERS.matches(memberTree.getExpression(), state); }) - .filter(Boolean::booleanValue) // Filtering the method invocation - // which is a subtype of one of the classes with lifecycle and name as + // Filtering the method invocation which is a + // subtype of one of the classes with lifecycle and name as + .filter(Boolean::booleanValue) .findFirst() .orElse(false); }; From 7965a81c1e2db4968461525f894fa4ce9a6736bd Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 21 Jan 2018 10:20:30 +0530 Subject: [PATCH 24/26] Fixed tags and fleshed out the comment --- .../error/prone/checker/AutoDisposeLeakChecker.java | 5 +++-- ...toDisposeLeakCheckerCustomClassPositiveCases.java | 10 +++++----- ...oDisposeLeakCheckerDefaultClassPositiveCases.java | 12 ++++++------ 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java index 83ac57e1f..6c18ead52 100644 --- a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java @@ -36,6 +36,7 @@ import java.util.Optional; import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; +import static com.google.errorprone.BugPattern.StandardTags.CONCURRENCY; import static com.google.errorprone.matchers.Matchers.instanceMethod; /** @@ -49,8 +50,8 @@ @AutoService(BugChecker.class) @BugPattern( name = "AutoDisposeLeakChecker", - summary = "Always apply an AutoDispose scope before subscribing", - tags = {BugPattern.StandardTags.CONCURRENCY}, + summary = "Always apply an AutoDispose scope before subscribing in lifecycle scopes", + tags = CONCURRENCY, severity = ERROR ) public final class AutoDisposeLeakChecker extends BugChecker diff --git a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java index 71b59a434..ef516cdb9 100644 --- a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java +++ b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java @@ -27,22 +27,22 @@ public class AutoDisposeLeakCheckerCustomClassPositiveCases extends ComponentWithLifeCycle { public void observable_subscribeWithoutAutoDispose() { - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes Observable.empty().subscribe(); } public void single_subscribeWithoutAutoDispose() { - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes Single.just(true).subscribe(); } public void completable_subscribeWithoutAutoDispose() { - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes Completable.complete().subscribe(); } public void maybe_subscribeWithoutAutoDispose() { - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes Maybe.empty().subscribe(); } @@ -55,7 +55,7 @@ public void parallelFlowable_subscribeWithoutAutoDispose() { Subscriber[] subscribers = new Subscriber[] {}; Flowable.just(1, 2) .parallel(2) - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes .subscribe(subscribers); } } diff --git a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java index c2aab60a1..944b8cd90 100644 --- a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java +++ b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java @@ -76,27 +76,27 @@ public Function[] subscribers = new Subscriber[] {}; Flowable.just(1, 2) .parallel(2) - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes .subscribe(subscribers); } } From 282291ba155b03cda9977e5f2bc08e06c51c8e22 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 21 Jan 2018 10:42:27 +0530 Subject: [PATCH 25/26] Renamed class to UseAutoDispose --- ...AutoDisposeLeakChecker.java => UseAutoDispose.java} | 8 ++++---- ...oseLeakCheckerTest.java => UseAutoDisposeTest.java} | 10 +++++----- ...ava => UseAutoDisposeCustomClassPositiveCases.java} | 2 +- ...va => UseAutoDisposeDefaultClassPositiveCases.java} | 4 ++-- ...tiveCases.java => UseAutoDisposeNegativeCases.java} | 10 ++-------- 5 files changed, 14 insertions(+), 20 deletions(-) rename static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/{AutoDisposeLeakChecker.java => UseAutoDispose.java} (97%) rename static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/{AutoDisposeLeakCheckerTest.java => UseAutoDisposeTest.java} (83%) rename static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/{AutoDisposeLeakCheckerCustomClassPositiveCases.java => UseAutoDisposeCustomClassPositiveCases.java} (96%) rename static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/{AutoDisposeLeakCheckerDefaultClassPositiveCases.java => UseAutoDisposeDefaultClassPositiveCases.java} (96%) rename static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/{AutoDisposeLeakCheckerNegativeCases.java => UseAutoDisposeNegativeCases.java} (91%) diff --git a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/UseAutoDispose.java similarity index 97% rename from static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java rename to static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/UseAutoDispose.java index 6c18ead52..94803055c 100644 --- a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java +++ b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/UseAutoDispose.java @@ -49,12 +49,12 @@ */ @AutoService(BugChecker.class) @BugPattern( - name = "AutoDisposeLeakChecker", + name = "UseAutoDispose", summary = "Always apply an AutoDispose scope before subscribing in lifecycle scopes", tags = CONCURRENCY, severity = ERROR ) -public final class AutoDisposeLeakChecker extends BugChecker +public final class UseAutoDispose extends BugChecker implements MethodInvocationTreeMatcher { private static final String AS = "as"; @@ -64,11 +64,11 @@ public final class AutoDisposeLeakChecker extends BugChecker private final Matcher matcher; - public AutoDisposeLeakChecker() { + public UseAutoDispose() { this(ErrorProneFlags.empty()); } - public AutoDisposeLeakChecker(ErrorProneFlags flags) { + public UseAutoDispose(ErrorProneFlags flags) { Optional> inputClasses = flags.getList("AutoDisposeLeakCheck"); ImmutableList defaultClassesWithLifecycle = new ImmutableList.Builder() .add("android.app.Activity") diff --git a/static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java b/static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/UseAutoDisposeTest.java similarity index 83% rename from static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java rename to static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/UseAutoDisposeTest.java index d04f65b56..add55cbbe 100644 --- a/static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerTest.java +++ b/static-analysis/autodispose-error-prone-checker/src/test/java/com/uber/autodispose/error/prone/checker/UseAutoDisposeTest.java @@ -26,7 +26,7 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class AutoDisposeLeakCheckerTest { +public class UseAutoDisposeTest { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -36,13 +36,13 @@ public class AutoDisposeLeakCheckerTest { public void setup() { compilationHelper = CompilationTestHelper - .newInstance(AutoDisposeLeakChecker.class, getClass()); + .newInstance(UseAutoDispose.class, getClass()); } @Test public void test_autodisposePositiveCasesWithDefaultClass() { compilationHelper - .addSourceFile("AutoDisposeLeakCheckerDefaultClassPositiveCases.java") + .addSourceFile("UseAutoDisposeDefaultClassPositiveCases.java") .doTest(); } @@ -52,14 +52,14 @@ public void test_autodisposePositiveCaseswithCustomClass() { Collections.singletonList("-XepOpt:AutoDisposeLeakCheck" + "=com.uber.autodispose.error.prone.checker.ComponentWithLifeCycle")); compilationHelper - .addSourceFile("AutoDisposeLeakCheckerCustomClassPositiveCases.java") + .addSourceFile("UseAutoDisposeCustomClassPositiveCases.java") .doTest(); } @Test public void test_autodisposeNegativeCases() { compilationHelper - .addSourceFile("AutoDisposeLeakCheckerNegativeCases.java") + .addSourceFile("UseAutoDisposeNegativeCases.java") .doTest(); } } diff --git a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeCustomClassPositiveCases.java similarity index 96% rename from static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java rename to static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeCustomClassPositiveCases.java index ef516cdb9..86f6fcbd5 100644 --- a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerCustomClassPositiveCases.java +++ b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeCustomClassPositiveCases.java @@ -25,7 +25,7 @@ import io.reactivex.Single; import org.reactivestreams.Subscriber; -public class AutoDisposeLeakCheckerCustomClassPositiveCases extends ComponentWithLifeCycle { +public class UseAutoDisposeCustomClassPositiveCases extends ComponentWithLifeCycle { public void observable_subscribeWithoutAutoDispose() { // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes Observable.empty().subscribe(); diff --git a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeDefaultClassPositiveCases.java similarity index 96% rename from static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java rename to static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeDefaultClassPositiveCases.java index 944b8cd90..552f190d3 100644 --- a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerDefaultClassPositiveCases.java +++ b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeDefaultClassPositiveCases.java @@ -32,9 +32,9 @@ import org.reactivestreams.Subscriber; /** - * Cases that don't use autodispose and should fail the {@link AutoDisposeLeakChecker} check. + * Cases that don't use autodispose and should fail the {@link UseAutoDispose} check. */ -public class AutoDisposeLeakCheckerDefaultClassPositiveCases +public class UseAutoDisposeDefaultClassPositiveCases implements LifecycleScopeProvider { private final BehaviorSubject lifecycleSubject = diff --git a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeNegativeCases.java similarity index 91% rename from static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java rename to static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeNegativeCases.java index e0a6fc24e..1f5a598c7 100644 --- a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/AutoDisposeLeakCheckerNegativeCases.java +++ b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeNegativeCases.java @@ -17,13 +17,7 @@ package com.uber.autodispose.error.prone.checker; import com.uber.autodispose.AutoDispose; -import com.uber.autodispose.AutoDisposeConverter; -import com.uber.autodispose.CompletableScoper; -import com.uber.autodispose.FlowableScoper; import com.uber.autodispose.LifecycleEndedException; -import com.uber.autodispose.MaybeScoper; -import com.uber.autodispose.ObservableScoper; -import com.uber.autodispose.SingleScoper; import com.uber.autodispose.LifecycleScopeProvider; import com.uber.autodispose.TestLifecycleScopeProvider; @@ -39,9 +33,9 @@ import org.reactivestreams.Subscriber; /** - * Cases that use {@link AutoDispose} and should not fail the {@link AutoDisposeLeakChecker} check. + * Cases that use {@link AutoDispose} and should not fail the {@link UseAutoDispose} check. */ -public class AutoDisposeLeakCheckerNegativeCases +public class UseAutoDisposeNegativeCases implements LifecycleScopeProvider { private final BehaviorSubject lifecycleSubject = From 64e5621fe018de4d4e8dafc7d0c2f7017d5aad12 Mon Sep 17 00:00:00 2001 From: Harshit Bangar Date: Sun, 21 Jan 2018 11:00:54 +0530 Subject: [PATCH 26/26] Fixed description --- .../error/prone/checker/UseAutoDispose.java | 3 ++- .../UseAutoDisposeCustomClassPositiveCases.java | 12 ++++++------ .../UseAutoDisposeDefaultClassPositiveCases.java | 12 ++++++------ 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/UseAutoDispose.java b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/UseAutoDispose.java index 94803055c..1ced255e3 100644 --- a/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/UseAutoDispose.java +++ b/static-analysis/autodispose-error-prone-checker/src/main/java/com/uber/autodispose/error/prone/checker/UseAutoDispose.java @@ -50,7 +50,8 @@ @AutoService(BugChecker.class) @BugPattern( name = "UseAutoDispose", - summary = "Always apply an AutoDispose scope before subscribing in lifecycle scopes", + summary = "Always apply an AutoDispose scope before " + + "subscribing within defined scoped elements.", tags = CONCURRENCY, severity = ERROR ) diff --git a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeCustomClassPositiveCases.java b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeCustomClassPositiveCases.java index 86f6fcbd5..b3b12e82a 100644 --- a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeCustomClassPositiveCases.java +++ b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeCustomClassPositiveCases.java @@ -27,27 +27,27 @@ public class UseAutoDisposeCustomClassPositiveCases extends ComponentWithLifeCycle { public void observable_subscribeWithoutAutoDispose() { - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing within defined scoped elements. Observable.empty().subscribe(); } public void single_subscribeWithoutAutoDispose() { - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing within defined scoped elements. Single.just(true).subscribe(); } public void completable_subscribeWithoutAutoDispose() { - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing within defined scoped elements. Completable.complete().subscribe(); } public void maybe_subscribeWithoutAutoDispose() { - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing within defined scoped elements. Maybe.empty().subscribe(); } public void flowable_subscribeWithoutAutoDispose() { - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing within defined scoped elements. Flowable.empty().subscribe(); } @@ -55,7 +55,7 @@ public void parallelFlowable_subscribeWithoutAutoDispose() { Subscriber[] subscribers = new Subscriber[] {}; Flowable.just(1, 2) .parallel(2) - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing within defined scoped elements. .subscribe(subscribers); } } diff --git a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeDefaultClassPositiveCases.java b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeDefaultClassPositiveCases.java index 552f190d3..794759b2d 100644 --- a/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeDefaultClassPositiveCases.java +++ b/static-analysis/autodispose-error-prone-checker/src/test/resources/com/uber/autodispose/error/prone/checker/UseAutoDisposeDefaultClassPositiveCases.java @@ -76,27 +76,27 @@ public Function[] subscribers = new Subscriber[] {}; Flowable.just(1, 2) .parallel(2) - // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing in lifecycle scopes + // BUG: Diagnostic contains: Always apply an AutoDispose scope before subscribing within defined scoped elements. .subscribe(subscribers); } }