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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a914d49
Update detekt config to clean the output for test projects
tasomaniac Nov 7, 2018
5f2a790
Use the same checkstyle collection task
tasomaniac Nov 7, 2018
7d3d1dd
Add xml report path by default for newer version of detekt
tasomaniac Nov 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions .../main/groovy/com/novoda/staticanalysis/internal/detekt/CollectDetektViolationsTask.groovy
View file
Open in desktop

This file was deleted.

26 changes: 21 additions & 5 deletions plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.novoda.staticanalysis.internal.detekt
import com.novoda.staticanalysis.StaticAnalysisExtension
import com.novoda.staticanalysis.Violations
import com.novoda.staticanalysis.internal.Configurator
import com.novoda.staticanalysis.internal.checkstyle.CollectCheckstyleViolationsTask
import org.gradle.api.GradleException
import org.gradle.api.NamedDomainObjectContainer
import org.gradle.api.Project
Expand All @@ -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.

private static final String LAST_COMPATIBLE_DETEKT_VERSION = '1.0.0-RC10'
private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more information see https://github.com/arturbosch/detekt.'
private static final String OUTPUT_NOT_DEFINED = 'Output not defined! To analyze the results, `output` needs to be defined in Detekt profile.'
private static final String DETEKT_CONFIGURATION_ERROR = "A problem occurred while configuring Detekt. Please make sure to use a compatible version (All versions up to $LAST_COMPATIBLE_DETEKT_VERSION)"
private static final String XML_REPORT_NOT_ENABLED = 'XML report must be enabled. Please make sure to enable "reports.xml" in your Detekt configuration'

private final Project project
private final Violations violations
private final Task evaluateViolations


static DetektConfigurator create(Project project,
NamedDomainObjectContainer<Violations> violationsContainer,
Task evaluateViolations) {
Expand All @@ -45,18 +48,31 @@ class DetektConfigurator implements Configurator {
}

def detekt = project.extensions.findByName('detekt')
setDefaultXmlReport(detekt)
config.delegate = detekt
config()

def collectViolations = configureToolTask(detekt)
evaluateViolations.dependsOn collectViolations
}
}

private CollectDetektViolationsTask configureToolTask(detekt) {

private void setDefaultXmlReport(detekt) {
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.

}
}
}

private CollectCheckstyleViolationsTask configureToolTask(detekt) {
def detektTask = project.tasks.findByName('detekt')
if (detektTask?.hasProperty('reports')) {
def reports = detektTask.reports
if (!reports.xml.enabled) {
throw new IllegalStateException(XML_REPORT_NOT_ENABLED)
}
return createCollectViolationsTask(
violations,
detektTask,
Expand Down Expand Up @@ -89,8 +105,8 @@ class DetektConfigurator implements Configurator {
}
}

private CollectDetektViolationsTask createCollectViolationsTask(Violations violations, detektTask, File xmlReportFile, File htmlReportFile) {
project.tasks.create('collectDetektViolations', CollectDetektViolationsTask) { task ->
private CollectCheckstyleViolationsTask createCollectViolationsTask(Violations violations, detektTask, File xmlReportFile, File htmlReportFile) {
project.tasks.create('collectDetektViolations', CollectCheckstyleViolationsTask) { task ->
task.xmlReportFile = xmlReportFile
task.htmlReportFile = htmlReportFile
task.violations = violations
Expand Down
18 changes: 3 additions & 15 deletions plugin/src/test/fixtures/rules/detekt/detekt.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

exclude:
# - 'FunctionCountProcessor'
# - 'PropertyCountProcessor'
# - 'ClassCountProcessor'
# - 'PackageCountProcessor'
# - 'KtFileCountProcessor'
active: false

console-reports:
active: true
exclude:
# - 'ProjectStatisticsReport'
# - 'ComplexityReport'
# - 'NotificationReport'
# - 'FindingsReport'
# - 'BuildFailureReport'
active: false

output-reports:
active: true
Expand Down Expand Up @@ -364,4 +352,4 @@ style:
active: false
WildcardImport:
active: true
excludeImports: 'java.util.*,kotlinx.android.synthetic.*'
excludeImports: 'java.util.*,kotlinx.android.synthetic.*'
19 changes: 19 additions & 0 deletions ...src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektNewIntegrationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.novoda.staticanalysis.internal.detekt


import com.novoda.test.Fixtures
import com.novoda.test.TestProject
import com.novoda.test.TestProjectRule
Expand All @@ -14,12 +15,16 @@ import static com.novoda.test.LogsSubject.assertThat
class DetektNewIntegrationTest {

private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.'
private static final String XML_REPORT_NOT_ENABLED = 'XML report must be enabled. Please make sure to enable "reports.xml" in your Detekt configuration'


@Parameterized.Parameters(name = "{0} with Detekt: {1}")
static Iterable rules() {
return [
[TestProjectRule.forKotlinProject(), "1.0.0.RC9.2"],
[TestProjectRule.forAndroidKotlinProject(), "1.0.0.RC9.2"],
[TestProjectRule.forKotlinProject(), "1.0.0-RC10"],
[TestProjectRule.forAndroidKotlinProject(), "1.0.0-RC10"],
]*.toArray()
}

Expand All @@ -41,6 +46,20 @@ class DetektNewIntegrationTest {
assertThat(result.logs).contains(DETEKT_NOT_APPLIED)
}

@Test
void shouldFailBuildOnConfigurationWhenDetektConfiguredWithoutXmlReport() {
def result = projectRule.newProject()
.withPlugin("io.gitlab.arturbosch.detekt", detektVersion)
.withToolsConfig('''detekt {
reports {
xml.enabled = false
}
}''')
.buildAndFail('check')

assertThat(result.logs).contains(XML_REPORT_NOT_ENABLED)
}

@Test
void shouldFailBuildWhenDetektWarningsOverTheThreshold() {
def result = createProjectWithZeroThreshold(Fixtures.Detekt.SOURCES_WITH_WARNINGS)
Expand Down