Error prone check added to check unbinded subscriptions by bangarharshit · Pull Request #156 · uber/AutoDispose

bangarharshit · 2018-01-03T03:19:44Z

This is a copy of the check from Uber/Ribs with 2 major differences:

Support for 3rd party libraries like Ribs and Conductor via error-prone flag -XepOpt:AutoDisposeLeakCheck.
Added support for the new as based API.

Things to do:

It still doesn't include matcher for ParallelFlowable.
Fix build.gradle for removing redundant dependencies and publishing artifact. Can look into this in the weekend.

Fixes - #152

ZacSweers · 2018-01-04T00:34:51Z

Will start review once the to-dos are finished, unless you wanted me to give early feedback now before. Let me know

bangarharshit · 2018-01-04T02:41:55Z

Yeah. It will be great if you can give some early feedback. Most of the remaining changes will only be in build.gradle. The ParallelFlowableMatcher is not yet added because the PR is still open.

ZacSweers

@msridhar could I get your review on this as well?

ZacSweers · 2018-01-04T02:47:51Z

gradle/dependencies.gradle

 ]

+def apt = [
+        autoService : "com.google.auto.service:auto-service:1.0-rc3",


nit: make this match the rest of this file's style

also rc4 is out

ZacSweers · 2018-01-04T02:48:15Z

gradle/dependencies.gradle


    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}",


nit: this goes above errorProneCheckApi alphabetically

ZacSweers · 2018-01-04T02:48:49Z

tooling/autodispose-rx-error-prone-checker/build.gradle

+
+// 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")


@kageiit can you confirm this is correct?

actually if you wouldn't mind taking a general review of this config, I'd appreciate the extra set of eyes

ZacSweers · 2018-01-04T02:49:21Z

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"


can you explain the shadow bits here?

I am trying to create a fat jar here and the steps are taken from - NullAway's build.gradle - dependency part.

It is not working in any case as I am not able to use it in the sample project. Will be fixing it over the weekend. I am not sure what is the best way to publish an error-prone check.

Hmm. Maybe @msridhar or @kageiit have advice here

You should not need to create a fat jar. the jar with just the classes from this module can be consumed by anyone that uses error prone

Yeah all the shadow stuff isn't needed here. NullAway needs it due to a forked dependence

ZacSweers · 2018-01-04T02:50:02Z

tooling/autodispose-rx-error-prone-checker/build.gradle

+}
+
+test {
+  maxHeapSize = "1024m"


What's this for?

ZacSweers · 2018-01-04T02:52:39Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "RxJavaMissingAutoDisposeErrorChecker",


Remove redundant RxJava

I would recommend renaming this to

MissingAutoDisposeErrorChecker

Adding Rxjava at the start seems redundant. Similar to @hzsweers comment earlier about removing rx from the artifact identifier

ZacSweers · 2018-01-04T02:53:29Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+  private static final ImmutableList<MethodMatchers.MethodNameMatcher> TO_CALL_MATCHERS;
+  private static final ImmutableList<MethodMatchers.MethodNameMatcher> SUBSCRIBE_MATCHERS;
+  private static final String SUBSCRIBE = "subscribe";
+  private static final String TO = "to";


We don't need to support the to implementation since it's going away

ZacSweers · 2018-01-04T02:54:15Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+    ImmutableList.Builder<String> builder = new ImmutableList.Builder<>();
+    classesWithLifecycle = builder
+        .add("android.app.Activity")
+        .add("android.app.Fragment")


we should support support fragments out of the box. I'm kind of thinking we shouldn't check any by default though, and leave it totally up to the user. Will think on it more

It is s already added.

ZacSweers · 2018-01-04T02:54:56Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+  }
+
+  static {
+    ImmutableList.Builder<MethodMatchers.MethodNameMatcher> builder = new ImmutableList.Builder<>();


remove all these

ZacSweers · 2018-01-04T02:55:09Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+
+    ImmutableList.Builder<MethodMatchers.MethodNameMatcher> builder2
+        = new ImmutableList.Builder<>();
+    builder2


why not continue the fluent API after creating the builder?

kageiit · 2018-01-04T05:24:50Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "RxJavaMissingAutoDisposeErrorChecker",


I would recommend renaming this to

MissingAutoDisposeErrorChecker

Adding Rxjava at the start seems redundant. Similar to @hzsweers comment earlier about removing rx from the artifact identifier

kageiit · 2018-01-04T05:27:44Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+@BugPattern(
+    name = "RxJavaMissingAutoDisposeErrorChecker",
+    summary = "Always apply an Autodispose scope before subscribing",
+    category = JDK,


I would recommend using the concurrency tag instead of the deprecated categories

kageiit · 2018-01-04T05:31:45Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+
+  @Override
+  public String linkUrl() {
+    return "https://github.com/uber/RIBs"


This should be updated

Update to a shortcut - d4883cc @hzsweers can this point to a wiki in autodispose? The readme in ribs is not updated.

kageiit · 2018-01-04T05:33:36Z

tooling/autodispose-rx-error-prone-checker/build.gradle

+
+// 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")


Many config options in this file are not required. They are specific to nullaway and do not translate here.

Remove this line

This actually might be needed for unit tests, to get EP java in bootclasspath. Can try without first.

This is needed for unit tests.

kageiit · 2018-01-04T05:35:34Z

tooling/autodispose-rx-error-prone-checker/build.gradle

+
+  compileOnly deps.build.errorProneCheckApi
+
+  errorprone deps.build.errorProne


Remove this line

kageiit · 2018-01-04T05:36:17Z

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"


You should not need to create a fat jar. the jar with just the classes from this module can be consumed by anyone that uses error prone

kageiit · 2018-01-04T05:36:23Z

tooling/autodispose-rx-error-prone-checker/build.gradle

@@ -0,0 +1,70 @@
+plugins {
+  id "net.ltgt.errorprone" version "0.0.13"


Remove this line

kageiit · 2018-01-04T05:36:28Z

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"


Remove this line

kageiit · 2018-01-04T05:36:35Z

tooling/autodispose-rx-error-prone-checker/build.gradle

+  }
+  testCompile project(':autodispose')
+
+  epJavac deps.build.errorProneCheckApi


Remove this line

kageiit · 2018-01-04T05:37:25Z

tooling/autodispose-rx-error-prone-checker/build.gradle

+  epJavac deps.build.errorProneCheckApi
+}
+
+shadowJar {


Remove everything from this line and below (Change the maven bits to just match what other projects do)

msridhar · 2018-01-04T19:45:03Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    if (matcher(classesWithLifecycle).matches(tree, state)) {


The result of matcher(classesWithLifecycle) should be cached in an instance field

msridhar · 2018-01-04T19:48:17Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+          Type.ClassType enclosingClassType = ASTHelpers.getType(enclosingClass);
+
+          for (String s : classesWithLifecycle) {
+            Type lifecycleType = state.getTypeFromString(s);


Small optimization: store classesWithLifecycle as a List<Type>. The call getTypeFromString() can be a bit costly (though for this checker it may not get called very often)

Missed it. Will fix in a separate commit.

@msridhar quick question - do you mean to cache it when we will call it the first time? Can we get the type from a string without visitor state? I am not sure how to do it eagerly.

Yeah you're right, hard to do eagerly. We can skip this optimization for now I think; this code shouldn't run all that often.

msridhar · 2018-01-04T19:48:59Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+          }
+          return false;
+        } catch (ClassCastException e) {
+          return false;


Why catch a ClassCastException here?

Removed - 96a8baf

msridhar · 2018-01-04T19:50:20Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+    SUBSCRIBE_MATCHERS = builder3.build();
+  }
+
+  private static final Matcher<ExpressionTree> METHOD_NAME_MATCHERS =


bangarharshit

Addressed comments except caching the types in a list. Will push a commit for that.

bangarharshit · 2018-01-06T03:12:49Z

gradle/dependencies.gradle

 ]

+def apt = [
+        autoService : "com.google.auto.service:auto-service:1.0-rc3",


bangarharshit · 2018-01-06T03:12:59Z

gradle/dependencies.gradle


    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}",


bangarharshit · 2018-01-06T03:13:04Z

tooling/autodispose-rx-error-prone-checker/build.gradle

@@ -0,0 +1,70 @@
+plugins {
+  id "net.ltgt.errorprone" version "0.0.13"


bangarharshit · 2018-01-06T03:13:09Z

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"


bangarharshit · 2018-01-06T03:13:14Z

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"


bangarharshit · 2018-01-06T03:17:32Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+          Type.ClassType enclosingClassType = ASTHelpers.getType(enclosingClass);
+
+          for (String s : classesWithLifecycle) {
+            Type lifecycleType = state.getTypeFromString(s);


Missed it. Will fix in a separate commit.

bangarharshit · 2018-01-06T03:17:38Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    if (matcher(classesWithLifecycle).matches(tree, state)) {


bangarharshit · 2018-01-06T03:17:50Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+    SUBSCRIBE_MATCHERS = builder3.build();
+  }
+
+  private static final Matcher<ExpressionTree> METHOD_NAME_MATCHERS =


bangarharshit · 2018-01-06T03:17:57Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+@BugPattern(
+    name = "RxJavaMissingAutoDisposeErrorChecker",
+    summary = "Always apply an Autodispose scope before subscribing",
+    category = JDK,


bangarharshit · 2018-01-06T03:19:01Z

...n/java/com/uber/autodispose/rx/error/prone/checker/RxJavaMissingAutoDisposeErrorChecker.java

+
+  @Override
+  public String linkUrl() {
+    return "https://github.com/uber/RIBs"


Update to a shortcut - d4883cc @hzsweers can this point to a wiki in autodispose? The readme in ribs is not updated.

ZacSweers

Can you edit this page? If so, can you write integration details for consumers?

Let's also add a section to the README linking to this. Something to the effect of:

There is an optional error-prone checker you can use to enforce use of AutoDispose. Integration steps can be found on the wiki

ZacSweers · 2018-01-06T07:46:47Z

tooling/autodispose-error-prone-checker/build.gradle

@@ -0,0 +1,32 @@
+plugins {
+  id "net.ltgt.errorprone" version "0.0.13"
+  id "java"


java-library, then use api and implementation configurations in dependencies

ZacSweers · 2018-01-06T07:49:48Z

...r/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java

+
+  public MissingAutoDisposeErrorChecker(ErrorProneFlags flags) {
+    Optional<ImmutableList<String>> inputClasses = flags.getList("AutoDisposeLeakCheck");
+    ImmutableList<String> classesWithLifecycle = new ImmutableList.Builder<String>()


This list seems to still be missing support fragments, LifecycleOwner, and ScopeProvider. Is that intentional? Also, we should have a way to allow the user to disable these built-in classes if they want.

I can add these. Not sure what is the best way to disable it. Another flag? cc: @msridhar if you have any suggestions.

Another option is to treat flag as an override and not an addition. The developers may have to pass all these base classes themselves. Con - The callers need to be aware of all the base cases. Pros - Simple.

Went with 2nd as it is much simpler and requirements for different projects can be different. cf07e46

ZacSweers · 2018-01-06T07:52:01Z

...r/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java

+
+/**
+ * Checker for subscriptions not binding to lifecycle in components with lifecycle.
+ * Use -XepOpt:AutoDisposeLeakCheck to add support for custom components with lifecycle.


Let's add a more fleshed out example for things like Conductor in a <code><pre> block

There are no code changes. These are just changes for build.gradle. Not sure if this will be the correct place. Maybe we should remove this line also.

Even a one liner is useful. This is a correct place for an example. We can also have a more fleshed out one in the wiki page.

ZacSweers · 2018-01-06T07:52:44Z

...r/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java

+            ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class);
+        Type.ClassType enclosingClassType = ASTHelpers.getType(enclosingClass);
+
+        for (String s : classesWithLifecycle) {


We can use a java 8 stream here

ZacSweers · 2018-01-06T07:53:08Z

...r/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java

+            return false;
+          }
+
+          for (MethodMatchers.MethodNameMatcher nameMatcher : AS_CALL_MATCHERS) {


ZacSweers · 2018-01-06T07:56:02Z

...c/test/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorCheckerTest.java

+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+@SuppressWarnings("CheckTestExtendsBaseClass")


Remove this, copy-pasta from internal

Cool. Please make this a separate commit in the future though, that commit was mostly unrelated changes

ZacSweers · 2018-01-06T07:56:45Z

...r/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java

+    tags = {BugPattern.StandardTags.CONCURRENCY},
+    severity = ERROR
+)
+public final class MissingAutoDisposeErrorChecker extends BugChecker


Assuming the AutoDisposeLeakCheck name sticks, we should call this AutoDisposeLeakChecker

ZacSweers · 2018-01-06T07:56:54Z

...c/test/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorCheckerTest.java

+
+@RunWith(JUnit4.class)
+@SuppressWarnings("CheckTestExtendsBaseClass")
+public class MissingAutoDisposeErrorCheckerTest {


Let's call this AutoDisposeLeakCheckerTest

ZacSweers · 2018-01-06T07:57:32Z

...c/test/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorCheckerTest.java

+  }
+
+  @Test
+  public void test_autodisposePositiveCases2() {


Can we call this something more descriptive? Or at least have comments explaining what these two cases are verifying?

47c35ce. Calling them default for default lifecycle and custom when input is provided.

Cool, but please make this a separate commit next time. That commit's message/desc mentioned nothing about the descriptive method changes

ZacSweers · 2018-01-06T07:58:53Z

...r/src/main/java/com/uber/autodispose/error/prone/checker/MissingAutoDisposeErrorChecker.java

+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "MissingAutoDisposeErrorChecker",


This should be AutoDisposeLeakCheck right? Not sure if I'm missing how these are conventionally done CC @msridhar

The name usually matches the class

It is not AutoDisposeLeakChecker. 47c35ce

I'm curious to know why you picked a 3rd new name. Checker should not be part of the name. Thinking more on this, let's call the bugpattern name MissingAutoDispose, and also call the class that

ZacSweers · 2018-01-07T10:25:46Z

The parallelflowable change is merged now if you want to rebase

ZacSweers · 2018-01-16T01:35:58Z

@bangarharshit are you planning to finisht this?

bangarharshit · 2018-01-16T02:14:27Z

@hzsweers Sorry was busy with another thing.

I am getting weird issues with the build for android projects (it is working fine for normal java projects as an apt). Strangely, I am getting a similar issue for even normal java project after migrating to java-library. Something similar to - google/error-prone#730. I will try to resolve this issue over the weekend.

I can address the rest of the comments and then solely look on the build related issues.

ZacSweers · 2018-01-16T02:45:41Z

Sounds good, I'm likely going to cut a new release without waiting for this one in the meantime

Error prone flag added to support components with custom lifecycle Removed unused dependencies

bangarharshit · 2018-01-20T09:43:54Z

Merge parallel flowable changes and there was a bug when superclass constructor is invoked it is not MemberSelectTree.

Will update the wiki in sometime.

…on can also contains lint related details in the future

ZacSweers · 2018-01-20T20:57:56Z

README.md


 Snapshots of the development version are available in [Sonatype's snapshots repository][snapshots].

+Static code analysis


just call this Static Analysis. Also, please move this to above the Download section

ZacSweers · 2018-01-20T20:59:02Z

...e-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java

@@ -49,7 +49,7 @@
 @AutoService(BugChecker.class)


In the future, this commit should have been two commits. One for the parallelflowable support, and another for the naming convention cleanups

ZacSweers · 2018-01-20T21:00:54Z

settings.gradle

 include 'autodispose-rxlifecycle'
 include ':sample'
 include ':test-utils'
+include ':tooling:autodispose-error-prone-checker'


Let's rename tooling to static-analysis. I suspect you borrowed the tooling convention from the internal monorepo, but there's not actually any build tooling here

Also changing the name in gradle.properties for the artifact.

No the artifact name is fine. Only the directory needs to change

ZacSweers · 2018-01-20T21:01:27Z

Will update the wiki in sometime.

Do you have a time frame for that? I'd like to have that in place before merging this

bangarharshit · 2018-01-21T01:17:43Z

I don't have the permission to update the wiki. Since Github doesn't support PR for the wiki, can you enable wiki edit for me?

…ference for filtering

ZacSweers · 2018-01-21T04:26:44Z

...e-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java

+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "AutoDisposeLeakChecker",
+    summary = "Always apply an AutoDispose scope before subscribing",


Let's flesh this out to mention that this is only when running in lifecycle scopes.

ZacSweers · 2018-01-21T04:27:00Z

...e-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java

+@BugPattern(
+    name = "AutoDisposeLeakChecker",
+    summary = "Always apply an AutoDispose scope before subscribing",
+    tags = {BugPattern.StandardTags.CONCURRENCY},


This doesn't need braces since it's one element. Also static import CONCURRENCY

ZacSweers · 2018-01-21T04:27:51Z

...e-checker/src/main/java/com/uber/autodispose/error/prone/checker/AutoDisposeLeakChecker.java

+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "AutoDisposeLeakChecker",


I left a comment somewhere that I can't find now, but let's call both the bugpattern name and the class UseAutoDispose. Don't have Checker as a suffix anywhere. This matches the error-prone convention

I think maybe that comment wasn't published because I never saw that. Error-prone doesn't have a common stand against it - https://github.com/google/error-prone/blob/master/core/src/main/java/com/google/errorprone/bugpatterns/CompileTimeConstantChecker.java. Fixing it for this case. Maybe you meant this comment - #156 (comment).

Renaming this class for the fourth time based on feedback from PR (RxJavAutoMissingDisposeLeakChecker - MissingAutoDisposeLeakChecker -AutoDisposeLeakChecker - UseAutoDispose).

ZacSweers · 2018-01-21T07:25:55Z

I think this looks good. Do you want to make a separate PR with the wiki contents (won't merge it, but just something we can review here), then I'll paste them into the wiki when that's done?

Also, after this is release, we should remove the one in uber/ribs and make it depend on this

ZacSweers · 2018-01-21T07:26:06Z

Thanks!

ZacSweers self-requested a review January 4, 2018 00:34

ZacSweers reviewed Jan 4, 2018

View reviewed changes

kageiit suggested changes Jan 4, 2018

View reviewed changes

msridhar suggested changes Jan 4, 2018

View reviewed changes

bangarharshit commented Jan 6, 2018

View reviewed changes

kageiit approved these changes Jan 6, 2018

View reviewed changes

msridhar approved these changes Jan 6, 2018

View reviewed changes

ZacSweers reviewed Jan 6, 2018

View reviewed changes

bangarharshit added 14 commits January 20, 2018 14:17

Error prone check added to check unbinded subscriptions

85ecc86

Error prone flag added to support components with custom lifecycle Removed unused dependencies

Removed shadow plugin and other unused configs from build.gradle

456ed87

Renamed package and module name to remove rx

db11fb2

Removed to based api and inlined the builders

872e71e

Fixes in the checker - caching matcher

f876a67

Updated link url to point to a shortcut to wiki/readmen on autodispose

93df2cd

Renamed test to AutoDisposeLeakCheck

47c35ce

Added logic to unset default classes

cf07e46

Updated link url

2547c2f

Used streams and lambdas to simplify code

702b929

Fixed javadoc and build errors

e1703ff

Fixed a bug when the invocation is not of the correct type

dd16a14

Rebased master and fixed naming conventions

2e8bf18

Changed plugin type to java library and tested it on sample project

a9bf402

bangarharshit force-pushed the autodispose-error-prone-checker branch from d4883cc to a9bf402 Compare January 20, 2018 09:33

bangarharshit added 2 commits January 20, 2018 15:26

Fixed checkstyle and removed redundant params from tesT

a90e3d6

Updated readme to point to the wiki for error prone check. This secti…

d4b3f22

…on can also contains lint related details in the future

ZacSweers reviewed Jan 20, 2018

View reviewed changes

bangarharshit added 7 commits January 21, 2018 07:14

Reverted to compileOnly and clubbed all testImplementation together

9fd6dde

Renamed tooling to static-analysis

5de25a5

Fixed readme

c51f4db

Converted for loop for subscriber matcher to lambda and use method re…

aca6ccf

…ference for filtering

Newline in readme

2a9ba37

Added comments to explain filtering

8308c39

Fixed comments

6fd81f3

ZacSweers reviewed Jan 21, 2018

View reviewed changes

bangarharshit added 3 commits January 21, 2018 10:44

Fixed tags and fleshed out the comment

7965a81

Renamed class to UseAutoDispose

282291b

Fixed description

64e5621

ZacSweers merged commit 46762c6 into uber:master Jan 21, 2018

ZacSweers mentioned this pull request Jan 21, 2018

Autodispose error prone checker #152

Closed

bangarharshit mentioned this pull request Jan 21, 2018

Wiki for AutoDispose scope missing error prone check #162

Closed

ZacSweers mentioned this pull request Feb 4, 2018

0.6.0 Release Prep #170

Closed


		compileOnly deps.build.errorProneCheckApi

		errorprone deps.build.errorProne

		@@ -0,0 +1,70 @@
		plugins {
		id "net.ltgt.errorprone" version "0.0.13"


		Snapshots of the development version are available in [Sonatype's snapshots repository][snapshots].

		Static code analysis

Conversation

bangarharshit commented Jan 3, 2018

Uh oh!

ZacSweers commented Jan 4, 2018

Uh oh!

bangarharshit commented Jan 4, 2018

Uh oh!

ZacSweers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!