Skip to content

Commit f07d95b

Browse files
jkschneiderjkschneider
committed
Make subproject lint and correction tasks defer to the root project so that results appear at the end
1 parent 9a6096c commit f07d95b

File tree

10 files changed

+231
-141
lines changed

10 files changed

+231
-141
lines changed

‎build.gradle‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ dependencies {
5555

5656
compile 'org.codenarc:CodeNarc:latest.release'
5757

58+
// these two dependencies exist so we can provide a test harness
59+
// to proprietary rule implementations packed in other jars
5860
provided ('org.spockframework:spock-core:latest.release') {
5961
exclude module: 'groovy-all'
6062
}

‎src/integTest/java/com/netflix/nebula/lint/plugin/GradleLintPluginSpec.groovy‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,62 @@ class GradleLintPluginSpec extends IntegrationSpec {
7979
""".toString()
8080
}
8181

82+
def 'auto correct all violations on a multi-module project'() {
83+
when:
84+
buildFile.text = """
85+
allprojects {
86+
apply plugin: ${GradleLintPlugin.name}
87+
gradleLint.rules = ['dependency-parentheses', 'dependency-tuple']
88+
}
89+
90+
subprojects {
91+
apply plugin: 'java'
92+
dependencies {
93+
compile('com.google.guava:guava:18.0')
94+
}
95+
}
96+
"""
97+
98+
def subDir = addSubproject('sub', """
99+
dependencies {
100+
testCompile group: 'junit',
101+
name: 'junit',
102+
version: '4.11'
103+
}
104+
105+
task taskA {}
106+
""")
107+
108+
then:
109+
def results = runTasksSuccessfully(':sub:fixGradleLint') // prove that this links to the root project task
110+
println results.standardOutput
111+
112+
buildFile.text == """
113+
allprojects {
114+
apply plugin: ${GradleLintPlugin.name}
115+
gradleLint.rules = ['dependency-parentheses', 'dependency-tuple']
116+
}
117+
118+
subprojects {
119+
apply plugin: 'java'
120+
dependencies {
121+
compile 'com.google.guava:guava:18.0'
122+
}
123+
}
124+
""".toString()
125+
126+
new File(subDir, 'build.gradle').text == """
127+
dependencies {
128+
testCompile 'junit:junit:4.11'
129+
}
130+
131+
task taskA {}
132+
"""
133+
}
134+
82135
def 'rules relative to each project'() {
83136
when:
84-
buildFile << """
137+
buildFile.text = """
85138
allprojects {
86139
apply plugin: ${GradleLintPlugin.name}
87140
gradleLint.rules = ['dependency-parentheses', 'dependency-tuple']
@@ -110,6 +163,7 @@ class GradleLintPluginSpec extends IntegrationSpec {
110163

111164
when:
112165
def console = results.standardOutput.readLines()
166+
println results.standardOutput
113167

114168
then:
115169
console.findAll { it.startsWith('warning') }.size() == 2

‎src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintCorrectionTask.groovy‎

Lines changed: 74 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.netflix.nebula.lint.rule.GradleViolation
66
import org.codenarc.rule.Rule
77
import org.codenarc.rule.Violation
88
import org.gradle.api.DefaultTask
9+
import org.gradle.api.Project
910
import org.gradle.api.tasks.TaskAction
1011
import org.gradle.logging.StyledTextOutput
1112
import org.gradle.logging.StyledTextOutputFactory
@@ -18,102 +19,97 @@ class GradleLintCorrectionTask extends DefaultTask {
1819
null // see http://gradle.1045684.n5.nabble.com/injecting-dependencies-into-task-instances-td5712637.html
1920
}
2021

21-
/**
22-
* Clean up any ugly artifacts we may have created through the auto-fix process
23-
* (e.g. empty extension object closures that have had all their property definitions removed)
24-
*/
25-
void postProcess() {
26-
def ruleSet = RuleSetFactory.configureRuleSet([new EmptyClosureRule()])
22+
void performCorrections(Project p, CorrectableStringSourceAnalyzer analyzer) {
23+
p.buildFile.text = analyzer.corrected // perform initial correction
2724

28-
def analyzer = new CorrectableStringSourceAnalyzer(project.buildFile.text)
29-
analyzer.analyze(ruleSet)
25+
// Clean up any ugly artifacts we may have created through the auto-fix process
26+
// (e.g. empty extension object closures that have had all their property definitions removed)
27+
def ruleSet = RuleSetFactory.configureRuleSet([new EmptyClosureRule()])
28+
def postProcessor = new CorrectableStringSourceAnalyzer(p.buildFile.text)
29+
postProcessor.analyze(ruleSet)
3030

31-
project.buildFile.newWriter().withWriter { w ->
32-
w << analyzer.corrected
33-
}
31+
p.buildFile.text = postProcessor.corrected
3432
}
3533

3634
@TaskAction
3735
void lintCorrections() {
38-
if(!project.buildFile.exists()) {
39-
return
40-
}
41-
42-
def registry = new LintRuleRegistry(project)
43-
def ruleSet = RuleSetFactory.configureRuleSet(project.extensions
44-
.getByType(GradleLintExtension)
45-
.rules
46-
.collect { registry.buildRules(it) }
47-
.flatten() as List<Rule>)
48-
4936
// look at org.gradle.logging.internal.DefaultColorMap
5037
def textOutput = textOutputFactory.create('lint')
38+
def registry = new LintRuleRegistry()
5139

52-
def analyzer = new CorrectableStringSourceAnalyzer(project.buildFile.text)
53-
def results = analyzer.analyze(ruleSet)
40+
def violationsByProject = [:]
5441

55-
if(results.violations.isEmpty()) {
56-
textOutput.style(StyledTextOutput.Style.Identifier).println("Passed lint check with 0 violations; no corrections necessary")
57-
return
58-
}
42+
([project] + project.subprojects).each { p ->
43+
if (p.buildFile.exists()) {
44+
def ruleSet = RuleSetFactory.configureRuleSet(p.extensions
45+
.getByType(GradleLintExtension)
46+
.rules
47+
.collect { registry.buildRules(it, p) }
48+
.flatten() as List<Rule>)
5949

60-
// perform initial correction
61-
project.buildFile.newWriter().withWriter { w ->
62-
w << analyzer.corrected
63-
}
64-
postProcess()
50+
def analyzer = new CorrectableStringSourceAnalyzer(p.buildFile.text)
51+
def results = analyzer.analyze(ruleSet)
6552

66-
printReport(results.violations, textOutput)
67-
}
53+
performCorrections(p, analyzer)
54+
violationsByProject[p] = results.violations
55+
}
56+
}
6857

69-
private List printReport(List<Violation> violations, textOutput) {
58+
def allViolations = violationsByProject.values().flatten()
7059
def correctedViolations = 0, uncorrectedViolations = 0
71-
def buildFilePath = relPath(project.rootDir, project.buildFile).path
72-
73-
violations.eachWithIndex { v, i ->
74-
def severity = v.rule.priority <= 3 ? 'warning' : 'error'
7560

76-
if (v instanceof GradleViolation && v.isFixable()) {
77-
textOutput.withStyle(StyledTextOutput.Style.Identifier).text('fixed'.padRight(10))
78-
} else {
79-
textOutput.withStyle(StyledTextOutput.Style.Failure).text(severity.padRight(10))
80-
}
81-
82-
textOutput.text(v.rule.ruleId.padRight(35))
83-
textOutput.withStyle(StyledTextOutput.Style.Description).println(v.message)
84-
85-
textOutput.withStyle(StyledTextOutput.Style.UserInput).println(buildFilePath + ':' + v.lineNumber)
86-
textOutput.println("$v.sourceLine")
87-
88-
if (v instanceof GradleViolation && v.isFixable()) {
89-
if (v.replacement) {
90-
textOutput.withStyle(StyledTextOutput.Style.UserInput).println('replaced with:')
91-
textOutput.println(v.replacement)
92-
correctedViolations++
93-
} else if (v.deleteLine) {
94-
textOutput.withStyle(StyledTextOutput.Style.UserInput).println("deleted line $v.deleteLine")
95-
correctedViolations++
96-
} else if (v.addition) {
97-
textOutput.withStyle(StyledTextOutput.Style.UserInput).println("adding:")
98-
textOutput.print(v.addition)
99-
correctedViolations++
61+
if(allViolations.isEmpty()) {
62+
textOutput.style(StyledTextOutput.Style.Identifier).println("Passed lint check with 0 violations; no corrections necessary")
63+
} else {
64+
textOutput.withStyle(StyledTextOutput.Style.UserInput).text('\nThis project contains lint violations. ')
65+
textOutput.println('A complete listing of my attempt to fix them follows. Please review and commit the changes.\n')
66+
67+
violationsByProject.entrySet().each {
68+
def buildFilePath = it.key.rootDir.toURI().relativize(it.key.buildFile.toURI()).toString()
69+
def violations = it.value
70+
71+
violations.each { Violation v ->
72+
def severity = v.rule.priority <= 3 ? 'warning' : 'error'
73+
74+
if (v instanceof GradleViolation && v.isFixable()) {
75+
textOutput.withStyle(StyledTextOutput.Style.Identifier).text('fixed'.padRight(10))
76+
} else {
77+
textOutput.withStyle(StyledTextOutput.Style.Failure).text(severity.padRight(10))
78+
}
79+
80+
textOutput.text(v.rule.ruleId.padRight(35))
81+
textOutput.withStyle(StyledTextOutput.Style.Description).println(v.message)
82+
83+
textOutput.withStyle(StyledTextOutput.Style.UserInput).println(buildFilePath + ':' + v.lineNumber)
84+
textOutput.println("$v.sourceLine")
85+
86+
if (v instanceof GradleViolation && v.isFixable()) {
87+
if (v.replacement) {
88+
textOutput.withStyle(StyledTextOutput.Style.UserInput).println('replaced with:')
89+
textOutput.println(v.replacement)
90+
correctedViolations++
91+
} else if (v.deleteLine) {
92+
textOutput.withStyle(StyledTextOutput.Style.UserInput).println("deleted line $v.deleteLine")
93+
correctedViolations++
94+
} else if (v.addition) {
95+
textOutput.withStyle(StyledTextOutput.Style.UserInput).println("adding:")
96+
textOutput.print(v.addition)
97+
correctedViolations++
98+
}
99+
} else {
100+
textOutput.withStyle(StyledTextOutput.Style.Error).println('\u2716 no auto-correct available')
101+
uncorrectedViolations++
102+
}
103+
104+
textOutput.println() // extra space between violations
100105
}
101-
} else {
102-
textOutput.withStyle(StyledTextOutput.Style.Error).println('\u2716 no auto-correct available')
103-
uncorrectedViolations++
104106
}
105107

106-
textOutput.println() // extra space between violations
107-
}
108-
109-
if(correctedViolations > 0)
110-
textOutput.style(StyledTextOutput.Style.Identifier).println("Corrected $correctedViolations lint problems")
108+
if(correctedViolations > 0)
109+
textOutput.style(StyledTextOutput.Style.Identifier).println("Corrected $correctedViolations lint problems\n")
111110

112-
if(uncorrectedViolations > 0)
113-
textOutput.style(StyledTextOutput.Style.Error).println("Corrected $correctedViolations lint problems")
114-
}
115-
116-
private static File relPath(File root, File f) {
117-
new File(root.toURI().relativize(f.toURI()).toString())
111+
if(uncorrectedViolations > 0)
112+
textOutput.style(StyledTextOutput.Style.Error).println("Corrected $correctedViolations lint problems\n")
113+
}
118114
}
119115
}

‎src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintPlugin.groovy‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,33 @@ import org.gradle.api.Project
55

66
class GradleLintPlugin implements Plugin<Project> {
77
private final exemptTasks = ['help', 'tasks', 'dependencies', 'dependencyInsight',
8-
'components', 'model', 'projects', 'properties']
8+
'components', 'model', 'projects', 'properties', 'fixGradleLint']
99

1010
@Override
1111
void apply(Project project) {
1212
LintRuleRegistry.classLoader = getClass().classLoader
1313
def lintExt = project.extensions.create('gradleLint', GradleLintExtension)
1414

15-
project.tasks.create('fixGradleLint', GradleLintCorrectionTask)
16-
def lint = project.tasks.create('gradleLint', GradleLintTask)
17-
configureReportTask(project, lintExt)
15+
// TODO
16+
// 1. Make gradleLint and fixGradleLint on root run against all subprojects
17+
// 2. Only automatically add root project's gradle lint the end of the build
18+
19+
if(project.rootProject == project) {
20+
project.tasks.create('fixGradleLint', GradleLintCorrectionTask)
21+
project.tasks.create('gradleLint', GradleLintTask)
22+
project.rootProject.apply plugin: GradleLintPlugin
23+
} else {
24+
project.tasks.create('gradleLint') // this task does nothing
25+
project.tasks.create('fixGradleLint').finalizedBy project.rootProject.tasks.getByName('fixGradleLint')
26+
}
1827

19-
project.rootProject.apply plugin: GradleLintPlugin
20-
def rootLint = project.rootProject.tasks.getByName('gradleLint')
28+
configureReportTask(project, lintExt)
2129

2230
// ensure that lint runs
2331
project.tasks.whenTaskAdded { task ->
24-
if(task != lint && !exemptTasks.contains(task.name)) {
25-
task.finalizedBy lint
26-
lint.shouldRunAfter task
27-
28-
// when running a lint-eligible task on a subproject, we want to lint the root project as well
32+
def rootLint = project.rootProject.tasks.getByName('gradleLint')
33+
if (task != rootLint && !exemptTasks.contains(task.name)) {
34+
// when running a lint-eligible task on a subproject, we want to lint the whole project
2935
task.finalizedBy rootLint
3036
rootLint.shouldRunAfter task
3137
}

‎src/main/groovy/com/netflix/nebula/lint/plugin/GradleLintReportTask.groovy‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,10 @@ class GradleLintReportTask extends DefaultTask implements VerificationTask, Repo
3535

3636
@TaskAction
3737
void generateReport() {
38-
if(!project.buildFile.exists()) {
39-
return
40-
}
41-
4238
if(reports.enabled) {
4339
def lintExt = project.extensions.getByType(GradleLintExtension)
44-
def registry = new LintRuleRegistry(project)
45-
def ruleSet = RuleSetFactory.configureRuleSet(lintExt.rules.collect { registry.buildRules(it) }.flatten() as List<Rule>)
40+
def registry = new LintRuleRegistry()
41+
def ruleSet = RuleSetFactory.configureRuleSet(lintExt.rules.collect { registry.buildRules(it, project) }.flatten() as List<Rule>)
4642
def results = new FilesystemSourceAnalyzer(baseDirectory: project.projectDir.absolutePath,
4743
includes: project.buildFile.absolutePath).analyze(ruleSet)
4844

0 commit comments

Comments
 (0)