replace libdparse in logic precedence visitor (#54)

This commit is contained in:
lucica28 2022-12-02 12:50:55 +02:00 committed by Vladiwostok
parent 7faa2cbae3
commit da10937067
3 changed files with 43 additions and 31 deletions

View File

@ -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,48 +15,62 @@ 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);

View File

@ -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));
@ -1333,6 +1329,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)
{ {
m.accept(visitor); m.accept(visitor);

View File

@ -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")
{ {