replace libdparse in logic precedence visitor (#54)
This commit is contained in:
parent
b90511573d
commit
9f961bf051
|
|
@ -5,12 +5,8 @@
|
||||||
|
|
||||||
module dscanner.analysis.logic_precedence;
|
module dscanner.analysis.logic_precedence;
|
||||||
|
|
||||||
import std.stdio;
|
|
||||||
import dparse.ast;
|
|
||||||
import dparse.lexer;
|
|
||||||
import dscanner.analysis.base;
|
import dscanner.analysis.base;
|
||||||
import dscanner.analysis.helpers;
|
import dscanner.analysis.helpers;
|
||||||
import dsymbol.scope_;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Checks for code with confusing && and || operator precedence
|
* Checks for code with confusing && and || operator precedence
|
||||||
|
|
@ -19,50 +15,64 @@ import dsymbol.scope_;
|
||||||
* if (a && (b || c)) // good
|
* if (a && (b || c)) // good
|
||||||
* ---
|
* ---
|
||||||
*/
|
*/
|
||||||
final class LogicPrecedenceCheck : BaseAnalyzer
|
extern(C++) class LogicPrecedenceCheck(AST) : BaseAnalyzerDmd
|
||||||
{
|
{
|
||||||
alias visit = BaseAnalyzer.visit;
|
|
||||||
|
|
||||||
enum string KEY = "dscanner.confusing.logical_precedence";
|
enum string KEY = "dscanner.confusing.logical_precedence";
|
||||||
mixin AnalyzerInfo!"logical_precedence_check";
|
mixin AnalyzerInfo!"logical_precedence_check";
|
||||||
|
alias visit = BaseAnalyzerDmd.visit;
|
||||||
|
|
||||||
this(BaseAnalyzerArguments args)
|
extern(D) this(string fileName, bool skipTests = false)
|
||||||
{
|
{
|
||||||
super(args);
|
super(fileName, skipTests);
|
||||||
}
|
}
|
||||||
|
|
||||||
override void visit(const OrOrExpression orOr)
|
override void visit(AST.LogicalExp le)
|
||||||
{
|
{
|
||||||
if (orOr.left is null || orOr.right is null)
|
import dmd.tokens : EXP;
|
||||||
return;
|
|
||||||
const AndAndExpression left = cast(AndAndExpression) orOr.left;
|
auto left = le.e1.isLogicalExp();
|
||||||
const AndAndExpression right = cast(AndAndExpression) orOr.right;
|
auto right = le.e2.isLogicalExp();
|
||||||
if (left is null && right is null)
|
|
||||||
return;
|
if (left)
|
||||||
if ((left !is null && left.right is null) && (right !is null && right.right is null))
|
left = left.op == EXP.andAnd ? left : null;
|
||||||
return;
|
if (right)
|
||||||
addErrorMessage(orOr, KEY,
|
right = right.op == EXP.andAnd ? right : null;
|
||||||
|
|
||||||
|
if (le.op != EXP.orOr)
|
||||||
|
goto END;
|
||||||
|
|
||||||
|
if (!left && !right)
|
||||||
|
goto END;
|
||||||
|
|
||||||
|
if ((left && left.parens) || (right && right.parens))
|
||||||
|
goto END;
|
||||||
|
|
||||||
|
if ((left !is null && left.e2 is null) && (right !is null && right.e2 is null))
|
||||||
|
goto END;
|
||||||
|
|
||||||
|
addErrorMessage(cast(ulong) le.loc.linnum, cast(ulong) le.loc.charnum, KEY,
|
||||||
"Use parenthesis to clarify this expression.");
|
"Use parenthesis to clarify this expression.");
|
||||||
orOr.accept(this);
|
|
||||||
|
END:
|
||||||
|
super.visit(le);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
unittest
|
unittest
|
||||||
{
|
{
|
||||||
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
|
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
|
||||||
|
import std.stdio : stderr;
|
||||||
|
|
||||||
StaticAnalysisConfig sac = disabledConfig();
|
StaticAnalysisConfig sac = disabledConfig();
|
||||||
sac.logical_precedence_check = Check.enabled;
|
sac.logical_precedence_check = Check.enabled;
|
||||||
assertAnalyzerWarnings(q{
|
assertAnalyzerWarningsDMD(q{
|
||||||
void testFish()
|
void testFish()
|
||||||
{
|
{
|
||||||
if (a && b || c) {} /+
|
if (a && b || c) {} // [warn]: Use parenthesis to clarify this expression.
|
||||||
^^^^^^^^^^^ [warn]: Use parenthesis to clarify this expression. +/
|
|
||||||
if ((a && b) || c) {} // Good
|
if ((a && b) || c) {} // Good
|
||||||
if (b || c && d) {} /+
|
if (b || c && d) {} // [warn]: Use parenthesis to clarify this expression.
|
||||||
^^^^^^^^^^^ [warn]: Use parenthesis to clarify this expression. +/
|
|
||||||
if (b || (c && d)) {} // Good
|
if (b || (c && d)) {} // Good
|
||||||
}
|
}
|
||||||
}c, sac);
|
}c, sac);
|
||||||
stderr.writeln("Unittest for LogicPrecedenceCheck passed.");
|
stderr.writeln("Unittest for LogicPrecedenceCheck passed.");
|
||||||
}
|
}
|
||||||
|
|
@ -876,10 +876,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
|
||||||
checks ~= new LabelVarNameCheck(args.setSkipTests(
|
checks ~= new LabelVarNameCheck(args.setSkipTests(
|
||||||
analysisConfig.label_var_same_name_check == Check.skipTests && !ut));
|
analysisConfig.label_var_same_name_check == Check.skipTests && !ut));
|
||||||
|
|
||||||
if (moduleName.shouldRun!LogicPrecedenceCheck(analysisConfig))
|
|
||||||
checks ~= new LogicPrecedenceCheck(args.setSkipTests(
|
|
||||||
analysisConfig.logical_precedence_check == Check.skipTests && !ut));
|
|
||||||
|
|
||||||
if (moduleName.shouldRun!MismatchedArgumentCheck(analysisConfig))
|
if (moduleName.shouldRun!MismatchedArgumentCheck(analysisConfig))
|
||||||
checks ~= new MismatchedArgumentCheck(args.setSkipTests(
|
checks ~= new MismatchedArgumentCheck(args.setSkipTests(
|
||||||
analysisConfig.mismatched_args_check == Check.skipTests && !ut));
|
analysisConfig.mismatched_args_check == Check.skipTests && !ut));
|
||||||
|
|
@ -1332,6 +1328,12 @@ MessageSet analyzeDmd(string fileName, ASTBase.Module m, const char[] moduleName
|
||||||
|
|
||||||
if (moduleName.shouldRunDmd!(AutoRefAssignmentCheck!ASTBase)(config))
|
if (moduleName.shouldRunDmd!(AutoRefAssignmentCheck!ASTBase)(config))
|
||||||
visitors ~= new AutoRefAssignmentCheck!ASTBase(fileName);
|
visitors ~= new AutoRefAssignmentCheck!ASTBase(fileName);
|
||||||
|
|
||||||
|
if (moduleName.shouldRunDmd!(LogicPrecedenceCheck!ASTBase)(config))
|
||||||
|
visitors ~= new LogicPrecedenceCheck!ASTBase(
|
||||||
|
fileName,
|
||||||
|
config.logical_precedence_check == Check.skipTests && !ut
|
||||||
|
);
|
||||||
|
|
||||||
foreach (visitor; visitors)
|
foreach (visitor; visitors)
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -228,7 +228,7 @@ abstract class UnusedIdentifierCheck : BaseAnalyzer
|
||||||
else if (idt.templateInstance && idt.templateInstance.identifier != tok!"")
|
else if (idt.templateInstance && idt.templateInstance.identifier != tok!"")
|
||||||
variableUsed(idt.templateInstance.identifier.text);
|
variableUsed(idt.templateInstance.identifier.text);
|
||||||
}
|
}
|
||||||
if (mixinDepth > 0 && primary.primary == tok!"stringLiteral"
|
if ((mixinDepth > 0 && primary.primary == tok!"stringLiteral")
|
||||||
|| primary.primary == tok!"wstringLiteral"
|
|| primary.primary == tok!"wstringLiteral"
|
||||||
|| primary.primary == tok!"dstringLiteral")
|
|| primary.primary == tok!"dstringLiteral")
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue