Issue #273: converts diff.groovy to use Checkstyle's CLI #747
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,8 +74,10 @@ def getCliOptions(args) { | |
| + ' as a shorter version to prevent long paths. (optional, default is false)') | ||
| m(longOpt: 'mode', args: 1, required: false, argName: 'mode', 'The mode of the tool:' \ | ||
| + ' \'diff\' or \'single\'. (optional, default is \'diff\')') | ||
| ce(longOpt: 'continueOnError', required: false, 'Whether to fail or continue the Checkstyle' \ | ||
| + ' run when it reports a non-zero return code. (optional, default is false)') | ||
| xo(longOpt: 'extraOptions', args: 1, required: false, 'Extra arguments to pass ' \ | ||
| + 'for Checkstyle execution (optional, ex: -Dprop=true)') | ||
| } | ||
| return cli.parse(args) | ||
| } | ||
|
|
@@ -218,11 +220,11 @@ def launchCheckstyleReport(cfg) { | |
|
|
||
| // If "no exception" testing, these may not be defined in repos other than checkstyle | ||
| if (isRegressionTesting) { | ||
| println "Building Checkstyle artifact ($cfg.branch) ..." | ||
| executeCmd("git checkout $cfg.branch", cfg.localGitRepo) | ||
| executeCmd("git log -1 --pretty=MSG:%s%nSHA-1:%H", cfg.localGitRepo) | ||
| executeCmd("mvn -e --no-transfer-progress --batch-mode clean package -Pno-validations,assembly " + | ||
| "-Dassembly.skipAssembly=true -Dmaven.test.skip=true", cfg.localGitRepo) | ||
| } | ||
|
|
||
| cfg.checkstyleVersion = | ||
|
|
@@ -247,11 +249,10 @@ def launchCheckstyleReport(cfg) { | |
| def generateCheckstyleReport(cfg) { | ||
| println 'Testing Checkstyle started' | ||
|
|
||
| def allJar = getOsSpecificPath(cfg.localGitRepo.path, "target", "checkstyle-" + cfg.checkstyleVersion + "-all.jar") | ||
| def reposDir = 'repositories' | ||
| def reportsDir = 'reports' | ||
| makeWorkDirsIfNotExist(reposDir, reportsDir) | ||
|
|
||
| final REPO_NAME_PARAM_NO = 0 | ||
| final REPO_TYPE_PARAM_NO = 1 | ||
|
|
@@ -261,11 +262,11 @@ def generateCheckstyleReport(cfg) { | |
| final FULL_PARAM_LIST_SIZE = 5 | ||
|
|
||
| def checkstyleConfig = cfg.checkstyleCfg | ||
| def allowExcludes = cfg.allowExcludes | ||
| def listOfProjectsFile = new File(cfg.listOfProjects) | ||
| def projects = listOfProjectsFile.readLines() | ||
| def continueOnError = cfg.continueOnError | ||
| def extraOptions = cfg.extraOptions | ||
|
|
||
| projects.each { | ||
| project -> | ||
|
|
@@ -287,27 +288,27 @@ def generateCheckstyleReport(cfg) { | |
| excludes = params[REPO_EXCLUDES_PARAM_NO] | ||
| } | ||
|
|
||
| def srcDir | ||
| def saveDir = getOsSpecificPath("$reportsDir", "$repoName") | ||
| if (repoType == 'local') { | ||
| srcDir = new File("$repoUrl").getCanonicalPath() | ||
| } else { | ||
| cloneRepository(repoName, repoType, repoUrl, commitId, reposDir) | ||
| srcDir = getOsSpecificPath("$reposDir", "$repoName") | ||
| } | ||
|
|
||
| def mainClassOptions = [ | ||
| srcDir: srcDir, | ||
| excludes: excludes, | ||
| checkstyleConfig: checkstyleConfig, | ||
| saveDir: saveDir, | ||
| continueOnError: continueOnError, | ||
| extraOptions: extraOptions, | ||
| ] | ||
|
|
||
| runCliExecution(allJar, mainClassOptions) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| def getLastCheckstyleCommitSha(gitRepo, branch) { | ||
|
|
@@ -538,11 +539,7 @@ def getFilenameWithoutExtension(filename) { | |
| return filenameWithoutExtension | ||
| } | ||
|
|
||
| def makeWorkDirsIfNotExist(repoDirPath, reportsDirPath) { | ||
| def repoDir = new File(repoDirPath) | ||
| if (!repoDir.exists()) { | ||
| repoDir.mkdir() | ||
|
|
@@ -629,36 +626,39 @@ def getProjectsStatistic(diffDir) { | |
| return projectsStatistic | ||
| } | ||
|
|
||
| def runCliExecution(allJar, mainClassOptions) { | ||
| def saveLoc = new File(mainClassOptions.saveDir) | ||
| if (!saveLoc.exists()) { | ||
| saveLoc.mkdirs() | ||
| } | ||
|
|
||
| println "Running Checkstyle CLI on ${mainClassOptions.srcDir} ... with excludes {${mainClassOptions.excludes}}" | ||
| def cliCommand = "java -jar $allJar -c ${mainClassOptions.checkstyleConfig} " + | ||
|
There was a problem hiding this comment. I can't help but feel that StringBuilder would be faster/easier to read/use less memory, especially with a bunch of excludes. There was a problem hiding this comment. You want me to switch? There was a problem hiding this comment. Keep in mind performance is not an issue here as it is one time execution. Better to keep code easy to read. I don't mind any approach There was a problem hiding this comment. It's not a big deal, but we certainly run this more than once (twice for each project). I was thinking more for readability since There was a problem hiding this comment. we need to disable trailing "+" rule. There was a problem hiding this comment. It is not a rule, this is required by the interpreter There was a problem hiding this comment. @romani Please see the message I posted for you at checkstyle/checkstyle#12725 (comment) . This is groovy itself, and not a style rule. There was a problem hiding this comment. sad, but ok, lets follow interpreter. |
||
| "${mainClassOptions.srcDir} -f xml -o " + | ||
| getOsSpecificPath("${mainClassOptions.saveDir}", "checkstyle-result.xml") + | ||
| " -e " + getOsSpecificPath("${mainClassOptions.srcDir}", ".git") + | ||
| " -e " + getOsSpecificPath("${mainClassOptions.srcDir}", ".hg") | ||
|
|
||
| if (mainClassOptions.excludes.length() > 0) { | ||
nrmancuso marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| for (String singleExclude : mainClassOptions.excludes.split(',')) { | ||
| if (isWindows()) { | ||
| singleExclude = singleExclude.replace("/", "\\") | ||
| } | ||
| cliCommand = cliCommand + " -x " + singleExclude | ||
| } | ||
| } | ||
|
|
||
| if (mainClassOptions.extraOptions) { | ||
| cliCommand = cliCommand + " " | ||
|
|
||
| if (!mainClassOptions.extraOptions.startsWith("-")) { | ||
| cliCommand = cliCommand + "-" | ||
| } | ||
| cliCommand = cliCommand + mainClassOptions.extraOptions | ||
| } | ||
|
|
||
| executeCliCmd(cliCommand, mainClassOptions.continueOnError) | ||
| println "Running Checkstyle CLI on ${mainClassOptions.srcDir} - finished" | ||
| } | ||
|
|
||
| def copyDir(source, destination) { | ||
|
|
@@ -688,6 +688,17 @@ def executeCmd(cmd, dir = new File("").absoluteFile) { | |
| } | ||
| } | ||
|
|
||
| def executeCliCmd(cmd, continueOnError) { | ||
| println "Running command: ${cmd}" | ||
| def osSpecificCmd = getOsSpecificCmd(cmd) | ||
| def proc = osSpecificCmd.execute(null, new File("").absoluteFile) | ||
| proc.consumeProcessOutput(System.out, System.err) | ||
| proc.waitFor() | ||
| if (!continueOnError && proc.exitValue() != 0) { | ||
| throw new GroovyRuntimeException("Error: ${proc.err.text}!") | ||
| } | ||
| } | ||
|
|
||
| def getOsSpecificCmd(cmd) { | ||
| def osSpecificCmd | ||
| if (System.properties['os.name'].toLowerCase().contains('windows')) { | ||
|
|
@@ -746,6 +757,7 @@ class Config { | |
| def shortFilePaths | ||
| def listOfProjects | ||
| def mode | ||
| def continueOnError | ||
|
|
||
| def baseBranch | ||
| def patchBranch | ||
|
|
@@ -761,7 +773,7 @@ class Config { | |
| def tmpMasterReportsDir | ||
| def tmpPatchReportsDir | ||
| def diffDir | ||
| def extraOptions | ||
|
|
||
| def checkstyleVersion | ||
| def sevntuVersion | ||
|
|
@@ -774,7 +786,8 @@ class Config { | |
|
|
||
| shortFilePaths = cliOptions.shortFilePaths | ||
| listOfProjects = cliOptions.listOfProjects | ||
| extraOptions = cliOptions.extraOptions | ||
| continueOnError = cliOptions.continueOnError | ||
|
|
||
| checkstyleVersion = cliOptions.checkstyleVersion | ||
| allowExcludes = cliOptions.allowExcludes | ||
|
|
@@ -824,8 +837,9 @@ class Config { | |
| checkstyleCfg: baseConfig, | ||
rnveach marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| listOfProjects: listOfProjects, | ||
| destDir: tmpMasterReportsDir, | ||
| extraOptions: extraOptions, | ||
| allowExcludes:allowExcludes, | ||
| continueOnError:continueOnError, | ||
|
There was a problem hiding this comment. Fixed a bug in Without listed them here, doing |
||
| ] | ||
| } | ||
|
|
||
|
|
@@ -836,8 +850,9 @@ class Config { | |
| checkstyleCfg: patchConfig, | ||
| listOfProjects: listOfProjects, | ||
| destDir: tmpPatchReportsDir, | ||
| extraOptions: extraOptions, | ||
| allowExcludes: allowExcludes, | ||
| continueOnError:continueOnError, | ||
| ] | ||
| } | ||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.