Skip to content

Commit

Permalink
Use Lazy APIs in Tasks (#396)
Browse files Browse the repository at this point in the history
* Introduce BaseIntegrationTestKitSpec

* FindMethodReferencesSpec: use BaseIntegrationTestKitSpec

* MultiProjectCircularDependencyRuleSpec: use BaseIntegrationTestKitSpec

* Use lazy registration for lintGradleTask

* FixGradleLintTask: use lazy apis

* GradleLintReportTask.groovy: use lazy apis
  • Loading branch information
rpalcolea committed Nov 21, 2023
1 parent 322d21b commit 526924c
Show file tree
Hide file tree
Showing 30 changed files with 166 additions and 411 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.netflix.nebula.lint.StyledTextService
import org.eclipse.jgit.api.ApplyCommand
import org.gradle.api.DefaultTask
import org.gradle.api.GradleException
import org.gradle.api.provider.ListProperty
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.Optional
Expand All @@ -32,24 +33,22 @@ import org.gradle.api.tasks.VerificationTask

import static com.netflix.nebula.lint.StyledTextService.Styling.*

class FixGradleLintTask extends DefaultTask implements VerificationTask {
abstract class FixGradleLintTask extends DefaultTask implements VerificationTask {
@Input
@Optional
List<GradleLintViolationAction> userDefinedListeners = []
abstract ListProperty<GradleLintViolationAction> getUserDefinedListeners()

/**
* Special listener tied into nebula.metrics via nebula.info to ship violation information to a
* metrics endpoint
*/
@Internal
GradleLintInfoBrokerAction infoBrokerAction = new GradleLintInfoBrokerAction(project)
GradleLintInfoBrokerAction infoBrokerAction

/**
* Whether or not the build should break when the verifications performed by this task fail.
*/
boolean ignoreFailures

FixGradleLintTask() {
infoBrokerAction = new GradleLintInfoBrokerAction(project)
userDefinedListeners.convention([])
outputs.upToDateWhen { false }
group = 'lint'
}
Expand All @@ -59,7 +58,7 @@ class FixGradleLintTask extends DefaultTask implements VerificationTask {
def violations = new LintService().lint(project, false).violations
.unique { v1, v2 -> v1.is(v2) ? 0 : 1 }

(getUserDefinedListeners() + infoBrokerAction + new GradleLintPatchAction(project)).each {
(userDefinedListeners.get() + infoBrokerAction + new GradleLintPatchAction(project)).each {
it.lintFinished(violations)
}

Expand All @@ -68,7 +67,7 @@ class FixGradleLintTask extends DefaultTask implements VerificationTask {
new ApplyCommand(new NotNecessarilyGitRepository(project.projectDir)).setPatch(patchFile.newInputStream()).call()
}

(getUserDefinedListeners() + infoBrokerAction + consoleOutputAction()).each {
(userDefinedListeners.get() + infoBrokerAction + consoleOutputAction()).each {
it.lintFixesApplied(violations)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,37 @@ abstract class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConf
@Override
void createTasks(Project project, GradleLintExtension lintExt) {
if (project.rootProject == project) {
def autoLintTask = project.tasks.create(AUTO_LINT_GRADLE, LintGradleTask)
autoLintTask.listeners = lintExt.listeners
def autoLintTask = project.tasks.register(AUTO_LINT_GRADLE, LintGradleTask)
autoLintTask.configure {
group = LINT_GROUP
listeners.set(lintExt.listeners)
projectRootDir.set(project.rootDir)
}

def manualLintTask = project.tasks.register(LINT_GRADLE, LintGradleTask) {
def manualLintTask = project.tasks.register(LINT_GRADLE, LintGradleTask)
manualLintTask.configure {
group = LINT_GROUP
failOnWarning = true
failOnWarning.set(true)
projectRootDir.set(project.rootDir)
}


def criticalLintTask = project.tasks.register(CRITICAL_LINT_GRADLE, LintGradleTask) {
def criticalLintTask = project.tasks.register(CRITICAL_LINT_GRADLE, LintGradleTask)
criticalLintTask.configure {
group = LINT_GROUP
onlyCriticalRules = true
onlyCriticalRules.set(true)
projectRootDir.set(project.rootDir)
}


def fixTask = project.tasks.register(FIX_GRADLE_LINT, FixGradleLintTask) {
userDefinedListeners = lintExt.listeners
def fixTask = project.tasks.register(FIX_GRADLE_LINT, FixGradleLintTask)
fixTask.configure {
userDefinedListeners.set(lintExt.listeners)
}

def fixTask2 = project.tasks.register(FIX_LINT_GRADLE, FixGradleLintTask) {
userDefinedListeners = lintExt.listeners
def fixTask2 = project.tasks.register(FIX_LINT_GRADLE, FixGradleLintTask)
fixTask2.configure {
userDefinedListeners.set(lintExt.listeners)
}

List<TaskProvider> lintTasks = [fixTask, fixTask2, manualLintTask]
Expand Down Expand Up @@ -71,7 +81,7 @@ abstract class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConf
}
}

protected void configureAutoLint(Task autoLintTask, Project project, GradleLintExtension lintExt, List<TaskProvider> lintTasks, TaskProvider criticalLintTask) {
protected void configureAutoLint(TaskProvider<LintGradleTask> autoLintTask, Project project, GradleLintExtension lintExt, List<TaskProvider> lintTasks, TaskProvider criticalLintTask) {
List<TaskProvider> lintTasksToVerify = lintTasks + criticalLintTask
project.afterEvaluate {
if (lintExt.autoLintAfterFailure) {
Expand All @@ -90,7 +100,7 @@ abstract class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConf
* @param lintExt
* @param lintTasksToVerify
*/
protected void configureAutoLintWithFailures(Task autoLintTask, Project project, GradleLintExtension lintExt, List<TaskProvider> lintTasksToVerify) {
protected void configureAutoLintWithFailures(TaskProvider<LintGradleTask> autoLintTask, Project project, GradleLintExtension lintExt, List<TaskProvider> lintTasksToVerify) {
boolean hasExplicitLintTask = project.gradle.startParameter.taskNames.any { lintTasksToVerify.name.contains(it) }
if (!hasValidTaskConfiguration(project, lintExt) || hasExplicitLintTask) {
return
Expand All @@ -108,7 +118,7 @@ abstract class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConf
* @param lintTasks
* @param criticalLintTask
*/
protected void configureAutoLintWithoutFailures(Task autoLintTask, Project project, GradleLintExtension lintExt, List<TaskProvider> lintTasks, TaskProvider criticalLintTask) {
protected void configureAutoLintWithoutFailures(TaskProvider<LintGradleTask> autoLintTask, Project project, GradleLintExtension lintExt, List<TaskProvider> lintTasks, TaskProvider criticalLintTask) {
project.gradle.taskGraph.whenReady { taskGraph ->
List<Task> allTasks = taskGraph.allTasks
if (hasValidTaskConfiguration(project, lintExt)) {
Expand All @@ -126,7 +136,7 @@ abstract class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConf
return
}
if (task.path == lastTask.path && !taskState.failure) {
autoLintTask.lint()
autoLintTask.get().lint()
}
}
})
Expand All @@ -141,7 +151,7 @@ abstract class GradleLintPluginTaskConfigurer extends AbstractLintPluginTaskConf
* @param lintTasks
* @param autoLintTask
*/
protected void finalizeAllTasksWithAutoLint(Project project, List<TaskProvider> lintTasks, Task autoLintTask, GradleLintExtension lintExt) {
protected void finalizeAllTasksWithAutoLint(Project project, List<TaskProvider> lintTasks, TaskProvider<LintGradleTask> autoLintTask, GradleLintExtension lintExt) {
project.tasks.configureEach { task ->
boolean skipForSpecificTask = lintExt.skipForTasks.any { taskToSkip -> task.name.endsWith(taskToSkip) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ import org.gradle.api.DefaultTask
import org.gradle.api.GradleException
import org.gradle.api.plugins.quality.CodeNarcReports
import org.gradle.api.plugins.quality.internal.CodeNarcReportsImpl
import org.gradle.api.provider.Property
import org.gradle.api.reporting.Report
import org.gradle.api.reporting.Reporting
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.Nested
import org.gradle.api.tasks.TaskAction
import org.gradle.api.tasks.VerificationTask
Expand All @@ -44,29 +46,16 @@ import javax.inject.Inject

import static com.netflix.nebula.lint.StyledTextService.Styling.Bold

class GradleLintReportTask extends DefaultTask implements VerificationTask, Reporting<CodeNarcReports> {
abstract class GradleLintReportTask extends DefaultTask implements VerificationTask, Reporting<CodeNarcReports> {

@Nested
private final CodeNarcReportsImpl reports
@Input
boolean reportOnlyFixableViolations

/**
* Whether or not the build should break when the verifications performed by this task fail.
*/
boolean ignoreFailures
@Input
abstract Property<Boolean> getReportOnlyFixableViolations()

GradleLintReportTask() {
CodeNarcReportsImpl codeNarcReports
if (GradleVersion.version(project.gradle.gradleVersion).compareTo(GradleVersion.version('4.4.1')) > 0) {
codeNarcReports = project.objects.newInstance(CodeNarcReportsImpl.class, this)
} else {
//TODO: remove this once we don't have customers in Gradle 4.1
DeprecationLoggerUtils.whileDisabled() {
codeNarcReports = instantiator.newInstance(CodeNarcReportsImpl, this)
}
}
reports = codeNarcReports
reports = project.objects.newInstance(CodeNarcReportsImpl.class, this)
outputs.upToDateWhen { false }
group = 'lint'
}
Expand Down Expand Up @@ -138,7 +127,7 @@ class GradleLintReportTask extends DefaultTask implements VerificationTask, Repo
}

void filterOnlyFixableViolations(Results results) {
if (reportOnlyFixableViolations) {
if (reportOnlyFixableViolations.isPresent() && reportOnlyFixableViolations.get()) {
new GradleLintPatchAction(project).lintFinished(results.violations)
List<Violation> toRemove = results.violations.findAll {
it.fixes.size() == 0 || it.fixes.any { it.reasonForNotFixing != null }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class GradleSevenZeroLintPluginTaskConfigurer extends GradleLintPluginTaskConfig
new Action<GradleLintReportTask>() {
@Override
void execute(GradleLintReportTask gradleLintReportTask) {
gradleLintReportTask.reportOnlyFixableViolations = getReportOnlyFixableViolations(project, extension)
gradleLintReportTask.reportOnlyFixableViolations.set(getReportOnlyFixableViolations(project, extension))
gradleLintReportTask.reports.all { report ->
report.conventionMapping.with {
enabled = { report.name == getReportFormat(project, extension) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,35 @@ import com.netflix.nebula.lint.*
import org.gradle.api.DefaultTask
import org.gradle.api.GradleException
import org.gradle.api.Task
import org.gradle.api.provider.ListProperty
import org.gradle.api.provider.Property
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.Optional
import org.gradle.api.tasks.TaskAction

import static com.netflix.nebula.lint.StyledTextService.Styling.*

class LintGradleTask extends DefaultTask {
abstract class LintGradleTask extends DefaultTask {
@Input
@Optional
List<GradleLintViolationAction> listeners = []
abstract ListProperty<GradleLintViolationAction> getListeners()

@Input
@Optional
Boolean failOnWarning = false
abstract Property<Boolean> getFailOnWarning()

@Input
@Optional
Boolean onlyCriticalRules = false
abstract Property<Boolean> getOnlyCriticalRules()

@Input
abstract Property<File> getProjectRootDir()

LintGradleTask() {
listeners.convention([])
failOnWarning.convention(false)
onlyCriticalRules.convention(false)
group = 'lint'
try {
def method = Task.getMethod("notCompatibleWithConfigurationCache")
Expand All @@ -50,10 +58,10 @@ class LintGradleTask extends DefaultTask {

@TaskAction
void lint() {
def violations = new LintService().lint(project, getOnlyCriticalRules()).violations
def violations = new LintService().lint(project, onlyCriticalRules.get()).violations
.unique { v1, v2 -> v1.is(v2) ? 0 : 1 }

(getListeners() + new GradleLintPatchAction(project) + new GradleLintInfoBrokerAction(project) + consoleOutputAction).each {
(listeners.get() + new GradleLintPatchAction(project) + new GradleLintInfoBrokerAction(project) + consoleOutputAction).each {
it.lintFinished(violations)
}
}
Expand Down Expand Up @@ -82,7 +90,7 @@ class LintGradleTask extends DefaultTask {
violations.groupBy { it.file }.each { buildFile, violationsByFile ->

violationsByFile.each { v ->
String buildFilePath = project.rootDir.toURI().relativize(v.file.toURI()).toString()
String buildFilePath = projectRootDir.get().toURI().relativize(v.file.toURI()).toString()
if (v.rule.priority == 1) {
textOutput.withStyle(Red).text('error'.padRight(10))
} else {
Expand Down Expand Up @@ -120,7 +128,7 @@ class LintGradleTask extends DefaultTask {
throw new GradleException("This build contains $errors critical lint violation${errors == 1 ? '' : 's'}")
}

if (getFailOnWarning()) {
if (failOnWarning.get()) {
throw new GradleException("This build contains $warnings lint violation${warnings == 1 ? '' : 's'}")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class LintPluginTaskConfigurer extends GradleLintPluginTaskConfigurer {
new Action<GradleLintReportTask>() {
@Override
void execute(GradleLintReportTask gradleLintReportTask) {
gradleLintReportTask.reportOnlyFixableViolations = getReportOnlyFixableViolations(project, extension)
gradleLintReportTask.reportOnlyFixableViolations.set(getReportOnlyFixableViolations(project, extension))
gradleLintReportTask.reports.all { report ->
def fileSuffix = report.name == 'text' ? 'txt' : report.name
report.conventionMapping.with {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.netflix.nebula.lint

import nebula.test.IntegrationTestKitSpec

abstract class BaseIntegrationTestKitSpec extends IntegrationTestKitSpec {
def setup() {
// Enable configuration cache :)
//new File(projectDir, 'gradle.properties') << '''org.gradle.configuration-cache=true'''.stripIndent()
}


void disableConfigurationCache() {
def propertiesFile = new File(projectDir, 'gradle.properties')
if(propertiesFile.exists()) {
propertiesFile.delete()
}
propertiesFile.createNewFile()
propertiesFile << '''org.gradle.configuration-cache=false'''.stripIndent()
}
}
Loading

0 comments on commit 526924c

Please sign in to comment.