From 04c2d80c69812ef75e300360bbe55c6d0cf1ad20 Mon Sep 17 00:00:00 2001 From: Roberto Perez Alcolea Date: Mon, 13 Sep 2021 16:37:16 -0700 Subject: [PATCH] Upgrade to Gradle 7.2 and fix report generation (#350) * Gradle 7.2 * Fix Gradle lint report generation with Gradle 7.2 --- gradle/wrapper/gradle-wrapper.properties | 2 +- .../AbstractLintPluginTaskConfigurer.groovy | 23 +- ...e7AndHigherLintPluginTaskConfigurer.groovy | 23 ++ ...etween5And7LintPluginTaskConfigurer.groovy | 31 ++ .../lint/plugin/GradleLintPlugin.groovy | 7 +- .../GradleLintPluginTaskConfigurer.groovy | 13 +- .../lint/plugin/GradleLintReportTask.groovy | 17 +- ...egacyGradleLintPluginTaskConfigurer.groovy | 20 + .../plugin/Gradle5LintReportTaskSpec.groovy | 342 ++++++++++++++++++ .../plugin/Gradle6LintReportTaskSpec.groovy | 342 ++++++++++++++++++ 10 files changed, 787 insertions(+), 33 deletions(-) create mode 100644 src/main/groovy/com/netflix/nebula/lint/plugin/Gradle7AndHigherLintPluginTaskConfigurer.groovy create mode 100644 src/main/groovy/com/netflix/nebula/lint/plugin/GradleBetween5And7LintPluginTaskConfigurer.groovy create mode 100644 src/test/groovy/com/netflix/nebula/lint/plugin/Gradle5LintReportTaskSpec.groovy create mode 100644 src/test/groovy/com/netflix/nebula/lint/plugin/Gradle6LintReportTaskSpec.groovy diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index f371643e..ffed3a25 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-7.0-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-7.2-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/src/main/groovy/com/netflix/nebula/lint/plugin/AbstractLintPluginTaskConfigurer.groovy b/src/main/groovy/com/netflix/nebula/lint/plugin/AbstractLintPluginTaskConfigurer.groovy index de4d8878..83df090f 100644 --- a/src/main/groovy/com/netflix/nebula/lint/plugin/AbstractLintPluginTaskConfigurer.groovy +++ b/src/main/groovy/com/netflix/nebula/lint/plugin/AbstractLintPluginTaskConfigurer.groovy @@ -31,6 +31,7 @@ abstract class AbstractLintPluginTaskConfigurer { } abstract void wireJavaPlugin(Project project) + abstract Action configureReportAction(Project project, GradleLintExtension extension) protected static boolean hasValidTaskConfiguration(Project project, GradleLintExtension lintExt) { boolean shouldLint = project.hasProperty('gradleLint.alwaysRun') ? @@ -50,29 +51,11 @@ abstract class AbstractLintPluginTaskConfigurer { return tasks.any { it == criticalLintTask && it.state.failure != null } } - protected Action configureReportAction(Project project, GradleLintExtension extension) { - new Action() { - @Override - void execute(GradleLintReportTask gradleLintReportTask) { - gradleLintReportTask.reportOnlyFixableViolations = getReportOnlyFixableViolations(project, extension) - gradleLintReportTask.reports.all { report -> - report.conventionMapping.with { - enabled = { report.name == getReportFormat(project, extension) } - destination = { - def fileSuffix = report.name == 'text' ? 'txt' : report.name - new File(project.buildDir, "reports/gradleLint/${project.name}.$fileSuffix") - } - } - } - } - } - } - - private static String getReportFormat(Project project, GradleLintExtension extension) { + protected static String getReportFormat(Project project, GradleLintExtension extension) { return project.hasProperty('gradleLint.reportFormat') ? project.property('gradleLint.reportFormat') : extension.reportFormat } - private static boolean getReportOnlyFixableViolations(Project project, GradleLintExtension extension) { + protected static boolean getReportOnlyFixableViolations(Project project, GradleLintExtension extension) { return project.hasProperty('gradleLint.reportOnlyFixableViolations') ? Boolean.valueOf(project.property('gradleLint.reportOnlyFixableViolations').toString()) : extension.reportOnlyFixableViolations } } diff --git a/src/main/groovy/com/netflix/nebula/lint/plugin/Gradle7AndHigherLintPluginTaskConfigurer.groovy b/src/main/groovy/com/netflix/nebula/lint/plugin/Gradle7AndHigherLintPluginTaskConfigurer.groovy new file mode 100644 index 00000000..cc971763 --- /dev/null +++ b/src/main/groovy/com/netflix/nebula/lint/plugin/Gradle7AndHigherLintPluginTaskConfigurer.groovy @@ -0,0 +1,23 @@ +package com.netflix.nebula.lint.plugin + +import org.gradle.api.Action +import org.gradle.api.Project + +class Gradle7AndHigherLintPluginTaskConfigurer extends GradleBetween5And7LintPluginTaskConfigurer { + @Override + Action configureReportAction(Project project, GradleLintExtension extension) { + new Action() { + @Override + void execute(GradleLintReportTask gradleLintReportTask) { + gradleLintReportTask.reportOnlyFixableViolations = getReportOnlyFixableViolations(project, extension) + gradleLintReportTask.reports.all { report -> + def fileSuffix = report.name == 'text' ? 'txt' : report.name + report.conventionMapping.with { + required.set(report.name == getReportFormat(project, extension)) + outputLocation.set(new File(project.buildDir, "reports/gradleLint/${project.name}.$fileSuffix")) + } + } + } + } + } +} diff --git a/src/main/groovy/com/netflix/nebula/lint/plugin/GradleBetween5And7LintPluginTaskConfigurer.groovy b/src/main/groovy/com/netflix/nebula/lint/plugin/GradleBetween5And7LintPluginTaskConfigurer.groovy new file mode 100644 index 00000000..dac937a2 --- /dev/null +++ b/src/main/groovy/com/netflix/nebula/lint/plugin/GradleBetween5And7LintPluginTaskConfigurer.groovy @@ -0,0 +1,31 @@ +package com.netflix.nebula.lint.plugin + +import org.gradle.api.Action +import org.gradle.api.Project +import org.gradle.api.Task +import org.gradle.api.execution.TaskExecutionListener +import org.gradle.api.plugins.JavaBasePlugin +import org.gradle.api.tasks.TaskProvider +import org.gradle.api.tasks.TaskState +import org.gradle.api.tasks.compile.AbstractCompile + +class GradleBetween5And7LintPluginTaskConfigurer extends GradleLintPluginTaskConfigurer{ + @Override + Action configureReportAction(Project project, GradleLintExtension extension) { + new Action() { + @Override + void execute(GradleLintReportTask gradleLintReportTask) { + gradleLintReportTask.reportOnlyFixableViolations = getReportOnlyFixableViolations(project, extension) + gradleLintReportTask.reports.all { report -> + report.conventionMapping.with { + enabled = { report.name == getReportFormat(project, extension) } + destination = { + def fileSuffix = report.name == 'text' ? 'txt' : report.name + new File(project.buildDir, "reports/gradleLint/${project.name}.$fileSuffix") + } + } + } + } + } + } +} diff --git a/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintPlugin.groovy b/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintPlugin.groovy index 431c66d6..254edf3f 100644 --- a/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintPlugin.groovy +++ b/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintPlugin.groovy @@ -32,8 +32,11 @@ class GradleLintPlugin implements Plugin { LintRuleRegistry.classLoader = getClass().classLoader - if (GradleKt.versionCompareTo(project.gradle, '5.0') >= 0) { - new GradleLintPluginTaskConfigurer().configure(project) + // TODO: we need to retire this once we have folks in Gradle 7.x+ + if (GradleKt.versionCompareTo(project.gradle, '7.0') >= 0) { + new Gradle7AndHigherLintPluginTaskConfigurer().configure(project) + } else if (GradleKt.versionCompareTo(project.gradle, '5.0') >= 0) { + new GradleBetween5And7LintPluginTaskConfigurer().configure(project) } else { new LegacyGradleLintPluginTaskConfigurer().configure(project) } diff --git a/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintPluginTaskConfigurer.groovy b/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintPluginTaskConfigurer.groovy index b8a3cefa..99882d10 100644 --- a/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintPluginTaskConfigurer.groovy +++ b/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintPluginTaskConfigurer.groovy @@ -9,7 +9,8 @@ import org.gradle.api.tasks.TaskProvider import org.gradle.api.tasks.TaskState import org.gradle.api.tasks.compile.AbstractCompile -class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConfigurer { +abstract class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConfigurer { + abstract Action configureReportAction(Project project, GradleLintExtension extension) @Override void createTasks(Project project, GradleLintExtension lintExt) { @@ -70,7 +71,7 @@ class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConfigurer { } } - private void configureAutoLint(Task autoLintTask, Project project, GradleLintExtension lintExt, List lintTasks, TaskProvider criticalLintTask) { + protected void configureAutoLint(Task autoLintTask, Project project, GradleLintExtension lintExt, List lintTasks, TaskProvider criticalLintTask) { List lintTasksToVerify = lintTasks + criticalLintTask project.afterEvaluate { if (lintExt.autoLintAfterFailure) { @@ -89,7 +90,7 @@ class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConfigurer { * @param lintExt * @param lintTasksToVerify */ - private void configureAutoLintWithFailures(Task autoLintTask, Project project, GradleLintExtension lintExt, List lintTasksToVerify) { + protected void configureAutoLintWithFailures(Task autoLintTask, Project project, GradleLintExtension lintExt, List lintTasksToVerify) { boolean hasExplicitLintTask = project.gradle.startParameter.taskNames.any { lintTasksToVerify.name.contains(it) } if (!hasValidTaskConfiguration(project, lintExt) || hasExplicitLintTask) { return @@ -107,7 +108,7 @@ class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConfigurer { * @param lintTasks * @param criticalLintTask */ - private void configureAutoLintWithoutFailures(Task autoLintTask, Project project, GradleLintExtension lintExt, List lintTasks, TaskProvider criticalLintTask) { + protected void configureAutoLintWithoutFailures(Task autoLintTask, Project project, GradleLintExtension lintExt, List lintTasks, TaskProvider criticalLintTask) { project.gradle.taskGraph.whenReady { taskGraph -> List allTasks = taskGraph.allTasks if (hasValidTaskConfiguration(project, lintExt)) { @@ -140,7 +141,7 @@ class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConfigurer { * @param lintTasks * @param autoLintTask */ - private void finalizeAllTasksWithAutoLint(Project project, List lintTasks, Task autoLintTask, GradleLintExtension lintExt) { + protected void finalizeAllTasksWithAutoLint(Project project, List lintTasks, Task autoLintTask, GradleLintExtension lintExt) { project.tasks.configureEach { task -> boolean skipForSpecificTask = lintExt.skipForTasks.any { taskToSkip -> task.name.endsWith(taskToSkip) } @@ -153,7 +154,7 @@ class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConfigurer { } } - private void configureReportTask(Project project, GradleLintExtension extension) { + protected void configureReportTask(Project project, GradleLintExtension extension) { TaskProvider reportTask = project.tasks.register(GENERATE_GRADLE_LINT_REPORT, GradleLintReportTask) reportTask.configure(configureReportAction(project, extension)) } diff --git a/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintReportTask.groovy b/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintReportTask.groovy index 8e06666d..210f3db0 100644 --- a/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintReportTask.groovy +++ b/src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintReportTask.groovy @@ -15,6 +15,7 @@ */ package com.netflix.nebula.lint.plugin +import com.netflix.nebula.interop.GradleKt import com.netflix.nebula.lint.GradleLintPatchAction import com.netflix.nebula.lint.StyledTextService import com.netflix.nebula.lint.utils.DeprecationLoggerUtils @@ -86,10 +87,18 @@ class GradleLintReportTask extends DefaultTask implements VerificationTask, Repo reports.enabled.each { Report r -> ReportWriter writer = null - switch (r.name) { - case 'xml': writer = new XmlReportWriter(outputFile: r.destination); break - case 'html': writer = new HtmlReportWriter(outputFile: r.destination); break - case 'text': writer = new TextReportWriter(outputFile: r.destination); break + if (GradleKt.versionCompareTo(project.gradle, '7.0') >= 0) { + switch (r.name) { + case 'xml': writer = new XmlReportWriter(outputFile: r.outputLocation.get().asFile); break + case 'html': writer = new HtmlReportWriter(outputFile: r.outputLocation.get().asFile); break + case 'text': writer = new TextReportWriter(outputFile: r.outputLocation.get().asFile); break + } + } else { + switch (r.name) { + case 'xml': writer = new XmlReportWriter(outputFile: r.destination); break + case 'html': writer = new HtmlReportWriter(outputFile: r.destination); break + case 'text': writer = new TextReportWriter(outputFile: r.destination); break + } } writer.writeReport(new AnalysisContext(ruleSet: lintService.ruleSet(project)), results) diff --git a/src/main/groovy/com/netflix/nebula/lint/plugin/LegacyGradleLintPluginTaskConfigurer.groovy b/src/main/groovy/com/netflix/nebula/lint/plugin/LegacyGradleLintPluginTaskConfigurer.groovy index 0901a3a0..46f54780 100644 --- a/src/main/groovy/com/netflix/nebula/lint/plugin/LegacyGradleLintPluginTaskConfigurer.groovy +++ b/src/main/groovy/com/netflix/nebula/lint/plugin/LegacyGradleLintPluginTaskConfigurer.groovy @@ -1,6 +1,7 @@ package com.netflix.nebula.lint.plugin import org.gradle.BuildResult +import org.gradle.api.Action import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.execution.TaskExecutionGraph @@ -87,4 +88,23 @@ class LegacyGradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConfigu private void configureReportTask(Project project, GradleLintExtension extension) { def task = project.tasks.create(GENERATE_GRADLE_LINT_REPORT, GradleLintReportTask, configureReportAction(project, extension)) } + + @Override + Action configureReportAction(Project project, GradleLintExtension extension) { + new Action() { + @Override + void execute(GradleLintReportTask gradleLintReportTask) { + gradleLintReportTask.reportOnlyFixableViolations = getReportOnlyFixableViolations(project, extension) + gradleLintReportTask.reports.all { report -> + report.conventionMapping.with { + enabled = { report.name == getReportFormat(project, extension) } + destination = { + def fileSuffix = report.name == 'text' ? 'txt' : report.name + new File(project.buildDir, "reports/gradleLint/${project.name}.$fileSuffix") + } + } + } + } + } + } } diff --git a/src/test/groovy/com/netflix/nebula/lint/plugin/Gradle5LintReportTaskSpec.groovy b/src/test/groovy/com/netflix/nebula/lint/plugin/Gradle5LintReportTaskSpec.groovy new file mode 100644 index 00000000..c2661dc3 --- /dev/null +++ b/src/test/groovy/com/netflix/nebula/lint/plugin/Gradle5LintReportTaskSpec.groovy @@ -0,0 +1,342 @@ +package com.netflix.nebula.lint.plugin + +import nebula.test.IntegrationTestKitSpec +import spock.lang.Issue + +class Gradle5LintReportTaskSpec extends IntegrationTestKitSpec { + def setup() { + debug = true + gradleVersion = '5.6.4' + } + + def 'generate a report'() { + when: + buildFile.text = """ + plugins { + id 'nebula.lint' + id 'java' + } + + gradleLint { + rules = ['dependency-parentheses'] + reportFormat = 'text' + } + + repositories { mavenCentral() } + + dependencies { + implementation('com.google.guava:guava:18.0') + } + """ + + then: + runTasks('generateGradleLintReport') + + when: + def report = new File(projectDir, 'build/reports/gradleLint').listFiles().find { it.name.endsWith('.txt') } + + then: + report.text.contains('Violation: Rule=dependency-parentheses') + report.text.contains('TotalFiles=1') + } + + def 'generate a report with only applied fixes'() { + given: + buildFile.text = """ + plugins { + id 'java' + id 'nebula.lint' + } + + gradleLint { + rules = ['dependency-tuple', 'dependency-parentheses'] + reportFormat = 'text' + reportOnlyFixableViolations = true + } + + repositories { mavenCentral() } + + dependencies { + implementation(group: 'com.google.guava', name: 'guava', version: '18.0') + } + """ + + when: + def result = runTasks('autoLintGradle') + + then: + result.output.contains("dependency-tuple") + result.output.contains("dependency-parentheses") + + then: + runTasks('generateGradleLintReport') + + when: + def report = new File(projectDir, 'build/reports/gradleLint').listFiles().find { it.name.endsWith('.txt') } + + then: + report.text.contains('Violation: Rule=dependency-tuple') + ! report.text.contains('Violation: Rule=dependency-parentheses') + report.text.contains('TotalFiles=1') + } + + def 'generate a report with different type through parameter from cli'() { + when: + buildFile.text = """ + plugins { + id 'nebula.lint' + id 'java' + } + + gradleLint { + rules = ['dependency-parentheses'] + } + + repositories { mavenCentral() } + + dependencies { + implementation('com.google.guava:guava:18.0') + } + """ + + then: + runTasks('generateGradleLintReport', '-PgradleLint.reportFormat=text') + + when: + def report = new File(projectDir, 'build/reports/gradleLint').listFiles().find { it.name.endsWith('.txt') } + + then: + report.text.contains('Violation: Rule=dependency-parentheses') + report.text.contains('TotalFiles=1') + } + + def 'generate a report with only applied fixes using cli param to enable it'() { + given: + buildFile.text = """ + plugins { + id 'java' + id 'nebula.lint' + } + + gradleLint { + rules = ['dependency-tuple', 'dependency-parentheses'] + reportFormat = 'text' + } + + repositories { mavenCentral() } + + dependencies { + implementation(group: 'com.google.guava', name: 'guava', version: '18.0') + } + """ + + when: + runTasks('generateGradleLintReport', '-PgradleLint.reportOnlyFixableViolations=true') + + then: + def report = new File(projectDir, 'build/reports/gradleLint').listFiles().find { it.name.endsWith('.txt') } + report.text.contains('Violation: Rule=dependency-tuple') + ! report.text.contains('Violation: Rule=dependency-parentheses') + report.text.contains('TotalFiles=1') + } + + def 'generate a report with all rules using cli param to disable it to override extension'() { + given: + buildFile.text = """ + plugins { + id 'java' + id 'nebula.lint' + } + + gradleLint { + rules = ['dependency-tuple', 'dependency-parentheses'] + reportFormat = 'text' + reportOnlyFixableViolations = true + } + + repositories { mavenCentral() } + + dependencies { + implementation(group: 'com.google.guava', name: 'guava', version: '18.0') + } + """ + + when: + runTasks('generateGradleLintReport', '-PgradleLint.reportOnlyFixableViolations=false') + + then: + def report = new File(projectDir, 'build/reports/gradleLint').listFiles().find { it.name.endsWith('.txt') } + report.text.contains('Violation: Rule=dependency-tuple') + report.text.contains('Violation: Rule=dependency-parentheses') + report.text.contains('TotalFiles=1') + } + + def 'warning without autofixes are not reported if flag is enabled'() { + given: + buildFile.text = """ + plugins { + id 'nebula.lint' + id 'java' + } + + gradleLint { + rules = ['duplicate-dependency-class'] + reportFormat = 'text' + reportOnlyFixableViolations = true + } + + repositories { mavenCentral() } + + dependencies { + implementation 'com.google.guava:guava:18.0' + implementation 'com.google.collections:google-collections:1.0' + } + """ + + when: + runTasks('generateGradleLintReport') + + then: + def report = new File(projectDir, 'build/reports/gradleLint').listFiles().find { it.name.endsWith('.txt') } + ! report.text.contains('Violation: Rule=duplicate-dependency-class') + } + + def 'critical rules fail task'() { + when: + buildFile.text = """ + plugins { + id 'java' + id 'nebula.lint' + } + + gradleLint.criticalRules = ['dependency-parentheses'] + + repositories { mavenCentral() } + + dependencies { + implementation('com.google.guava:guava:18.0') + } + """ + + then: + def results = runTasksAndFail('generateGradleLintReport') + results.output.contains('FAIL') + results.output.contains('Task failed with an exception') + } + + def 'generate a report for a multi-module project'() { + when: + buildFile.text = """ + plugins { + id 'nebula.lint' + } + """ + + def sub = addSubproject('sub') + new File(sub, 'build.gradle').text = """ + plugins { + id 'java' + } + + gradleLint { + rules = ['dependency-parentheses'] + reportFormat = 'text' + } + + repositories { mavenCentral() } + + dependencies { + implementation('com.google.guava:guava:18.0') + } + """ + + then: + runTasks('generateGradleLintReport') + + when: + def report = new File(projectDir, 'build/reports/gradleLint').listFiles().find { it.name.endsWith('.txt') } + + then: + report.text.contains('Violation: Rule=dependency-parentheses') + report.text.contains('TotalFiles=1') + } + + @Issue('#137') + def 'generate a report for a DependencyService based rule'() { + when: + buildFile.text = """ + plugins { + id 'nebula.lint' + id 'java' + } + + gradleLint { + rules = ['duplicate-dependency-class'] + reportFormat = 'text' + } + + repositories { mavenCentral() } + + dependencies { + implementation 'com.google.guava:guava:18.0' + implementation 'com.google.collections:google-collections:1.0' + } + """ + + then: + runTasks('generateGradleLintReport') + def report = new File(projectDir, 'build/reports/gradleLint').listFiles().find { it.name.endsWith('.txt') } + + then: + report.text.contains('Violation: Rule=duplicate-dependency-class') + report.text.contains('TotalFiles=1') + } + + def 'generate a xml report for a multi-module project'() { + when: + buildFile.text = """ + plugins { + id 'java' + id 'nebula.lint' + } + + allprojects { + gradleLint { + rules = ['dependency-parentheses'] + reportFormat = 'xml' + } + + repositories { mavenCentral() } + } + + dependencies { + implementation('com.google.guava:guava:19.0') + } + """ + + def sub = addSubproject('sub') + new File(sub, 'build.gradle').text = """ + plugins { + id 'java' + } + + dependencies { + implementation('com.google.guava:guava:18.0') + } + """ + + then: + runTasks('generateGradleLintReport') + + when: + def report = new File(projectDir, 'build/reports/gradleLint').listFiles().find { it.name.endsWith('.xml') } + + then: + def reportXml = report.text + reportXml.count('Violation ruleName=\'dependency-parentheses\'') == 2 + reportXml.contains('implementation(\'com.google.guava:guava:19.0\')') + reportXml.contains('implementation(\'com.google.guava:guava:18.0\')') + reportXml.contains("