mirror of
https://github.com/dlang-community/D-Scanner.git
synced 2025-04-26 21:30:14 +03:00
Replace libdparse with DMD in AlwaysCurlyCheck (#109)
This commit is contained in:
Vladiwostok
Vladiwostok
parent
ee4e19292d
commit
7288aea5f8
2 changed files with 146 additions and 111 deletions
|
|
@ -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.");
|
||||
}+/
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue