diff --git a/src/dscanner/analysis/always_curly.d b/src/dscanner/analysis/always_curly.d index e320fcd..bce21c9 100644 --- a/src/dscanner/analysis/always_curly.d +++ b/src/dscanner/analysis/always_curly.d @@ -4,179 +4,212 @@ module dscanner.analysis.always_curly; -import dparse.lexer; -import dparse.ast; import dscanner.analysis.base; -import dsymbol.scope_ : Scope; -import std.array : back, front; -import std.algorithm; -import std.range; -import std.stdio; - -final class AlwaysCurlyCheck : BaseAnalyzer +// TODO: Fix Autofix +extern (C++) class AlwaysCurlyCheck(AST) : BaseAnalyzerDmd { + alias visit = BaseAnalyzerDmd.visit; mixin AnalyzerInfo!"always_curly_check"; - alias visit = BaseAnalyzer.visit; + private enum string KEY = "dscanner.style.always_curly"; + private enum string MESSAGE_POSTFIX = " must be follow by a BlockStatement aka. { }"; + + private bool hasCurlyBraces; + private bool inCurlyStatement; /// - this(BaseAnalyzerArguments args) + extern (D) this(string fileName, bool skipTests = false) { - super(args); + super(fileName, skipTests); } - void test(L, B)(L loc, B s, string stmtKind) + mixin VisitBraceStatement!(AST.IfStatement, "if"); + mixin VisitBraceStatement!(AST.ForStatement, "for"); + mixin VisitBraceStatement!(AST.ForeachStatement, "foreach"); + mixin VisitBraceStatement!(AST.ForeachRangeStatement, "foreach"); + mixin VisitBraceStatement!(AST.WhileStatement, "while"); + mixin VisitBraceStatement!(AST.DoStatement, "do"); + mixin VisitBraceStatement!(AST.Catch, "catch"); + + private template VisitBraceStatement(NodeType, string nodeName) { - if (!is(s == BlockStatement)) + override void visit(NodeType node) { - if (!s.tokens.empty) + auto oldHasCurlyBraces = hasCurlyBraces; + auto oldInCurlyStatement = inCurlyStatement; + hasCurlyBraces = false; + inCurlyStatement = true; + super.visit(node); + + if (!hasCurlyBraces) { - AutoFix af = AutoFix.insertionBefore(s.tokens.front, " { ") - .concat(AutoFix.insertionAfter(s.tokens.back, " } ")); - af.name = "Wrap in braces"; - - addErrorMessage(loc, KEY, stmtKind ~ MESSAGE_POSTFIX, [af]); - } - else - { - addErrorMessage(loc, KEY, stmtKind ~ MESSAGE_POSTFIX); + auto msg = nodeName ~ MESSAGE_POSTFIX; + addErrorMessage(node.loc.linnum, node.loc.charnum, KEY, msg); } + + hasCurlyBraces = oldHasCurlyBraces; + inCurlyStatement = oldInCurlyStatement; } } - override void visit(const(IfStatement) stmt) + override void visit(AST.CompoundStatement cs) { - auto s = stmt.thenStatement.statement; - this.test(stmt.thenStatement, s, "if"); - if (stmt.elseStatement !is null) - { - auto e = stmt.elseStatement.statement; - this.test(stmt.elseStatement, e, "else"); - } + if (inCurlyStatement) + hasCurlyBraces = true; + super.visit(cs); } - override void visit(const(ForStatement) stmt) + override void visit(AST.TryCatchStatement tryCatchStatement) { - auto s = stmt.declarationOrStatement; - if (s.statement !is null) - { - this.test(s, s, "for"); - } + auto oldHasCurlyBraces = hasCurlyBraces; + auto oldInCurlyStatement = inCurlyStatement; + hasCurlyBraces = false; + inCurlyStatement = true; + + checkStatement(tryCatchStatement._body, "try"); + + hasCurlyBraces = oldHasCurlyBraces; + inCurlyStatement = oldInCurlyStatement; + + foreach (catchStatement; *tryCatchStatement.catches) + visit(catchStatement); } - override void visit(const(ForeachStatement) stmt) + override void visit(AST.TryFinallyStatement tryFinallyStatement) { - auto s = stmt.declarationOrStatement; - if (s.statement !is null) + auto oldHasCurlyBraces = hasCurlyBraces; + auto oldInCurlyStatement = inCurlyStatement; + + if (tryFinallyStatement._body.isTryCatchStatement()) { - this.test(s, s, "foreach"); + tryFinallyStatement._body.accept(this); } + else + { + hasCurlyBraces = false; + inCurlyStatement = true; + checkStatement(tryFinallyStatement._body, "try"); + } + + hasCurlyBraces = false; + inCurlyStatement = true; + checkStatement(tryFinallyStatement.finalbody, "finally"); + + hasCurlyBraces = oldHasCurlyBraces; + inCurlyStatement = oldInCurlyStatement; } - override void visit(const(TryStatement) stmt) + extern (D) private void checkStatement(AST.Statement statement, string statementName) { - auto s = stmt.declarationOrStatement; - if (s.statement !is null) - { - this.test(s, s, "try"); - } + statement.accept(this); - if (stmt.catches !is null) + if (!hasCurlyBraces) { - foreach (const(Catch) ct; stmt.catches.catches) - { - this.test(ct, ct.declarationOrStatement, "catch"); - } - if (stmt.catches.lastCatch !is null) - { - auto sncnd = stmt.catches.lastCatch.statementNoCaseNoDefault; - if (sncnd !is null) - { - this.test(stmt.catches.lastCatch, sncnd, "finally"); - } - } + auto msg = statementName ~ MESSAGE_POSTFIX; + addErrorMessage(statement.loc.linnum, statement.loc.charnum, KEY, msg); } } - - override void visit(const(WhileStatement) stmt) - { - auto s = stmt.declarationOrStatement; - if (s.statement !is null) - { - this.test(s, s, "while"); - } - } - - override void visit(const(DoStatement) stmt) - { - auto s = stmt.statementNoCaseNoDefault; - if (s !is null) - { - this.test(s, s, "do"); - } - } - - enum string KEY = "dscanner.style.always_curly"; - enum string MESSAGE_POSTFIX = " must be follow by a BlockStatement aka. { }"; } unittest { import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix; + import dscanner.analysis.helpers : assertAnalyzerWarningsDMD, assertAutoFix; import std.stdio : stderr; StaticAnalysisConfig sac = disabledConfig(); sac.always_curly_check = Check.enabled; - - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ void testIf() { - if(true) return; // [warn]: if must be follow by a BlockStatement aka. { } + if (true) + { + return; + } + + if (true) return; // [warn]: if must be follow by a BlockStatement aka. { } } }, sac); - assertAnalyzerWarnings(q{ - void testIf() + assertAnalyzerWarningsDMD(q{ + void testFor() { - if(true) return; /+ - ^^^^^^^ [warn]: if must be follow by a BlockStatement aka. { } +/ + for (int i = 0; i < 10; ++i) + { + return; + } + + for (int i = 0; i < 10; ++i) return; // [warn]: for must be follow by a BlockStatement aka. { } } }, sac); - assertAnalyzerWarnings(q{ - void testIf() + assertAnalyzerWarningsDMD(q{ + void testForEach() { - for(int i = 0; i < 10; ++i) return; // [warn]: for must be follow by a BlockStatement aka. { } + foreach (it; 0 .. 10) + { + return; + } + + foreach (it; 0 .. 10) return; // [warn]: foreach must be follow by a BlockStatement aka. { } } }, sac); - assertAnalyzerWarnings(q{ - void testIf() + assertAnalyzerWarningsDMD(q{ + void testWhile() { - foreach(it; 0 .. 10) return; // [warn]: foreach must be follow by a BlockStatement aka. { } + while (true) + { + return; + } + + while (true) return; // [warn]: while must be follow by a BlockStatement aka. { } } }, sac); - assertAnalyzerWarnings(q{ - void testIf() + assertAnalyzerWarningsDMD(q{ + void testDoWhile() { - while(true) return; // [warn]: while must be follow by a BlockStatement aka. { } + do + { + return; + } while (true); + + do return; while (true); return; // [warn]: do must be follow by a BlockStatement aka. { } } }, sac); - assertAnalyzerWarnings(q{ - void testIf() + assertAnalyzerWarningsDMD(q{ + void testTryCatchFinally() { - do return; while(true); return; // [warn]: do must be follow by a BlockStatement aka. { } + try + { + return; + } + catch (Exception e) + { + return; + } + finally + { + return; + } + + try return; // [warn]: try must be follow by a BlockStatement aka. { } + catch (Exception e) return; // [warn]: catch must be follow by a BlockStatement aka. { } + finally return; // [warn]: finally must be follow by a BlockStatement aka. { } } - }, sac); + }c, sac); + + stderr.writeln("Unittest for AutoFix AlwaysCurly passed."); } -unittest { +/+ TODO: Fix Autofix +unittest +{ import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix; + import dscanner.analysis.helpers : assertAnalyzerWarningsDMD, assertAutoFix; import std.stdio : stderr; StaticAnalysisConfig sac = disabledConfig(); @@ -223,5 +256,5 @@ unittest { }c, sac); - stderr.writeln("Unittest for AlwaysCurly passed."); -} + stderr.writeln("Unittest for AutoFix AlwaysCurly passed."); +}+/ diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index 1c95a51..5e3a634 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -878,10 +878,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName, checks ~= new AllManCheck(args.setSkipTests( analysisConfig.allman_braces_check == Check.skipTests && !ut)); - if (moduleName.shouldRun!AlwaysCurlyCheck(analysisConfig)) - checks ~= new AlwaysCurlyCheck(args.setSkipTests( - analysisConfig.always_curly_check == Check.skipTests && !ut)); - if (moduleName.shouldRun!HasPublicExampleCheck(analysisConfig)) checks ~= new HasPublicExampleCheck(args.setSkipTests( analysisConfig.has_public_example == Check.skipTests && !ut)); @@ -1344,6 +1340,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN config.lambda_return_check == Check.skipTests && !ut ); + if (moduleName.shouldRunDmd!(AlwaysCurlyCheck!ASTCodegen)(config)) + visitors ~= new AlwaysCurlyCheck!ASTCodegen( + fileName, + config.always_curly_check == Check.skipTests && !ut + ); + foreach (visitor; visitors) { m.accept(visitor);