Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Fix detekt rc10 integration #144

Merged
merged 3 commits into from
Nov 8, 2018
Merged

Fix detekt rc10 integration #144

merged 3 commits into from
Nov 8, 2018

Conversation

tasomaniac
Copy link
Contributor

Detekt RC10 recently changes the destionation File to be lazy by default. The way they do it is pretty weird. They only use the Lazy FileProvider API from Gradle when consumed at a very late stage. By default destination is always null in configuration time.

This causes problems with our integration.

I tried many things to integrate into their new lazy initialization but it does not seem to be possible. They need to fully use the Lazy FileProvider API.

Instead I set default values for xml reports. If user changes them, it is no problem, they are still picked up.

Additionally, added a check to enabled being false. If the user manually disable the xml report, we fail with a proper error message.

import com.novoda.staticanalysis.internal.CollectViolationsTask
import groovy.util.slurpersupport.GPathResult

class CollectDetektViolationsTask extends CollectViolationsTask {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted since Detekt uses Checkstyle format and I don't think they will ever change that.

@@ -29,22 +29,10 @@ build:
comments: 1

processors:
active: true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed them since the test output will be cleaner without them.

Copy link
Contributor

@mr-archano mr-archano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, left a couple of (probably dumb) questions

if (detekt.hasProperty('reports')) {
detekt.reports {
xml.enabled = true
xml.destination = new File(project.buildDir, 'reports/detekt/detekt.xml')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tasomaniac does it mean we should warn users about this? is it possible that we read this value and then later the destination is amended lazily by Detekt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this means that we always set it by default. Detekt's lazy fallback will never be used.

But that's fine because in case users but their destionation, that will be used anyways.

@@ -11,15 +12,17 @@ import org.gradle.api.Task
class DetektConfigurator implements Configurator {

private static final String DETEKT_PLUGIN = 'io.gitlab.arturbosch.detekt'
private static final String LAST_COMPATIBLE_DETEKT_VERSION = '1.0.0.RC9.2'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean "we support Detekt up to version X"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I keep updating this whenever I fix new problems :) To be honest, the problems so far were all very weird and were crashing earlier. It might be the case that this was never shown. 🤷

But anyways, it is nice to have.

@mr-archano mr-archano merged commit 03cd383 into develop Nov 8, 2018
@mr-archano mr-archano deleted the taso/fix/detekt-rc10 branch November 8, 2018 14:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants