From 02ecc50d37695ca9d5865535dc3cc26893cdf64e Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 12:22:42 +0100 Subject: [PATCH 01/13] Skip unknown checks instead of stopping I'm not sure I like this behavior, this makes it easier to miss misspelled checks. However, I think it makes sense at least until the token-tree branch is merged, since the current behavior makes switching between the branches somewhat annoying. --- checkstyle/ChecksInfo.hx | 5 ++++- checkstyle/Main.hx | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/checkstyle/ChecksInfo.hx b/checkstyle/ChecksInfo.hx index fc8c50f9..5d12af77 100644 --- a/checkstyle/ChecksInfo.hx +++ b/checkstyle/ChecksInfo.hx @@ -51,7 +51,10 @@ class ChecksInfo { @SuppressWarnings('checkstyle:Dynamic') public function build(name:String):Dynamic { - if (!name2info.exists(name)) throw 'Unknown check: $name'; + if (!name2info.exists(name)) { + trace('Skipping unknown check: $name'); + return null; + } var cl = name2info[name].clazz; return Type.createInstance(cl, []); } diff --git a/checkstyle/Main.hx b/checkstyle/Main.hx index c40cea8d..dc9251fc 100644 --- a/checkstyle/Main.hx +++ b/checkstyle/Main.hx @@ -105,6 +105,7 @@ class Main { var checks:Array = config.checks; for (checkConf in checks) { var check:Check = cast info.build(checkConf.type); + if (check == null) continue; if (checkConf.props != null) { var props = Reflect.fields(checkConf.props); for (prop in props) { From ea3738c2181c768d215186eba25273c42fd45027 Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 12:26:39 +0100 Subject: [PATCH 02/13] Output errors to stderr --- checkstyle/ChecksInfo.hx | 2 +- checkstyle/Main.hx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/checkstyle/ChecksInfo.hx b/checkstyle/ChecksInfo.hx index 5d12af77..f55641f7 100644 --- a/checkstyle/ChecksInfo.hx +++ b/checkstyle/ChecksInfo.hx @@ -52,7 +52,7 @@ class ChecksInfo { @SuppressWarnings('checkstyle:Dynamic') public function build(name:String):Dynamic { if (!name2info.exists(name)) { - trace('Skipping unknown check: $name'); + Sys.stderr().writeString('Skipping unknown check: ${name}\n'); return null; } var cl = name2info[name].clazz; diff --git a/checkstyle/Main.hx b/checkstyle/Main.hx index dc9251fc..691b9fd9 100644 --- a/checkstyle/Main.hx +++ b/checkstyle/Main.hx @@ -38,8 +38,8 @@ class Main { } } catch (e:Dynamic) { - trace(e); - trace(CallStack.toString(CallStack.exceptionStack())); + Sys.stderr().writeString(e + "\n"); + Sys.stderr().writeString(CallStack.toString(CallStack.exceptionStack()) + "\n"); } if (oldCwd != null) Sys.setCwd(oldCwd); Sys.exit(exitCode); From 463eae73fa051d0dbc23a9e061274b8691972aaf Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 12:29:52 +0100 Subject: [PATCH 03/13] Exit with 1 in case of errors --- checkstyle/Main.hx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/checkstyle/Main.hx b/checkstyle/Main.hx index 691b9fd9..d2247cce 100644 --- a/checkstyle/Main.hx +++ b/checkstyle/Main.hx @@ -40,6 +40,7 @@ class Main { catch (e:Dynamic) { Sys.stderr().writeString(e + "\n"); Sys.stderr().writeString(CallStack.toString(CallStack.exceptionStack()) + "\n"); + exitCode = 1; } if (oldCwd != null) Sys.setCwd(oldCwd); Sys.exit(exitCode); @@ -87,7 +88,7 @@ class Main { if (args.length == 0) { Sys.println(argHandler.getDoc()); - Sys.exit(0); + Sys.exit(1); } argHandler.parse(args); From ce2d91c7df1ec8224657fa63813255d1235c3dd2 Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 12:30:19 +0100 Subject: [PATCH 04/13] Output the reporter's name in the error case --- checkstyle/Main.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkstyle/Main.hx b/checkstyle/Main.hx index d2247cce..11b4e985 100644 --- a/checkstyle/Main.hx +++ b/checkstyle/Main.hx @@ -138,7 +138,7 @@ class Main { case "xml": new XMLReporter(XML_PATH, STYLE); case "json": new JSONReporter(JSON_PATH); case "text": new Reporter(); - default: throw "Unknown reporter"; + default: throw 'Unknown reporter: $REPORT_TYPE'; } } From 0fb55b25528e2f056164f04e1db4dec624aef840 Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 12:37:28 +0100 Subject: [PATCH 05/13] Detect unknown properties --- checkstyle/Main.hx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/checkstyle/Main.hx b/checkstyle/Main.hx index 11b4e985..66a87059 100644 --- a/checkstyle/Main.hx +++ b/checkstyle/Main.hx @@ -111,6 +111,9 @@ class Main { var props = Reflect.fields(checkConf.props); for (prop in props) { var val = Reflect.field(checkConf.props, prop); + if (!Reflect.hasField(check, prop)) { + throw 'Check ${check.getModuleName()} has no property named \'$prop\''; + } Reflect.setField(check, prop, val); } if (defaultSeverity != null && props.indexOf("severity") < 0) { From 41388d1de1393e457d7fa1e35cd867aa21cffa6c Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 13:22:51 +0100 Subject: [PATCH 06/13] Only exit with 1 for invalid input arguments --- checkstyle/Main.hx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/checkstyle/Main.hx b/checkstyle/Main.hx index 66a87059..dca77ec8 100644 --- a/checkstyle/Main.hx +++ b/checkstyle/Main.hx @@ -40,7 +40,6 @@ class Main { catch (e:Dynamic) { Sys.stderr().writeString(e + "\n"); Sys.stderr().writeString(CallStack.toString(CallStack.exceptionStack()) + "\n"); - exitCode = 1; } if (oldCwd != null) Sys.setCwd(oldCwd); Sys.exit(exitCode); @@ -83,12 +82,12 @@ class Main { @doc("Show report") ["-report"] => function() REPORT = true, @doc("Return number of failed checks in exitcode") ["-exitcode"] => function() EXIT_CODE = true, @doc("Set source folder to process") ["-s", "--source"] => function(sourcePath:String) traverse(sourcePath, files), - _ => function(arg:String) throw "Unknown command: " + arg + _ => function(arg:String) failWith("Unknown command: " + arg) ]); if (args.length == 0) { Sys.println(argHandler.getDoc()); - Sys.exit(1); + Sys.exit(0); } argHandler.parse(args); @@ -112,7 +111,7 @@ class Main { for (prop in props) { var val = Reflect.field(checkConf.props, prop); if (!Reflect.hasField(check, prop)) { - throw 'Check ${check.getModuleName()} has no property named \'$prop\''; + failWith('Check ${check.getModuleName()} has no property named \'$prop\''); } Reflect.setField(check, prop, val); } @@ -141,7 +140,7 @@ class Main { case "xml": new XMLReporter(XML_PATH, STYLE); case "json": new JSONReporter(JSON_PATH); case "text": new Reporter(); - default: throw 'Unknown reporter: $REPORT_TYPE'; + default: failWith('Unknown reporter: $REPORT_TYPE'); null; } } @@ -164,6 +163,11 @@ class Main { else if (~/(.hx)$/i.match(node)) files.push(node); } + static function failWith(message:String) { + Sys.stderr().writeString(message + "\n"); + Sys.exit(1); + } + public static function setExitCode(newExitCode:Int) { exitCode = newExitCode; } From 6ca8f70dfaff7e9dd32fac3fcde5012b25d2f86d Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 15:38:54 +0100 Subject: [PATCH 07/13] Add some defines for the standard library, closes #68 --- checkstyle/Checker.hx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/checkstyle/Checker.hx b/checkstyle/Checker.hx index 8a6a343d..9ac953df 100644 --- a/checkstyle/Checker.hx +++ b/checkstyle/Checker.hx @@ -112,6 +112,9 @@ class Checker { function makeAST() { var code = file.content; var parser = new HaxeParser(byte.ByteData.ofString(code), file.name); + parser.define("cross"); + parser.define("scriptable"); + parser.define("unsafe"); ast = parser.parse(); } From 04a0cb71560237bad1599aec5683deb75f6a5aba Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 18:28:41 +0100 Subject: [PATCH 08/13] Main: improve readability by moving check creation into a separate function --- checkstyle/Main.hx | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/checkstyle/Main.hx b/checkstyle/Main.hx index dca77ec8..3bc56a0d 100644 --- a/checkstyle/Main.hx +++ b/checkstyle/Main.hx @@ -103,30 +103,31 @@ class Main { var config = Json.parse(configText); var defaultSeverity = config.defaultSeverity; var checks:Array = config.checks; - for (checkConf in checks) { - var check:Check = cast info.build(checkConf.type); - if (check == null) continue; - if (checkConf.props != null) { - var props = Reflect.fields(checkConf.props); - for (prop in props) { - var val = Reflect.field(checkConf.props, prop); - if (!Reflect.hasField(check, prop)) { - failWith('Check ${check.getModuleName()} has no property named \'$prop\''); - } - Reflect.setField(check, prop, val); - } - if (defaultSeverity != null && props.indexOf("severity") < 0) { - check.severity = defaultSeverity; - } - } - checker.addCheck(check); - } + for (checkConf in checks) createCheck(checkConf, defaultSeverity); } checker.addReporter(createReporter()); if (EXIT_CODE) checker.addReporter(new ExitCodeReporter()); checker.process(toProcess); } + function createCheck(checkConf:Dynamic, defaultSeverity:String) { + var check:Check = cast info.build(checkConf.type); + if (check == null || checkConf.props == null) return; + var props = Reflect.fields(checkConf.props); + + for (prop in props) { + var val = Reflect.field(checkConf.props, prop); + if (!Reflect.hasField(check, prop)) { + failWith('Check ${check.getModuleName()} has no property named \'$prop\''); + } + Reflect.setField(check, prop, val); + } + if (defaultSeverity != null && props.indexOf("severity") < 0) { + check.severity = defaultSeverity; + } + checker.addCheck(check); + } + function addAllChecks() { for (check in info.checks()) checker.addCheck(info.build(check.name)); } From 10f70b367b6fcd2d5e2018839ec95d3be625314a Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 18:56:58 +0100 Subject: [PATCH 09/13] Improved invalid config field detection --- checkstyle/Main.hx | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/checkstyle/Main.hx b/checkstyle/Main.hx index 3bc56a0d..3eb91d0e 100644 --- a/checkstyle/Main.hx +++ b/checkstyle/Main.hx @@ -101,6 +101,7 @@ class Main { else { var configText = File.getContent(configPath); var config = Json.parse(configText); + verifyAllowedFields(config, ["checks", "defaultSeverity"], "Config"); var defaultSeverity = config.defaultSeverity; var checks:Array = config.checks; for (checkConf in checks) createCheck(checkConf, defaultSeverity); @@ -112,9 +113,11 @@ class Main { function createCheck(checkConf:Dynamic, defaultSeverity:String) { var check:Check = cast info.build(checkConf.type); - if (check == null || checkConf.props == null) return; - var props = Reflect.fields(checkConf.props); + if (check == null) return; + verifyAllowedFields(checkConf, ["type", "props"], check.getModuleName()); + if (checkConf.props == null) return; + var props = Reflect.fields(checkConf.props); for (prop in props) { var val = Reflect.field(checkConf.props, prop); if (!Reflect.hasField(check, prop)) { @@ -128,6 +131,14 @@ class Main { checker.addCheck(check); } + function verifyAllowedFields(object:Dynamic, allowedFields:Array, messagePrefix:String) { + for (field in Reflect.fields(object)) { + if (allowedFields.indexOf(field) < 0) { + failWith(messagePrefix + " has unknown field '" + field + "'"); + } + } + } + function addAllChecks() { for (check in info.checks()) checker.addCheck(info.build(check.name)); } From 4f00526233f617b75da05bb2bc7aad39d35d0356 Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 20:01:42 +0100 Subject: [PATCH 10/13] IndentationCharacterCheck: less misleading error message It sounded like the current indent char was tab / space, when it's really the other way around (_should_ be tab / space). --- checkstyle/checks/IndentationCharacterCheck.hx | 2 +- test/IndentationCharacterCheckTest.hx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/checkstyle/checks/IndentationCharacterCheck.hx b/checkstyle/checks/IndentationCharacterCheck.hx index 8c7aca5c..17dd15ca 100644 --- a/checkstyle/checks/IndentationCharacterCheck.hx +++ b/checkstyle/checks/IndentationCharacterCheck.hx @@ -25,7 +25,7 @@ class IndentationCharacterCheck extends Check { } for (i in 0 ... checker.lines.length) { var line = checker.lines[i]; - if (line.length > 0 && !re.match(line)) log('Wrong indentation character (${character})', i + 1, 0, null, Reflect.field(SeverityLevel, severity)); + if (line.length > 0 && !re.match(line)) log('Wrong indentation character (should be ${character})', i + 1, 0, null, Reflect.field(SeverityLevel, severity)); } } } \ No newline at end of file diff --git a/test/IndentationCharacterCheckTest.hx b/test/IndentationCharacterCheckTest.hx index 8486c983..60b54246 100644 --- a/test/IndentationCharacterCheckTest.hx +++ b/test/IndentationCharacterCheckTest.hx @@ -6,7 +6,7 @@ class IndentationCharacterCheckTest extends CheckTestCase { public function testWrongIndentation() { var msg = checkMessage(IndentationTests.TEST1, new IndentationCharacterCheck()); - assertEquals(msg, 'Wrong indentation character (tab)'); + assertEquals(msg, 'Wrong indentation character (should be tab)'); } public function testCorrectIndentation() { @@ -19,7 +19,7 @@ class IndentationCharacterCheckTest extends CheckTestCase { check.character = "space"; var msg = checkMessage(IndentationTests.TEST3, check); - assertEquals(msg, 'Wrong indentation character (space)'); + assertEquals(msg, 'Wrong indentation character (should be space)'); } public function testMultilineIfIndentation() { From 6e53d6dcc3711648dcbec3cc9b9d06054d914790 Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 22:36:39 +0100 Subject: [PATCH 11/13] EmptyLinesCheck: separate options for single and multi line comments --- checkstyle/checks/EmptyLinesCheck.hx | 17 ++++++++++++----- test/EmptyLinesCheckTest.hx | 19 +++++++++++-------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/checkstyle/checks/EmptyLinesCheck.hx b/checkstyle/checks/EmptyLinesCheck.hx index c5340ac5..a4c0e776 100644 --- a/checkstyle/checks/EmptyLinesCheck.hx +++ b/checkstyle/checks/EmptyLinesCheck.hx @@ -8,12 +8,14 @@ import checkstyle.LintMessage.SeverityLevel; class EmptyLinesCheck extends Check { public var max:Int; - public var allowEmptyLineAfterComment:Bool; + public var allowEmptyLineAfterSingleLineComment:Bool; + public var allowEmptyLineAfterMultiLineComment:Bool; public function new() { super(); max = 1; - allowEmptyLineAfterComment = true; + allowEmptyLineAfterSingleLineComment = true; + allowEmptyLineAfterMultiLineComment = true; } override function actualRun() { @@ -29,9 +31,8 @@ class EmptyLinesCheck extends Check { } end = i; - if (i > 0 && !allowEmptyLineAfterComment && ~/^(\/\/).*|^(\/\*).*|(\*\/)$/.match(StringTools.trim(checker.lines[i - 1]))) { - log('Empty line not allowed after comment(s)', start, 0, null, Reflect.field(SeverityLevel, severity)); - } + if (!allowEmptyLineAfterSingleLineComment) checkComment(i, start, ~/^(\/\/).*$/); + if (!allowEmptyLineAfterMultiLineComment) checkComment(i, start, ~/^^(\/\*).*|(\*\/)$/); } else { if (inGroup) { @@ -47,6 +48,12 @@ class EmptyLinesCheck extends Check { } } + function checkComment(i, start, regex) { + if (i > 0 && regex.match(StringTools.trim(checker.lines[i - 1]))) { + log('Empty line not allowed after comment(s)', start, 0, null, Reflect.field(SeverityLevel, severity)); + } + } + function logInfo(pos) { log('Too many consecutive empty lines (> ${max})', pos, 0, null, Reflect.field(SeverityLevel, severity)); } diff --git a/test/EmptyLinesCheckTest.hx b/test/EmptyLinesCheckTest.hx index 3bfff1ed..2110a704 100644 --- a/test/EmptyLinesCheckTest.hx +++ b/test/EmptyLinesCheckTest.hx @@ -22,22 +22,19 @@ class EmptyLinesCheckTest extends CheckTestCase { public function testEmptyLineAfterSingleLineComment() { var check = new EmptyLinesCheck(); - check.allowEmptyLineAfterComment = false; + check.allowEmptyLineAfterSingleLineComment = false; var msg = checkMessage(EmptyLinesTests.TEST4, check); assertEquals(msg, 'Empty line not allowed after comment(s)'); msg = checkMessage(EmptyLinesTests.TEST5, check); assertEquals(msg, 'Empty line not allowed after comment(s)'); - - msg = checkMessage(EmptyLinesTests.TEST6, check); - assertEquals(msg, 'Empty line not allowed after comment(s)'); } public function testEmptyLineAfterMultiLineComment() { var check = new EmptyLinesCheck(); - check.allowEmptyLineAfterComment = false; - + check.allowEmptyLineAfterMultiLineComment = false; + var msg = checkMessage(EmptyLinesTests.TEST6, check); assertEquals(msg, 'Empty line not allowed after comment(s)'); @@ -47,10 +44,16 @@ class EmptyLinesCheckTest extends CheckTestCase { public function testAllowEmptyLineAfterComment() { var check = new EmptyLinesCheck(); - + + var msg = checkMessage(EmptyLinesTests.TEST4, check); + assertEquals(msg, ''); + + var msg = checkMessage(EmptyLinesTests.TEST5, check); + assertEquals(msg, ''); + var msg = checkMessage(EmptyLinesTests.TEST6, check); assertEquals(msg, ''); - + msg = checkMessage(EmptyLinesTests.TEST7, check); assertEquals(msg, ''); } From 44eb0db9b63f56ed878c9139c15a764889a1296d Mon Sep 17 00:00:00 2001 From: Gama11 Date: Mon, 29 Feb 2016 22:45:42 +0100 Subject: [PATCH 12/13] Suppress Dynamic warnings for createCheck() / verifyAllowedFields() --- checkstyle/Main.hx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/checkstyle/Main.hx b/checkstyle/Main.hx index 3eb91d0e..26a7c540 100644 --- a/checkstyle/Main.hx +++ b/checkstyle/Main.hx @@ -111,6 +111,7 @@ class Main { checker.process(toProcess); } + @SuppressWarnings('checkstyle:Dynamic') function createCheck(checkConf:Dynamic, defaultSeverity:String) { var check:Check = cast info.build(checkConf.type); if (check == null) return; @@ -131,6 +132,7 @@ class Main { checker.addCheck(check); } + @SuppressWarnings('checkstyle:Dynamic') function verifyAllowedFields(object:Dynamic, allowedFields:Array, messagePrefix:String) { for (field in Reflect.fields(object)) { if (allowedFields.indexOf(field) < 0) { From 2b60eb894c024ab8212f7f13a09853b992bf3468 Mon Sep 17 00:00:00 2001 From: Adi Date: Tue, 1 Mar 2016 09:24:00 +0000 Subject: [PATCH 13/13] new release --- haxelib.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/haxelib.json b/haxelib.json index 3f7a3b57..a3365f07 100644 --- a/haxelib.json +++ b/haxelib.json @@ -15,7 +15,7 @@ "contributors": [ "adireddy" ], - "releasenote": "bug fixes and minor updates", + "releasenote": "more improvements and bug fixes", "version": "1.2.4", "url": "https://github.com/adireddy/haxe-checkstyle", "dependencies": { diff --git a/package.json b/package.json index d2585f07..ea7b5246 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "checkstyle", - "version": "1.2.4", + "version": "1.2.5", "description": "Automated code analysis ideal for projects that want to enforce a coding standard.", "repository": { "type": "git",