diff --git a/src/analysis/allman.d b/src/analysis/allman.d new file mode 100644 index 00000000..a00d1fe8 --- /dev/null +++ b/src/analysis/allman.d @@ -0,0 +1,221 @@ +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) + +module analysis.allman; + +import dparse.lexer; +import dparse.ast; +import analysis.base : BaseAnalyzer; +import dsymbol.scope_ : Scope; + +/** +Checks for the allman style (braces should be on their own line) + +------------ +if (param < 0) { + +} +------------ + +should be + +------------ +if (param < 0) +{ + +} +------------ +*/ +class AllManCheck : BaseAnalyzer +{ + /// + this(string fileName, const(ubyte)[] code, bool skipTests = false) + { + super(fileName, null, skipTests); + this.code = code; + } + + override void visit(const WhileStatement st) + { + if (st.declarationOrStatement !is null) + checkForBrace(st.declarationOrStatement, st.expression.line, st.expression.column); + } + + override void visit(const ForeachStatement st) + { + checkForBrace(st.declarationOrStatement, st.low.line, st.low.column); + } + + override void visit(const ForStatement st) + { + checkForBrace(st.declarationOrStatement, st.test.line, st.test.column); + } + + override void visit(const DoStatement st) + { + // the DoStatement only knows about the line and column of the expression + checkForBrace(st.statementNoCaseNoDefault, 0, 0); + st.statementNoCaseNoDefault.accept(this); + } + + override void visit(const IfStatement st) + { + checkForBrace(st.thenStatement, st.expression.line, st.expression.column); + if (st.elseStatement !is null) + checkForBrace(st.elseStatement, st.expression.line, st.expression.column); + } + + alias visit = ASTVisitor.visit; + +private: + + const(ubyte)[] code; + + enum string KEY = "dscanner.style.allman"; + enum string MESSAGE = "Braces should be on their own line"; + + void checkForBrace(const DeclarationOrStatement declOrSt, size_t line, size_t column) + { + if(auto stst = declOrSt.statement) + { + checkForBrace(stst.statementNoCaseNoDefault, line, column); + } + declOrSt.accept(this); + } + + void checkForBrace(const StatementNoCaseNoDefault st, size_t line, size_t column) + { + if(st !is null) + { + findBraceOrNewLine(st.startLocation, st.endLocation, line, column); + } + } + + /** + Checks whether a brace or newline comes first + */ + void findBraceOrNewLine(size_t start, size_t end, size_t line, size_t column) + { + import std.algorithm : canFind; + import std.utf : byCodeUnit; + + auto codeRange = (cast(char[]) code[start..end]).byCodeUnit; + + // inline statements are allowed -> search for newline + if (codeRange.canFind('\n')) + { + foreach (s; codeRange) + { + // first brace + if (s == '{') + { + // DoStatement hasn't a proper line and column attached + // -> calculate ourselves + if (line == 0 && column == 0) + { + // find line & column of brace + auto t = findLineAndColumnForPos(start); + line = t.line + 1; // Dscanner starts lines at 1 + column = t.column; + } + addErrorMessage(line, column, KEY, MESSAGE); + break; + } + // newline - test passed + else if (s == '\n') + { + break; + } + } + } + } + + /** + Counts all matches of an symbol and the number of iterated characters + */ + auto findLineAndColumnForPos(size_t pos) + { + import std.utf : byCodeUnit; + import std.typecons : tuple; + + auto textBefore = (cast(char[]) code[0..pos]).byCodeUnit; + size_t line = 0; + size_t column = 0; + + foreach (s; textBefore) + { + if (s == '\n') + { + line++; + column = 0; + } + else if (s != '\r') + { + // ignore carriage return + column++; + } + } + return tuple!("line", "column")(line, column); + } + +} + +unittest +{ + import analysis.config : StaticAnalysisConfig, Check; + import analysis.helpers : assertAnalyzerWarnings; + import std.format : format; + import std.stdio : stderr; + + StaticAnalysisConfig sac; + sac.allman_braces_check = Check.enabled; + + assertAnalyzerWarnings(q{ + void testAllman() + { + while (true) { // [warn]: %s + auto f = 1; + } + + do { // [warn]: %s + auto f = 1; + } while (true); + + // inline braces are OK + while (true) { auto f = 1; } + + if (true) { // [warn]: %s + auto f = 1; + } + if (true) { auto f = 1; } + foreach (r; [1]) { // [warn]: %s + } + foreach (r; [1]) { } + foreach_reverse (r; [1]) { // [warn]: %s + } + foreach_reverse (r; [1]) { } + for (int i = 0; i < 10; i++) { // [warn]: %s + } + for (int i = 0; i < 10; i++) { } + + // nested check + while (true) { // [warn]: %s + while (true) { // [warn]: %s + auto f = 1; + } + } + } + }c.format( + AllManCheck.MESSAGE, + AllManCheck.MESSAGE, + AllManCheck.MESSAGE, + AllManCheck.MESSAGE, + AllManCheck.MESSAGE, + AllManCheck.MESSAGE, + AllManCheck.MESSAGE, + AllManCheck.MESSAGE, + ), sac); + + stderr.writeln("Unittest for Allman passed."); +} diff --git a/src/analysis/config.d b/src/analysis/config.d index 32ccc753..a5d5b22a 100644 --- a/src/analysis/config.d +++ b/src/analysis/config.d @@ -156,4 +156,16 @@ struct StaticAnalysisConfig @INI("Check for useless usage of the final attribute") string final_attribute_check = Check.disabled; + + @INI("Check allman brace style") + string allman_braces_check = Check.disabled; + + @INI("Check for trailing whitespace") + string trailing_whitespace_check = Check.disabled; + + @INI("Check for two or more consecutive empty lines") + string consecutive_empty_lines = Check.disabled; + + @INI("If constraint whitespace check") + string if_constraints_index = Check.disabled; } diff --git a/src/analysis/consecutive_empty_lines.d b/src/analysis/consecutive_empty_lines.d new file mode 100644 index 00000000..b28f2d82 --- /dev/null +++ b/src/analysis/consecutive_empty_lines.d @@ -0,0 +1,91 @@ +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) + +module analysis.consecutive_empty_lines; + +import dparse.lexer; +import dparse.ast; +import analysis.base : BaseAnalyzer, Message; +import dsymbol.scope_ : Scope; + +/** +Checks whether a file contains two or more consecutive empty lines +*/ +class ConsecutiveEmptyLines: BaseAnalyzer +{ + /// + this(string fileName, const(ubyte)[] code, bool skipTests = false) + { + super(fileName, null, skipTests); + this.code = code; + } + + override void visit(const Module) + { + findConsecutiveLines(); + } + + alias visit = ASTVisitor.visit; + +private: + + const(ubyte)[] code; + + enum string KEY = "dscanner.style.consecutive_empty_lines"; + enum string MESSAGE = "Consecutive empty lines detected"; + + /** + Searches for two or more consecutive empty lines + */ + void findConsecutiveLines() + { + import std.utf: byCodeUnit; + import std.ascii: isWhite; + + size_t line = 0; + size_t newLineCount = 0; + + foreach (s; (cast(char[]) code).byCodeUnit) + { + if (s == '\n') + { + if (newLineCount >= 2) + addErrorMessage(line, 0, KEY, MESSAGE); + line++; + newLineCount++; + } + // ignore carriage returns for windows compatibility + else if (!(s == '\r' || isWhite(s))) + { + newLineCount = 0; + } + } + } +} + +unittest +{ + import analysis.config : StaticAnalysisConfig, Check; + import analysis.helpers; + import std.stdio; + + StaticAnalysisConfig sac; + sac.consecutive_empty_lines = Check.enabled; + + auto msgs = getAnalyzerWarnings(q{ + void testConsecutiveEmptyLines(){ + + + } + + void foo(){ + + } + }c, sac); + assert(msgs.length == 1); + Message msg = Message("test", 3, 0, ConsecutiveEmptyLines.KEY, ConsecutiveEmptyLines.MESSAGE); + assert(msgs.front == msg); + + stderr.writeln("Unittest for ConsecutiveEmptyLines passed."); +} diff --git a/src/analysis/helpers.d b/src/analysis/helpers.d index f609d660..663990e4 100644 --- a/src/analysis/helpers.d +++ b/src/analysis/helpers.d @@ -40,11 +40,9 @@ S after(S)(S value, S separator) if (isSomeString!S) } /** - * This assert function will analyze the passed in code, get the warnings, - * and make sure they match the warnings in the comments. Warnings are - * marked like so: // [warn]: Failed to do somethings. - */ -void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, +* Get analzer warnings for the given code +*/ +MessageSet getAnalyzerWarnings(string code, const StaticAnalysisConfig config, string file = __FILE__, size_t line = __LINE__) { import analysis.run : parseModule; @@ -58,7 +56,19 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, auto moduleCache = ModuleCache(new CAllocatorImpl!Mallocator); // Run the code and get any warnings - MessageSet rawWarnings = analyze("test", m, config, moduleCache, tokens); + return analyze("test", m, config, moduleCache, tokens, cast(ubyte[]) code); +} + +/** + * This assert function will analyze the passed in code, get the warnings, + * and make sure they match the warnings in the comments. Warnings are + * marked like so: // [warn]: Failed to do somethings. + */ +void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, + string file = __FILE__, size_t line = __LINE__) +{ + + MessageSet rawWarnings = getAnalyzerWarnings(code, config, file, line); string[] codeLines = code.split("\n"); // Get the warnings ordered by line diff --git a/src/analysis/if_constraints_indent.d b/src/analysis/if_constraints_indent.d new file mode 100644 index 00000000..749569e8 --- /dev/null +++ b/src/analysis/if_constraints_indent.d @@ -0,0 +1,166 @@ +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) + +module analysis.if_constraints_index; + +import dparse.lexer; +import dparse.ast; +import analysis.base : BaseAnalyzer, Message; +import dsymbol.scope_ : Scope; + +/** +Checks whether all if constraints have the same indention as their declaration. +*/ +class IfConstraintsIndexCheck : BaseAnalyzer +{ + /// + this(string fileName, const(ubyte)[] code, bool skipTests = false) + { + super(fileName, null, skipTests); + this.code = code; + } + + override void visit(const FunctionDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + override void visit(const InterfaceDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + + override void visit(const ClassDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + override void visit(const TemplateDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + override void visit(const UnionDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + override void visit(const StructDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + override void visit(const Constructor decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.location); + } + + alias visit = ASTVisitor.visit; + +private: + + const(ubyte)[] code; + + enum string KEY = "dscanner.style.if_constraints_index"; + enum string MESSAGE = "If constraints should have the same indentation as the function"; + + /** + Check indentation of constraints + */ + void checkConstraintSpace(const Constraint constraint, const Token token) + { + checkConstraintSpace(constraint, token.index); + } + + void checkConstraintSpace(const Constraint constraint, size_t location) + { + import std.utf : byCodeUnit; + import std.ascii : isWhite; + + import std.algorithm; + import std.range; + + auto lineIndent = code[0 .. constraint.location].retro + .until('\n') + .walkLength; + + auto tokenLineStart = code[0 .. location].retro + .until('\n') + .walkLength; + + auto tokenIndent = code[location - tokenLineStart .. location] + .until!(x => !x.isWhite) + .walkLength; + + if (tokenIndent != lineIndent) + addErrorMessage(constraint.expression.line, constraint.expression.column, KEY, MESSAGE); + } +} + +unittest +{ + import analysis.config : StaticAnalysisConfig, Check; + import analysis.helpers; + import std.format : format; + import std.stdio : stderr; + + StaticAnalysisConfig sac; + sac.if_constraints_index = Check.enabled; + + assertAnalyzerWarnings(q{ +void foo(R)(R r) +if (R == int) +{} + +void foo(R)(R r) + if (R == int) // [warn]: %s +{} + }c.format( + IfConstraintsIndexCheck.MESSAGE, + ), sac); + + assertAnalyzerWarnings(q{ + void foo(R)(R r) + if (R == int) + {} + + void foo(R)(R r) +if (R == int) // [warn]: %s + {} + + void foo(R)(R r) + if (R == int) // [warn]: %s + {} + }c.format( + IfConstraintsIndexCheck.MESSAGE, + IfConstraintsIndexCheck.MESSAGE, + ), sac); + + assertAnalyzerWarnings(q{ + struct Foo(R) + if (R == int) + {} + + struct Foo(R) +if (R == int) // [warn]: %s + {} + + struct Foo(R) + if (R == int) // [warn]: %s + {} + }c.format( + IfConstraintsIndexCheck.MESSAGE, + IfConstraintsIndexCheck.MESSAGE, + ), sac); + + stderr.writeln("Unittest for IfConstraintsIndexCheck passed."); +} diff --git a/src/analysis/run.d b/src/analysis/run.d index 0bc5ec4c..4219be67 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -63,6 +63,10 @@ import analysis.auto_function; import analysis.imports_sortedness; import analysis.explicitly_annotated_unittests; import analysis.final_attribute; +import analysis.allman; +import analysis.trailing_whitespace; +import analysis.consecutive_empty_lines; +import analysis.if_constraints_index; import dsymbol.string_interning : internString; import dsymbol.scope_; @@ -128,7 +132,7 @@ void generateReport(string[] fileNames, const StaticAnalysisConfig config, const(Token)[] tokens; const Module m = parseModule(fileName, code, &r, cache, true, tokens, &lineOfCodeCount); stats.visit(m); - MessageSet results = analyze(fileName, m, config, moduleCache, tokens, true); + MessageSet results = analyze(fileName, m, config, moduleCache, tokens, code, true); foreach (result; results[]) { writeJSON(result.key, result.fileName, result.line, result.column, result.message); @@ -171,7 +175,7 @@ bool analyze(string[] fileNames, const StaticAnalysisConfig config, assert(m); if (errorCount > 0 || (staticAnalyze && warningCount > 0)) hasErrors = true; - MessageSet results = analyze(fileName, m, config, moduleCache, tokens, staticAnalyze); + MessageSet results = analyze(fileName, m, config, moduleCache, tokens, code, staticAnalyze); if (results is null) continue; foreach (result; results[]) @@ -201,7 +205,7 @@ const(Module) parseModule(string fileName, ubyte[] code, RollbackAllocator* p, } MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig analysisConfig, - ref ModuleCache moduleCache, const(Token)[] tokens, bool staticAnalyze = true) + ref ModuleCache moduleCache, const(Token)[] tokens, const(ubyte)[] code, bool staticAnalyze = true) { import dsymbol.symbol : DSymbol; @@ -369,10 +373,24 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a if (analysisConfig.explicitly_annotated_unittests != Check.disabled) checks ~= new ExplicitlyAnnotatedUnittestCheck(fileName, analysisConfig.explicitly_annotated_unittests == Check.skipTests && !ut); + if (analysisConfig.allman_braces_check != Check.disabled) + checks ~= new AllManCheck(fileName, code, + analysisConfig.allman_braces_check == Check.skipTests && !ut); if (analysisConfig.final_attribute_check != Check.disabled) checks ~= new FinalAttributeChecker(fileName, analysisConfig.final_attribute_check == Check.skipTests && !ut); + if (analysisConfig.trailing_whitespace_check != Check.disabled) + checks ~= new TrailingWhitespaceCheck(fileName, code, + analysisConfig.trailing_whitespace_check == Check.skipTests && !ut); + + if (analysisConfig.consecutive_empty_lines != Check.disabled) + checks ~= new ConsecutiveEmptyLines(fileName, code, + analysisConfig.consecutive_empty_lines == Check.skipTests && !ut); + + if (analysisConfig.if_constraints_index != Check.disabled) + checks ~= new IfConstraintsIndexCheck(fileName, code, + analysisConfig.if_constraints_index == Check.skipTests && !ut); version (none) if (analysisConfig.redundant_if_check != Check.disabled) diff --git a/src/analysis/trailing_whitespace.d b/src/analysis/trailing_whitespace.d new file mode 100644 index 00000000..fee59d5b --- /dev/null +++ b/src/analysis/trailing_whitespace.d @@ -0,0 +1,93 @@ +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) + +module analysis.trailing_whitespace; + +import dparse.lexer; +import dparse.ast; +import analysis.base : BaseAnalyzer, Message; +import dsymbol.scope_ : Scope; + +/** +Checks whether a file contains trailing whitespace +*/ +class TrailingWhitespaceCheck : BaseAnalyzer +{ + /// + this(string fileName, const(ubyte)[] code, bool skipTests = false) + { + super(fileName, null, skipTests); + this.code = code; + } + + override void visit(const Module) + { + findTrailingWhitespace(); + } + + alias visit = ASTVisitor.visit; + +private: + + const(ubyte)[] code; + + enum string KEY = "dscanner.style.trailing_whitespace"; + enum string MESSAGE = "Trailing whitespace detected"; + + /** + Searches for trailing whitespace + */ + void findTrailingWhitespace() + { + import std.utf : byCodeUnit; + import std.ascii : isWhite; + import std.typecons : tuple; + + auto text = (cast(char[]) code).byCodeUnit; + size_t line = 0; + size_t column = 0; + bool hasWhitespace; + + foreach (s; text) + { + if (s == '\n') + { + if (hasWhitespace) + addErrorMessage(line, column, KEY, MESSAGE); + line++; + column = 0; + } + else + { + if (isWhite(s)) + hasWhitespace = true; + else + hasWhitespace = false; + column++; + } + } + } +} + +unittest +{ + import analysis.config : StaticAnalysisConfig, Check; + import analysis.helpers; + import std.stdio; + + StaticAnalysisConfig sac; + sac.trailing_whitespace_check = Check.enabled; + + auto msgs = getAnalyzerWarnings(q{ + void testTrailing() + { + a = 1; + } + }c, sac); + assert(msgs.length == 1); + Message msg = Message("test", 3, 11, TrailingWhitespaceCheck.KEY, TrailingWhitespaceCheck.MESSAGE); + assert(msgs.front == msg); + + stderr.writeln("Unittest for TrailingWhitespaceCheck passed."); +}