Skip to content

Commit

Permalink
Merge pull request #13 from nebula-plugins/support-for-file-operations
Browse files Browse the repository at this point in the history
Add ability to create/delete files and symlinks for lint fixes
  • Loading branch information
nadavc committed Apr 29, 2016
2 parents ca9666a + b9b512e commit 34ed4f4
Show file tree
Hide file tree
Showing 18 changed files with 418 additions and 54 deletions.
18 changes: 11 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

## Purpose

The Gradle Lint plugin is a pluggable and configurable linter tool for identifying and reporting on patterns of misuse or deprecations in Gradle scripts. It is inspired by the excellent ESLint tool for Javascript and by the formatting in NPM's [eslint-friendly-formatter](https://www.npmjs.com/package/eslint-friendly-formatter) package.
The Gradle Lint plugin is a pluggable and configurable linter tool for identifying and reporting on patterns of misuse or deprecations in Gradle scripts and related files. It is inspired by the excellent ESLint tool for Javascript and by the formatting in NPM's [eslint-friendly-formatter](https://www.npmjs.com/package/eslint-friendly-formatter) package.

It assists a centralized build tools team in gently introducing and maintaining a standard build script style across their organization.

Expand All @@ -38,7 +38,7 @@ It assists a centralized build tools team in gently introducing and maintaining
To apply this plugin:

plugins {
id 'nebula.lint' version '0.22.0'
id 'nebula.lint' version '0.23.0'
}

Alternatively:
Expand Down Expand Up @@ -103,7 +103,9 @@ You can also be selective about which rules to ignore in the block by providing

## Building your own rules

A lint rule consists of a `GradleLintRule` implementation plus a properties file. Let's build a simple rule that blocks build scripts from applying an imaginary Gradle plugin `nebula.fix-jersey-bundle` that might do something like replace a jersey-bundle dependency with a more narrowly defined dependency.
A lint rule consists of a `GradleLintRule` implementation plus a properties file. Once called, the rule _visits_ different components in `build.gradle` and allows the code to report violations relating to `build.gradle` or other files via the `addBuildLintViolation` or `addLintViolation` methods respectively.

Let's build a simple rule that blocks build scripts from applying an imaginary Gradle plugin `nebula.fix-jersey-bundle` that might do something like replace a jersey-bundle dependency with a more narrowly defined dependency.

### The `Rule` implementation

Expand All @@ -121,17 +123,15 @@ Lint rules are AST visiting rules, because the AST gives us the ingredients we n
}

if(!foundJerseyBundle)
addLintViolation('since there is no dependency on jersey-bundle this plugin has no effect', call)
addBuildLintViolation('since there is no dependency on jersey-bundle this plugin has no effect', call)
.delete(call) // Note: we could keep chaining additional fixes here if there was more to do
}
}
}

We use the AST to look for the specific piece of code where `nebula.fix-jersey-bundle` was applied. We could determine through the Gradle model that the plugin had been applied, but not how or where this had been accomplished. Then we transition to using the Gradle model to determine if `jersey-bundle` is in our transitive dependencies. We could not have determined this with the AST alone! Also, since linting runs not only after project configuration but in fact LAST in the task execution order, we can comfortably use Gradle bits like `resolvedConfiguration` without fear of introducing side effects.

Finally, we use `addLintViolation` to indicate to the lint plugin that this block of code applying `nebula.fix-jersey-bundle` violates the rule, and the `delete` fix hint tells `fixGradleLint` that it can safely delete this code snippet.

Currently, `delete`, `insertBefore`, `insertAfter`, and `replaceWith` are provided as fix hints.
As shown above, `addBuildLintViolation` is used to indicate to the lint plugin that this block of code applying `nebula.fix-jersey-bundle` violates the rule, and the `delete` fix hint tells `fixGradleLint` that it can safely delete this code snippet.

Finally, notice how we overrode the `visitApplyPlugin` method. `GradleLintRule` implements the `GradleAstVisitor` interface which adds several convenience hooks for Gradle specific constructs to the rich set of hooks already provided by CodeNarc's `AbstractAstVisitor`, including:

Expand All @@ -141,6 +141,10 @@ Finally, notice how we overrode the `visitApplyPlugin` method. `GradleLintRule`
* `visitGradleDependency(MethodCallExpression call, String conf, GradleDependency dep)`
* `visitConfigurationExclude(MethodCallExpression call, String conf, GradleDependency exclude)`

### Lint violation fixes

There are several lint violation fixes currently available. For AST-based fixes, you would typically want to use `insertAfter`, `insertBefore`, `replaceWith`, or `delete`. For fixes that cannot specify AST nodes, such as text files, `replaceAll`, `deleteLines`, `deleteFile` and `createFile` are also available.

### The properties file

Lastly, provide a properties file that ties the `Rule` implementation to a short name that is used to apply the rule through the `gradleLint.rules` extension property. For our example, add a properties file: `META-INF/lint-rules/fix-jersey-bundle.properties'. Any jar that provides properties files in `META-INF/lint-rules` and is on the buildscript classpath effectively contributes usable rules to the plugin.
Expand Down
41 changes: 41 additions & 0 deletions src/main/groovy/com/netflix/nebula/lint/FileType.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright 2015-2016 Netflix, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.netflix.nebula.lint

import static java.nio.file.Files.isSymbolicLink

enum FileType {
Regular(100644),
Symlink(120000),
Executable(100755)

int mode

FileType(int mode) {
this.mode = mode
}

static fromFile(File file) {
if (isSymbolicLink(file.toPath()))
return Symlink
else if (file.canExecute()) {
return Executable
} else {
return Regular
}
}
}
52 changes: 50 additions & 2 deletions src/main/groovy/com/netflix/nebula/lint/GradleLintFix.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ package com.netflix.nebula.lint

import groovy.transform.Canonical

import java.nio.file.Files

/**
* Fixes that implement this marker interface will generate a single patchset per fix
*/
interface RequiresOwnPatchset {}
interface DeletesFile extends RequiresOwnPatchset {}
interface CreatesFile extends RequiresOwnPatchset {}

/**
* Used to generate a unified diff format of auto-corrections for violations
*/
Expand Down Expand Up @@ -57,9 +66,10 @@ class GradleLintReplaceWith extends GradleLintMultilineFix {
String changes() { changes }
}

class GradleLintDelete extends GradleLintMultilineFix {
@Canonical
class GradleLintDeleteLines extends GradleLintMultilineFix {

GradleLintDelete(File affectedFile, Range<Integer> affectedLines) {
GradleLintDeleteLines(File affectedFile, Range<Integer> affectedLines) {
this.affectedFile = affectedFile
this.affectedLines = affectedLines
}
Expand All @@ -68,6 +78,22 @@ class GradleLintDelete extends GradleLintMultilineFix {
String changes() { null }
}

@Canonical
class GradleLintReplaceAll extends GradleLintMultilineFix {

String changes

GradleLintReplaceAll(File affectedFile, String changes) {
this.affectedFile = affectedFile
this.affectedLines = 1..affectedFile.readLines().size()
this.changes = changes
}

@Override
String changes() { changes }
}

@Canonical
class GradleLintInsertAfter extends GradleLintFix {
Integer afterLine // 1-based
String changes
Expand Down Expand Up @@ -107,4 +133,26 @@ class GradleLintInsertBefore extends GradleLintFix {

@Override
String changes() { changes }
}

@Canonical
class GradleLintDeleteFile extends GradleLintMultilineFix implements DeletesFile {
GradleLintDeleteFile(File affectedFile) {
this.affectedFile = affectedFile
def numberOfLines = Files.isSymbolicLink(affectedFile.toPath()) ? 1 : affectedFile.readLines().size()
this.affectedLines = 1..numberOfLines
}

@Override
String changes() { null }
}

@Canonical
class GradleLintCreateFile extends GradleLintInsertBefore implements CreatesFile {
FileType fileType

GradleLintCreateFile(File newFile, String changes, FileType fileType = FileType.Regular) {
super(newFile, 1, changes)
this.fileType = fileType
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.netflix.nebula.lint

import groovy.transform.Canonical
import nebula.plugin.info.InfoBrokerPlugin
import org.gradle.api.Project

@Canonical
Expand All @@ -9,11 +10,13 @@ class GradleLintInfoBrokerAction extends GradleLintViolationAction {

@Override
void lintFinished(Collection<GradleViolation> violations) {
violations.collect { v ->
def reportItems = violations.collect { v ->
def buildFilePath = project.rootDir.toURI().relativize(v.buildFile.toURI()).toString()
new LintViolationReportItem(buildFilePath, v.rule.ruleId as String, v.level.toString(),
v.lineNumber ?: -1, v.sourceLine ?: 'unspecified')
}

project.getPlugins().withType(InfoBrokerPlugin) { it.addReport('gradleLintViolations', reportItems) }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import groovy.transform.Canonical
import org.apache.commons.lang.StringUtils
import org.gradle.api.Project

import static com.netflix.nebula.lint.FileType.Symlink
import static com.netflix.nebula.lint.PatchType.*
import static java.nio.file.Files.readSymbolicLink

@Canonical
class GradleLintPatchAction extends GradleLintViolationAction {
Project project
Expand All @@ -35,20 +39,53 @@ class GradleLintPatchAction extends GradleLintViolationAction {
}
}

static determinePatchType(List<GradleLintFix> patchFixes) {
if (patchFixes.size() == 1 && patchFixes.get(0) instanceof DeletesFile)
return Delete
else if (patchFixes.size() == 1 && patchFixes.get(0) instanceof CreatesFile) {
return Create
} else {
return Update
}
}

static readFileOrSymlink(File file, FileType type) {
return type == Symlink ? [readSymbolicLink(file.toPath()).toString()] : file.readLines()
}

static diffHints(String path, PatchType patchType, FileType fileType) {
def headers = ["diff --git a/$path b/$path"]
switch (patchType) {
case Create:
headers += "new file mode ${fileType.mode}"
break
case Delete:
headers += "deleted file mode ${fileType.mode}"
break
case Update:
// no hint necessary
break
}
return headers.collect { "|$it" }.join('\n')
}

String patch(List<GradleLintFix> fixes) {
List<List<GradleLintFix>> patchSets = []

fixes.groupBy { it.affectedFile }.each { file, fileFixes ->
fixes.groupBy { it.affectedFile }.each { file, fileFixes -> // internal ordering of fixes per file is maintained (file order does not)
List<GradleLintFix> curPatch = []
GradleLintFix last = null

for (f in fileFixes.sort { f1, f2 -> f1.from() <=> f2.from() ?: f1.to() <=> f2.to() ?: f1.changes() <=> f2.changes() }) {
def (individualFixes, maybeCombinedFixes) = fileFixes.split { it instanceof RequiresOwnPatchset }
individualFixes.each { patchSets.add([it] as List<GradleLintFix>) }

GradleLintFix last = null
for (f in maybeCombinedFixes.sort { f1, f2 -> f1.from() <=> f2.from() ?: f1.to() <=> f2.to() ?: f1.changes() <=> f2.changes() }) {
if (!last || f.from() - last.to() <= MIN_LINES_CONTEXT * 2) {
// if context lines would overlap or abut, put these fixes in the same patch
curPatch += f
} else {
patchSets.add(curPatch)
curPatch = [f]
curPatch = [f] as List<GradleLintFix>
}
last = f
}
Expand All @@ -59,17 +96,25 @@ class GradleLintPatchAction extends GradleLintViolationAction {

String combinedPatch = ''

def lastPathDeleted = null
patchSets.eachWithIndex { patchFixes, i ->
def patchType = determinePatchType(patchFixes)

def file = patchFixes[0].affectedFile
def newlineAtEndOfOriginal = file.text.empty ? false : file.text[-1] == '\n'
def fileType = patchType == Create ? (patchFixes[0] as GradleLintCreateFile).fileType : FileType.fromFile(file)
def emptyFile = file.exists() ? (lastPathDeleted == file.absolutePath || patchType == Create ||
readFileOrSymlink(file, fileType).size() == 0) : true
def newlineAtEndOfOriginal = emptyFile ? false : fileType != Symlink && file.text[-1] == '\n'

def firstLineOfContext = 1

def beforeLineCount = 0
def afterLineCount = 0

// generate just this patch
def lines = [''] + file.readLines() // the extra empty line is so we don't have to do a bunch of zero-based conversions for line arithmetic
def lines = [''] // the extra empty line is so we don't have to do a bunch of zero-based conversions for line arithmetic
if (!emptyFile) lines += readFileOrSymlink(file, fileType)

def patch = []
patchFixes.eachWithIndex { fix, j ->
def lastFix = j == patchFixes.size() - 1
Expand All @@ -83,7 +128,7 @@ class GradleLintPatchAction extends GradleLintViolationAction {
.collect { line -> ' ' + line }
.dropWhile { String line -> j == 0 && StringUtils.isBlank(line) }

if(j == 0) {
if (j == 0) {
firstLineOfContext = fix.from() - beforeContext.size()
}

Expand All @@ -103,7 +148,7 @@ class GradleLintPatchAction extends GradleLintViolationAction {
if (j == 0 && fix.to() + 1 == lines.size() && !newlineAtEndOfOriginal && changed[-1] != '\n') {
patch += /\ No newline at end of file/
}
} else if (fix instanceof GradleLintInsertAfter && fix.afterLine == lines.size() - 1 && !newlineAtEndOfOriginal && !file.text.empty) {
} else if (fix instanceof GradleLintInsertAfter && fix.afterLine == lines.size() - 1 && !newlineAtEndOfOriginal && !emptyFile) {
patch = patch.dropRight(1)
patch.addAll(['-' + lines[-1], /\ No newline at end of file/, '+' + lines[-1]])
}
Expand Down Expand Up @@ -149,7 +194,7 @@ class GradleLintPatchAction extends GradleLintViolationAction {

patch += afterContext

if (lastFix && lastLineOfContext == lines.size() && file.text[-1] != '\n') {
if (lastFix && lastLineOfContext == lines.size() && !newlineAtEndOfOriginal) {
patch += /\ No newline at end of file/
}
} else if (lastFix && fix.changes() && fix.changes()[-1] != '\n' && !newlineAtEndOfOriginal) {
Expand All @@ -162,13 +207,23 @@ class GradleLintPatchAction extends GradleLintViolationAction {

if (i > 0)
combinedPatch += '\n'
combinedPatch += """\
--- a/$path
+++ b/$path
@@ -${file.text.empty ? 0 : firstLineOfContext},$beforeLineCount +${afterLineCount == 0 ? 0 : firstLineOfContext},$afterLineCount @@
""".stripIndent() + patch.join('\n')

def diffHeader = """\
${diffHints(path, patchType, fileType)}
|--- ${patchType == Create ? '/dev/null' : 'a/' + path}
|+++ ${patchType == Delete ? '/dev/null' : 'b/' + path}
|@@ -${emptyFile ? 0 : firstLineOfContext},$beforeLineCount +${afterLineCount == 0 ? 0 : firstLineOfContext},$afterLineCount @@
|""".stripMargin()

combinedPatch += diffHeader + patch.join('\n')

lastPathDeleted = patchType == Delete ? file.absolutePath : null
}

combinedPatch + '\n'
}
}

enum PatchType {
Update, Create, Delete
}
21 changes: 21 additions & 0 deletions src/main/groovy/com/netflix/nebula/lint/GradleViolation.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,25 @@ class GradleViolation extends Violation {
fixes += new GradleLintInsertBefore(file, beforeLine, changes)
this
}

GradleViolation replaceAll(File file, String changes) {
fixes += new GradleLintReplaceAll(file, changes)
this
}

GradleViolation deleteLines(File file, Range<Integer> linesToDelete) {
fixes += new GradleLintDeleteLines(file, linesToDelete)
this
}

GradleViolation deleteFile(File file) {
fixes += new GradleLintDeleteFile(file)
this
}

GradleViolation createFile(File file, String changes) {
fixes += new GradleLintCreateFile(file, changes)
this
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class EmptyClosureRule extends GradleLintRule {
@Override
protected void visitClassComplete(ClassNode node) {
potentialDeletes.unique().each {
addLintViolation('this is an empty configuration closure that can be removed', it)
addBuildLintViolation('this is an empty configuration closure that can be removed', it)
.delete(it)
}
}
Expand Down
Loading

0 comments on commit 34ed4f4

Please sign in to comment.