Gradle 5.x

[ZacSweers]

Resolves #296

Note there's still some warnings about accessing compiler APIs, but that's internal to the error prone plugin. Will need to wait for tbroyer/gradle-errorprone-plugin#22

[ZacSweers]

For the life of me I don't understand this lint/gradle error. Continues to happen even if I manually create that configuration too

[tbroyer]

Fwiw, the warning in net.ltgt.errorprone is due to AGP 3.3, which you introduced in #326, not Gradle 5.

[ZacSweers]

Yep, I was just noting that they're coming from the plugin and not something we can completely resolve yet

[ShaishavGandhi]

Looks like they just released 5.3

[ZacSweers]

Now lint OOMs CI. -_-

[ZacSweers]

@lazaroclapp @msridhar any ideas about that NullAway wiring failure?

Caused by: java.lang.IllegalStateException: DO NOT report an issue to Error Prone for this crash!  NullAway configuration is incorrect.  Must specify annotated packages, using the -XepOpt:NullAway:AnnotatedPackages=[...] flag.  If you feel you have gotten this message in error report an issue at https://github.com/uber/NullAway/issues.
  	at com.uber.nullaway.ErrorProneCLIFlagsConfig.<init>(ErrorProneCLIFlagsConfig.java:112)
  	at com.uber.nullaway.NullAway.<init>(NullAway.java:236)
  	... 53 more

[tbroyer]

Task :android:autodispose-android:compileDebugUnitTestJavaWithJavac FAILED

You changed from configuring every JavaCompile task to only configuring those from libraryVariants, so testVariants and unitTestVariants aren't configured.

[ZacSweers]

Yep just realized that in trying to repro this on the nullaway sample. Is there a way to disable nullaway for those variants though? I know only running a subset of error prone checks in tests is a pattern some projects use, but it seems like that may not be possible if it's always initialized just for being on the classpath

[tbroyer]

Either only put NullAway in the classpath for non-test variants (annotationProcessor would likely work), or disable it in test variants, something like (using net.ltgt.errorprone 0.7.1)

afterEvaluate {
  tasks.withType(JavaCompile).configureEach {
    options.errorprone {
      if (compilingTestOnlyCode) {
        check("NullAway", CheckSeverity.OFF)
      }
    }
  }
}

(or alternatively using testVariants and unitTestVariants)

I would however configure NullAway:AnnotatedPackages on each and every task, and only change the check severity depending on whether this is a test task or not (or only put NullAway in the classpath where you want to use it). YMMV

[tbroyer]

BTW (seeing your last commit), net.ltgt.nullaway would currently (v0.1) always pass -Xep:NullAway with a severity, so unless you use -XepIgnoreUnknownCheckNames this forces you to put NullAway in the classpath for all compile tasks. You could however use a provider to lazily compute the check severity depending on compilingTestOnlyCode:

tasks.withType(JavaCompile).configureEach {
  options.errorprone.nullaway.severity = provider { options.errorprone.compilingTestOnlyCode ? CheckSeverity.OFF : CheckSeverity.ERROR }
}

(Fwiw, I've just updated net.ltgt.errorprone to use lazy properties so you could use compilingTestOnlyCode.map { … } instead, but I have more API changes to do before cutting a new release)

[ZacSweers]

Yep I agree with putting it in every variant normally. Thanks for the example, continuing that thread in uber/NullAway#294

Unfortunately even if I manually wire this for each of those variants in 0916401, the error still comes up. Will dig in more as I'm not sure where else this could be configured unless DomainObjectSet#addAll is noop-ing for some reason.

[ZacSweers]

confirmed locally that DomainObjectSet#addAll was indeed not doing anything :|

[ZacSweers]

@shaishavgandhi05 want to re-review this since it's changed a fair bit now?

[ZacSweers]

I've also tweaked the travis CI settings to only run branch builds on master (for snapshots) and limit PR builds to just the single PR build type

[ShaishavGandhi]

Why the annotation on separate line? All other tests have it on the same line right?

[ZacSweers]

Going to start slowly transitioning this to GJF, started with this