novoda/gradle-static-analysis-plugin

Unable to support JLLeitschuh/ktlint-gradle plugin version 9.0.0

AdamMc331 opened this issue · 7 comments

I recently updated to version 9.0.0 of the ktlint-gradle plugin and my builds started failing with this stack trace:

Caused by: groovy.lang.MissingPropertyException: Could not get unknown property 'reportOutputFiles' for task ':app:ktlintTestDebugSourceSetCheck' of type org.jlleitschuh.gradle.ktlint.KtlintCheckTask.
        at org.gradle.internal.metaobject.AbstractDynamicObject.getMissingProperty(AbstractDynamicObject.java:84)
        at org.gradle.internal.metaobject.AbstractDynamicObject.getProperty(AbstractDynamicObject.java:61)
        at org.jlleitschuh.gradle.ktlint.KtlintCheckTask_Decorated.getProperty(Unknown Source)
        at com.novoda.staticanalysis.internal.ktlint.KtlintConfigurator$_createCollectViolationsTask_closure9.doCall(KtlintConfigurator.groovy:137)

Then I noticed, in the latest changelog, this task was replaced by allReportsOutputFiles: https://github.com/JLLeitschuh/ktlint-gradle/blob/master/CHANGELOG.md#added-1

But this plugin appears to still look for reportOutputFiles:

I don't think it's as simple as just updating this plugin because it may break for people who don't want to also update the KtLint gradle plugin.

What are some options we have? Let me know if I can help contribute a solution.

That's one of the reasons why we don't update this plugin to Kotlin. Groovy is more dynamic.

We support multiple versions of plugins by checking availability of certain properties. The solution is to check if the property is available. One example is available here:

@tasomaniac So do you think we should do like an if/else at the suspected line above? If the allReportsOutputFiles exists, use that, otherwise use what we use now. And if even that fails, we'll still throw?

First thing we should do is to add the new ktlint version into integration tests. That has a really good coverage. I believe a simple if like you mentioned would fix the issue for now. And tests should verify that.

For external properties that are changing all the time, we have some custom error reporting support that gives a better error message into console. We do that by try-catching and rethrowing the exception with a better message.

Hmm I added the new version to the integration tests but they're all failing for me. Here's a compare diff: master...AdamMc331:GSAP-201/support_allReportsOutputFiles

They all are giving me this error:

* What went wrong:
An exception occurred applying plugin request [id: 'org.jlleitschuh.gradle.ktlint', version: '9.0.0']
> Failed to apply plugin [class 'org.jlleitschuh.gradle.ktlint.KtlintBasePlugin']
   > Could not create an instance of type org.jlleitschuh.gradle.ktlint.KtlintExtension_Decorated.
      > org.gradle.api.model.ObjectFactory.fileProperty()Lorg/gradle/api/file/RegularFileProperty;

I'm not sure what to make of this? I looked at the plugin's repo, and there is a KtlintExtension class, but I don't know where this _Decorated is coming from and what that means.

Hmmm, this is because we still have Gradle 4.x on this project although we support newer versions. It looks like ktlint Gradle plugin is not supporting 4.x anymore.

The reason we were stuck at 4.x was memory usage. Especially because of Android lint which is memory hungry.

To unblock you working on this, you can try a newer Gradle version. For that either use local Gradle installation or update the wrapper in this project. When you do that, I would recommend only running ktlint tests. For that, you would do this in command line:

gradle test --tests "*.KtlintIntegrationTest"

I'm upset I didn't think about it. That seemed to do it.

I have to run but I will take a look soon - perhaps there is a version of 5.x that helped with memory usage we could upgrade to, in order to make this work?

Fixed by #202