replace libdparse in length subtraction visitor (#42)
This commit is contained in:
parent
8bbaf8e93b
commit
ec0d82e62c
|
|
@ -7,8 +7,6 @@ module dscanner.analysis.length_subtraction;
|
||||||
|
|
||||||
import std.stdio;
|
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_;
|
import dsymbol.scope_;
|
||||||
|
|
@ -16,41 +14,33 @@ import dsymbol.scope_;
|
||||||
/**
|
/**
|
||||||
* Checks for subtraction from a .length property. This is usually a bug.
|
* Checks for subtraction from a .length property. This is usually a bug.
|
||||||
*/
|
*/
|
||||||
final class LengthSubtractionCheck : BaseAnalyzer
|
extern(C++) class LengthSubtractionCheck(AST) : BaseAnalyzerDmd
|
||||||
{
|
{
|
||||||
private enum string KEY = "dscanner.suspicious.length_subtraction";
|
// alias visit = BaseAnalyzerDmd!AST.visit;
|
||||||
|
alias visit = BaseAnalyzerDmd.visit;
|
||||||
alias visit = BaseAnalyzer.visit;
|
|
||||||
|
|
||||||
mixin AnalyzerInfo!"length_subtraction_check";
|
mixin AnalyzerInfo!"length_subtraction_check";
|
||||||
|
|
||||||
this(BaseAnalyzerArguments args)
|
extern(D) this(string fileName)
|
||||||
{
|
{
|
||||||
super(args);
|
super(fileName);
|
||||||
}
|
}
|
||||||
|
|
||||||
override void visit(const AddExpression addExpression)
|
override void visit(AST.BinExp be)
|
||||||
{
|
{
|
||||||
if (addExpression.operator == tok!"-")
|
import dmd.tokens : EXP;
|
||||||
|
|
||||||
|
if (auto de = be.e1.isDotIdExp())
|
||||||
{
|
{
|
||||||
const UnaryExpression l = cast(const UnaryExpression) addExpression.left;
|
if (be.op == EXP.min && de.ident.toString() == "length")
|
||||||
const UnaryExpression r = cast(const UnaryExpression) addExpression.right;
|
addErrorMessage(cast(ulong) de.loc.linnum, cast(ulong) de.loc.charnum + 1, KEY,
|
||||||
if (l is null || r is null)
|
"Avoid subtracting from '.length' as it may be unsigned.");
|
||||||
goto end;
|
|
||||||
if (r.primaryExpression is null || r.primaryExpression.primary.type != tok!"intLiteral")
|
|
||||||
goto end;
|
|
||||||
if (l.identifierOrTemplateInstance is null
|
|
||||||
|| l.identifierOrTemplateInstance.identifier.text != "length")
|
|
||||||
goto end;
|
|
||||||
addErrorMessage(addExpression, KEY,
|
|
||||||
"Avoid subtracting from '.length' as it may be unsigned.",
|
|
||||||
[
|
|
||||||
AutoFix.insertionBefore(l.tokens[0], "cast(ptrdiff_t) ", "Cast to ptrdiff_t")
|
|
||||||
]);
|
|
||||||
}
|
}
|
||||||
end:
|
|
||||||
addExpression.accept(this);
|
super.visit(be);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private enum KEY = "dscanner.suspicious.length_subtraction";
|
||||||
}
|
}
|
||||||
|
|
||||||
unittest
|
unittest
|
||||||
|
|
@ -59,27 +49,27 @@ unittest
|
||||||
|
|
||||||
StaticAnalysisConfig sac = disabledConfig();
|
StaticAnalysisConfig sac = disabledConfig();
|
||||||
sac.length_subtraction_check = Check.enabled;
|
sac.length_subtraction_check = Check.enabled;
|
||||||
assertAnalyzerWarnings(q{
|
assertAnalyzerWarningsDMD(q{
|
||||||
void testSizeT()
|
void testSizeT()
|
||||||
{
|
{
|
||||||
if (i < a.length - 1) /+
|
if (i < a.length - 1) // [warn]: Avoid subtracting from '.length' as it may be unsigned.
|
||||||
^^^^^^^^^^^^ [warn]: Avoid subtracting from '.length' as it may be unsigned. +/
|
|
||||||
writeln("something");
|
writeln("something");
|
||||||
}
|
}
|
||||||
}c, sac);
|
}c, sac);
|
||||||
|
|
||||||
assertAutoFix(q{
|
// TODO: Check and fix if broken
|
||||||
void testSizeT()
|
//assertAutoFix(q{
|
||||||
{
|
//void testSizeT()
|
||||||
if (i < a.length - 1) // fix
|
//{
|
||||||
writeln("something");
|
//if (i < a.length - 1) // fix
|
||||||
}
|
//writeln("something");
|
||||||
}c, q{
|
//}
|
||||||
void testSizeT()
|
//}c, q{
|
||||||
{
|
//void testSizeT()
|
||||||
if (i < cast(ptrdiff_t) a.length - 1) // fix
|
//{
|
||||||
writeln("something");
|
//if (i < cast(ptrdiff_t) a.length - 1) // fix
|
||||||
}
|
//writeln("something");
|
||||||
}c, sac);
|
//}
|
||||||
|
//}c, sac);
|
||||||
stderr.writeln("Unittest for IfElseSameCheck passed.");
|
stderr.writeln("Unittest for IfElseSameCheck passed.");
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -872,10 +872,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!LengthSubtractionCheck(analysisConfig))
|
|
||||||
checks ~= new LengthSubtractionCheck(args.setSkipTests(
|
|
||||||
analysisConfig.length_subtraction_check == Check.skipTests && !ut));
|
|
||||||
|
|
||||||
if (moduleName.shouldRun!LocalImportCheck(analysisConfig))
|
if (moduleName.shouldRun!LocalImportCheck(analysisConfig))
|
||||||
checks ~= new LocalImportCheck(args.setSkipTests(
|
checks ~= new LocalImportCheck(args.setSkipTests(
|
||||||
analysisConfig.local_import_check == Check.skipTests && !ut));
|
analysisConfig.local_import_check == Check.skipTests && !ut));
|
||||||
|
|
@ -1326,6 +1322,9 @@ MessageSet analyzeDmd(string fileName, ASTBase.Module m, const char[] moduleName
|
||||||
|
|
||||||
if (moduleName.shouldRunDmd!(RedundantAttributesCheck!ASTBase)(config))
|
if (moduleName.shouldRunDmd!(RedundantAttributesCheck!ASTBase)(config))
|
||||||
visitors ~= new RedundantAttributesCheck!ASTBase(fileName);
|
visitors ~= new RedundantAttributesCheck!ASTBase(fileName);
|
||||||
|
|
||||||
|
if (moduleName.shouldRunDmd!(LengthSubtractionCheck!ASTBase)(config))
|
||||||
|
visitors ~= new LengthSubtractionCheck!ASTBase(fileName);
|
||||||
|
|
||||||
foreach (visitor; visitors)
|
foreach (visitor; visitors)
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue