-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exclude #142
Exclude #142
Changes from all commits
3ad4350
c83e7a0
8f19501
71ccb94
fbea9f3
c43ae58
75be793
de0afa1
eb0ca52
b9a0043
b306dfb
e31b9e7
0ab7156
0404d95
61291ce
c2cba3e
ec9d698
56d937d
55e044f
bc7a541
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
{ | ||
"all": [], | ||
"Dynamic": [ | ||
"checkstyle.Main", | ||
"checkstyle.Checker", | ||
"checkstyle.ChecksInfo" | ||
], | ||
"EmptyLines": [ | ||
"checks" | ||
], | ||
"MultipleStringLiterals": [ | ||
"checks", | ||
"token" | ||
], | ||
"MultipleVariableDeclarations": [ | ||
"checkstyle.checks.coding.MultipleVariableDeclarationsCheck", | ||
"checks.coding.MultipleVariableDeclarationsCheckTest" | ||
], | ||
"TrailingWhitespace": [ | ||
"checks.whitespace.SpacingCheckTest", | ||
"checks.whitespace.TrailingWhitespaceCheckTest", | ||
"checks.naming.MemberNameCheckTest" | ||
], | ||
"TabForAligning": [ | ||
"checks.whitespace.IndentationCharacterCheckTest", | ||
"checks.whitespace.TabForAligningCheckTest" | ||
], | ||
"LineLength": [ | ||
"checks.size.LineLengthCheckTest" | ||
], | ||
"IndentationCharacter": [ | ||
"checks.whitespace.IndentationCharacterCheckTest" | ||
], | ||
"CyclomaticComplexity": [ | ||
"checkstyle.checks.Check", | ||
"checkstyle.utils.ExprUtils", | ||
"checkstyle.utils.ComplexTypeUtils", | ||
"checkstyle.checks.CyclomaticComplexityCheck", | ||
"checkstyle.checks.block.LeftCurlyCheck", | ||
"checkstyle.checks.block.RightCurlyCheck", | ||
"checkstyle.checks.naming.MethodNameCheck", | ||
"checkstyle.checks.type.ReturnCheck", | ||
"checkstyle.checks.whitespace.OperatorWrapCheck", | ||
"checkstyle.checks.whitespace.WhitespaceAfterCheck", | ||
"checkstyle.checks.whitespace.WhitespaceAroundCheck", | ||
"checkstyle.token.TokenTreeBuilder" | ||
], | ||
"LeftCurly": [ | ||
"checkstyle.checks.CyclomaticComplexityCheck" | ||
], | ||
"RightCurly": [ | ||
"checkstyle.checks.CyclomaticComplexityCheck" | ||
], | ||
"MethodLength": [ | ||
"checkstyle.checks.whitespace.WhitespaceAfterCheck", | ||
"checkstyle.checks.whitespace.WhitespaceAroundCheck", | ||
"checkstyle.token.TokenTreeBuilder" | ||
], | ||
"MagicNumber": [ | ||
"checkstyle.reporter.ProgressReporter" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,9 @@ import sys.io.File; | |
import checkstyle.token.TokenTree; | ||
import checkstyle.token.TokenTreeBuilder; | ||
|
||
using checkstyle.utils.ArrayUtils; | ||
using StringTools; | ||
|
||
class Checker { | ||
|
||
public var file:LintFile; | ||
|
@@ -27,6 +30,7 @@ class Checker { | |
var lineSeparator:String; | ||
var tokenTree:TokenTree; | ||
var asts:Array<Ast>; | ||
var excludes:Map<String, Array<String>>; | ||
|
||
public function new() { | ||
checks = []; | ||
|
@@ -135,7 +139,8 @@ class Checker { | |
return parser.parse(); | ||
} | ||
|
||
public function process(files:Array<LintFile>) { | ||
public function process(files:Array<LintFile>, excludesMap:Map<String, Array<String>>) { | ||
excludes = excludesMap; | ||
var advanceFrame = function() {}; | ||
#if hxtelemetry | ||
var hxt = new hxtelemetry.HxTelemetry(); | ||
|
@@ -166,7 +171,6 @@ class Checker { | |
lintFile.content = null; | ||
} | ||
|
||
@SuppressWarnings("checkstyle:Dynamic") | ||
function createContext(lintFile:LintFile):Bool { | ||
this.file = lintFile; | ||
for (reporter in reporters) reporter.fileStart(file); | ||
|
@@ -235,9 +239,9 @@ class Checker { | |
message1.moduleName == message2.moduleName; | ||
} | ||
|
||
@SuppressWarnings("checkstyle:Dynamic") | ||
function runCheck(check:Check):Array<LintMessage> { | ||
try { | ||
if (checkForExclude(check.getModuleName())) return []; | ||
return check.run(this); | ||
} | ||
catch (e:Dynamic) { | ||
|
@@ -246,7 +250,23 @@ class Checker { | |
} | ||
} | ||
|
||
@SuppressWarnings("checkstyle:Dynamic") | ||
function checkForExclude(moduleName:String):Bool { | ||
if (excludes == null) return false; | ||
var excludesForCheck:Array<String> = excludes.get(moduleName); | ||
if (excludesForCheck == null || excludesForCheck.length == 0) return false; | ||
|
||
var cls = file.name.substring(0, file.name.indexOf(".hx")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We can move the extensions into There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure it should be specific to extensions, it might be more flexible to apply a regex to the entire path. OpenFL generates .hx file in the export directory, which makes it difficult for me to use checkstyle in say, flixel-demos, which has 75 projects. For generating API docs, I have excluded these specific file names / packages for dox with a regex: https://github.com/HaxeFlixel/flixel-docs/blob/master/api/dox-gen/gendocs.bat --exclude "(__ASSET__|ApplicationMain|DocumentClass|DefaultAssetLibrary|Main|NMEPreloader|zpp_nape)" |
||
if (excludesForCheck.contains(cls)) return true; | ||
|
||
cls = cls.replace("/", ":"); | ||
for (exclude in excludesForCheck) { | ||
var regStr:String = exclude + ":.*?" + cls.substring(cls.lastIndexOf(":") + 1, cls.length) + "$"; | ||
var r = new EReg(regStr.replace("/", ":"), "i"); | ||
if (r.match(cls)) return true; | ||
} | ||
return false; | ||
} | ||
|
||
function getErrorMessage(e:Dynamic, fileName:String, step:String):LintMessage { | ||
return { | ||
fileName:fileName, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,38 +17,12 @@ import sys.FileSystem; | |
import sys.io.File; | ||
|
||
using checkstyle.utils.ArrayUtils; | ||
using checkstyle.utils.StringUtils; | ||
|
||
class Main { | ||
|
||
@SuppressWarnings('checkstyle:Dynamic') | ||
public static function main() { | ||
var args; | ||
var cwd; | ||
var oldCwd = null; | ||
|
||
try { | ||
args = Sys.args(); | ||
cwd = Sys.getCwd(); | ||
if (Sys.getEnv("HAXELIB_RUN") != null) { | ||
cwd = args.pop(); | ||
oldCwd = Sys.getCwd(); | ||
} | ||
if (oldCwd != null) Sys.setCwd(cwd); | ||
|
||
new Main().run(args); | ||
} | ||
catch (e:Dynamic) { | ||
Sys.stderr().writeString(e + "\n"); | ||
Sys.stderr().writeString(CallStack.toString(CallStack.exceptionStack()) + "\n"); | ||
} | ||
if (oldCwd != null) Sys.setCwd(oldCwd); | ||
Sys.exit(exitCode); | ||
} | ||
|
||
var info:ChecksInfo; | ||
var checker:Checker; | ||
|
||
static var DEFAULT_CONFIG:String = "checkstyle.json"; | ||
static var DEFAULT_EXCLUDE_CONFIG:String = "checkstyle-exclude.json"; | ||
static var REPORT_TYPE:String = "text"; | ||
static var XML_PATH:String = "check-style-report.xml"; | ||
static var JSON_PATH:String = "check-style-report.json"; | ||
|
@@ -58,19 +32,29 @@ class Main { | |
static var EXIT_CODE:Bool = false; | ||
static var exitCode:Int; | ||
|
||
var info:ChecksInfo; | ||
var checker:Checker; | ||
var paths:Array<String>; | ||
var allExcludes:Array<String>; | ||
var excludesMap:Map<String, Array<String>>; | ||
|
||
function new() { | ||
info = new ChecksInfo(); | ||
checker = new Checker(); | ||
paths = []; | ||
allExcludes = []; | ||
excludesMap = new Map(); | ||
exitCode = 0; | ||
} | ||
|
||
function run(args:Array<String>) { | ||
var files:Array<String> = []; | ||
var configPath:String = null; | ||
var excludePath:String = null; | ||
|
||
var argHandler = Args.generate([ | ||
@doc("Set source folder to process (multiple allowed)") ["-s", "--source"] => function(path:String) traverse(path, files), | ||
@doc("Set source folder to process (multiple allowed)") ["-s", "--source"] => function(path:String) paths.push(path), | ||
@doc("Set config file (default: checkstyle.json)") ["-c", "--config"] => function(path:String) configPath = path, | ||
@doc("Set exclude config file (default: checkstyle-exclude.json)") ["-e", "--exclude"] => function(path:String) excludePath = path, | ||
@doc("Set reporter (xml, json or text, default: text)") ["-r", "--reporter"] => function(name:String) REPORT_TYPE = name, | ||
@doc("Set reporter output path") ["-p", "--path"] => function(path:String) { | ||
XML_PATH = path; | ||
|
@@ -93,19 +77,19 @@ class Main { | |
} | ||
argHandler.parse(args); | ||
|
||
var i:Int = 0; | ||
var toProcess:Array<LintFile> = [for (file in files) {name:file, content:null, index:i++}]; | ||
|
||
if (configPath == null && FileSystem.exists(DEFAULT_CONFIG) && !FileSystem.isDirectory(DEFAULT_CONFIG)) { | ||
configPath = DEFAULT_CONFIG; | ||
} | ||
|
||
if (excludePath == null && FileSystem.exists(DEFAULT_EXCLUDE_CONFIG) && !FileSystem.isDirectory(DEFAULT_EXCLUDE_CONFIG)) { | ||
excludePath = DEFAULT_EXCLUDE_CONFIG; | ||
} | ||
|
||
if (configPath == null) addAllChecks(); | ||
else loadConfig(configPath); | ||
checker.addReporter(createReporter(files.length)); | ||
if (SHOW_PROGRESS) checker.addReporter(new ProgressReporter(files.length)); | ||
if (EXIT_CODE) checker.addReporter(new ExitCodeReporter()); | ||
checker.process(toProcess); | ||
|
||
if (excludePath != null) loadExcludeConfig(excludePath); | ||
else start(); | ||
} | ||
|
||
function loadConfig(configPath:String) { | ||
|
@@ -127,6 +111,32 @@ class Main { | |
} | ||
} | ||
|
||
function loadExcludeConfig(excludeConfigPath:String) { | ||
var config:Config = Json.parse(File.getContent(excludeConfigPath)); | ||
var excludes = Reflect.fields(config); | ||
for (e in excludes) { | ||
createExcludeMapElement(e); | ||
var excludeValues:Array<String> = Reflect.field(config, e); | ||
if (excludeValues != null && excludeValues.length > 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could invert this if and use |
||
for (val in excludeValues) { | ||
for (p in paths) { | ||
var path = p + "/" + val.split(".").join("/"); | ||
if (e == "all") allExcludes.push(path); | ||
else { | ||
if (!p.contains(":")) excludesMap.get(e).push(path); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
start(); | ||
} | ||
|
||
function createExcludeMapElement(name:String) { | ||
if (excludesMap.get(name) == null) excludesMap.set(name, []); | ||
} | ||
|
||
function createCheck(checkConf:CheckConfig):Check { | ||
var check:Check = info.build(checkConf.type); | ||
if (check == null) failWith('Unknown check \'${checkConf.type}\''); | ||
|
@@ -239,12 +249,27 @@ class Main { | |
return s + "/" + t; | ||
} | ||
|
||
function traverse(node:String, files:Array<String>) { | ||
if (FileSystem.isDirectory(node)) { | ||
var nodes = FileSystem.readDirectory(node); | ||
for (child in nodes) traverse(pathJoin(node, child), files); | ||
function start() { | ||
var files:Array<String> = []; | ||
for (path in paths) traverse(path, files); | ||
|
||
var i:Int = 0; | ||
var toProcess:Array<LintFile> = [for (file in files) {name:file, content:null, index:i++}]; | ||
|
||
checker.addReporter(createReporter(files.length)); | ||
if (SHOW_PROGRESS) checker.addReporter(new ProgressReporter(files.length)); | ||
if (EXIT_CODE) checker.addReporter(new ExitCodeReporter()); | ||
checker.process(toProcess, excludesMap); | ||
} | ||
|
||
function traverse(path:String, files:Array<String>) { | ||
if (FileSystem.isDirectory(path) && !allExcludes.contains(path)) { | ||
var nodes = FileSystem.readDirectory(path); | ||
for (child in nodes) traverse(pathJoin(path, child), files); | ||
} | ||
else if (~/(.hx)$/i.match(path) && !allExcludes.contains(path.substring(0, path.indexOf(".hx")))) { | ||
files.push(path); | ||
} | ||
else if (~/(.hx)$/i.match(node)) files.push(node); | ||
} | ||
|
||
function failWith(message:String) { | ||
|
@@ -255,4 +280,28 @@ class Main { | |
public static function setExitCode(newExitCode:Int) { | ||
exitCode = newExitCode; | ||
} | ||
|
||
public static function main() { | ||
var args; | ||
var cwd; | ||
var oldCwd = null; | ||
|
||
try { | ||
args = Sys.args(); | ||
cwd = Sys.getCwd(); | ||
if (Sys.getEnv("HAXELIB_RUN") != null) { | ||
cwd = args.pop(); | ||
oldCwd = Sys.getCwd(); | ||
} | ||
if (oldCwd != null) Sys.setCwd(cwd); | ||
|
||
new Main().run(args); | ||
} | ||
catch (e:Dynamic) { | ||
Sys.stderr().writeString(e + "\n"); | ||
Sys.stderr().writeString(CallStack.toString(CallStack.exceptionStack()) + "\n"); | ||
} | ||
if (oldCwd != null) Sys.setCwd(oldCwd); | ||
Sys.exit(exitCode); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice to have these in a separated file now! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really enjoyed removing all the
@SuppressWarnings
in code 😄There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're all class-wide now though (for now at least?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.. for now.. need to find a way to implement method level exclude.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean field level? (for variables and properties it should be possible as well)