Replace libdparse in UnusedResultChecker (#80)
* Replace libdparse with DMD in UnusedResultChecker * Run semantic analysis in unit test * Add more unit tests * Remove debug prints * Fix no return test * Remove semantic analysis
This commit is contained in:
Vladiwostok
GitHub
parent
630bd37b32
commit
c9829ebf2d
|
|
@ -853,10 +853,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
|
|||
checks ~= new IfConstraintsIndentCheck(args.setSkipTests(
|
||||
analysisConfig.if_constraints_indent == Check.skipTests && !ut));
|
||||
|
||||
if (moduleName.shouldRun!UnusedResultChecker(analysisConfig))
|
||||
checks ~= new UnusedResultChecker(args.setSkipTests(
|
||||
analysisConfig.unused_result == Check.skipTests && !ut));
|
||||
|
||||
return checks;
|
||||
}
|
||||
|
||||
|
|
@ -1364,6 +1360,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN
|
|||
config.max_line_length
|
||||
);
|
||||
|
||||
if (moduleName.shouldRunDmd!(UnusedResultChecker!ASTCodegen)(config))
|
||||
visitors ~= new UnusedResultChecker!ASTCodegen(
|
||||
fileName,
|
||||
config.unused_result == Check.skipTests && !ut
|
||||
);
|
||||
|
||||
foreach (visitor; visitors)
|
||||
{
|
||||
m.accept(visitor);
|
||||
|
|
|
|||
|
|
@ -4,15 +4,7 @@
|
|||
// http://www.boost.org/LICENSE_1_0.txt)
|
||||
module dscanner.analysis.unused_result;
|
||||
|
||||
import dparse.ast;
|
||||
import dparse.lexer;
|
||||
import dscanner.analysis.base;
|
||||
import dscanner.analysis.mismatched_args : IdentVisitor, resolveSymbol;
|
||||
import dscanner.utils;
|
||||
import dsymbol.scope_;
|
||||
import dsymbol.symbol;
|
||||
import std.algorithm : canFind, countUntil;
|
||||
import std.range : retro;
|
||||
|
||||
/**
|
||||
* Checks for function call statements which call non-void functions.
|
||||
|
|
@ -24,105 +16,212 @@ import std.range : retro;
|
|||
* When the return value is intentionally discarded, `cast(void)` can
|
||||
* be prepended to silence the check.
|
||||
*/
|
||||
final class UnusedResultChecker : BaseAnalyzer
|
||||
extern (C++) class UnusedResultChecker(AST) : BaseAnalyzerDmd
|
||||
{
|
||||
alias visit = BaseAnalyzer.visit;
|
||||
alias visit = BaseAnalyzerDmd.visit;
|
||||
mixin AnalyzerInfo!"unused_result";
|
||||
|
||||
mixin AnalyzerInfo!"unused_result";
|
||||
private enum KEY = "dscanner.performance.enum_array_literal";
|
||||
private enum string MSG = "Function return value is discarded";
|
||||
|
||||
private:
|
||||
private bool[string] nonVoidFuncs;
|
||||
private string[] aggregateStack;
|
||||
|
||||
enum string KEY = "dscanner.unused_result";
|
||||
enum string MSG = "Function return value is discarded";
|
||||
extern (D) this(string fileName, bool skipTests = false)
|
||||
{
|
||||
super(fileName, skipTests);
|
||||
}
|
||||
|
||||
public:
|
||||
private template VisitAggregate(NodeType)
|
||||
{
|
||||
override void visit(NodeType aggregate)
|
||||
{
|
||||
string name = cast(string) aggregate.ident.toString();
|
||||
aggregateStack ~= name;
|
||||
super.visit(aggregate);
|
||||
aggregateStack.length -= 1;
|
||||
}
|
||||
}
|
||||
|
||||
const(DSymbol)* void_;
|
||||
const(DSymbol)* noreturn_;
|
||||
mixin VisitAggregate!(AST.StructDeclaration);
|
||||
mixin VisitAggregate!(AST.ClassDeclaration);
|
||||
|
||||
///
|
||||
this(BaseAnalyzerArguments args)
|
||||
{
|
||||
super(args);
|
||||
void_ = sc.getSymbolsByName(internString("void"))[0];
|
||||
auto symbols = sc.getSymbolsByName(internString("noreturn"));
|
||||
if (symbols.length > 0)
|
||||
noreturn_ = symbols[0];
|
||||
}
|
||||
override void visit(AST.FuncDeclaration funcDeclaration)
|
||||
{
|
||||
import dmd.astenums : TY;
|
||||
|
||||
override void visit(const(ExpressionStatement) decl)
|
||||
{
|
||||
import std.typecons : scoped;
|
||||
auto typeFunc = funcDeclaration.type.isTypeFunction();
|
||||
if (typeFunc && typeFunc.next && typeFunc.next.ty != TY.Tvoid && typeFunc.next.ty != TY.Tnoreturn)
|
||||
{
|
||||
auto typeIdent = typeFunc.next.isTypeIdentifier();
|
||||
bool isNoReturn = typeIdent is null ? false : typeIdent.ident.toString() == "noreturn";
|
||||
|
||||
super.visit(decl);
|
||||
if (!decl.expression)
|
||||
return;
|
||||
if (decl.expression.items.length != 1)
|
||||
return;
|
||||
auto ue = cast(UnaryExpression) decl.expression.items[0];
|
||||
if (!ue)
|
||||
return;
|
||||
auto fce = ue.functionCallExpression;
|
||||
if (!fce)
|
||||
return;
|
||||
if (!isNoReturn)
|
||||
{
|
||||
string funcName = buildFullyQualifiedName(cast(string) funcDeclaration.ident.toString());
|
||||
nonVoidFuncs[funcName] = true;
|
||||
}
|
||||
}
|
||||
|
||||
auto identVisitor = scoped!IdentVisitor;
|
||||
if (fce.unaryExpression !is null)
|
||||
identVisitor.visit(fce.unaryExpression);
|
||||
else if (fce.type !is null)
|
||||
identVisitor.visit(fce.type);
|
||||
super.visit(funcDeclaration);
|
||||
}
|
||||
|
||||
if (!identVisitor.names.length)
|
||||
return;
|
||||
override void visit(AST.AliasDeclaration aliasDecl)
|
||||
{
|
||||
import std.algorithm : canFind, endsWith;
|
||||
import std.array : replace;
|
||||
|
||||
const(DSymbol)*[] symbols = resolveSymbol(sc, identVisitor.names);
|
||||
auto typeIdent = aliasDecl.type.isTypeIdentifier();
|
||||
if (typeIdent is null)
|
||||
return;
|
||||
|
||||
if (!symbols.length)
|
||||
return;
|
||||
string aliasName = cast(string) aliasDecl.ident.toString();
|
||||
string targetName = cast(string) typeIdent.ident.toString();
|
||||
|
||||
foreach (sym; symbols)
|
||||
{
|
||||
if (!sym)
|
||||
return;
|
||||
if (!sym.type)
|
||||
return;
|
||||
if (sym.kind != CompletionKind.functionName)
|
||||
return;
|
||||
if (sym.type is void_)
|
||||
return;
|
||||
if (noreturn_ && sym.type is noreturn_)
|
||||
return;
|
||||
}
|
||||
foreach(func; nonVoidFuncs.byKey())
|
||||
{
|
||||
if (func.endsWith(targetName) || func.canFind(targetName ~ "."))
|
||||
{
|
||||
string newAliasName = func.replace(targetName, aliasName);
|
||||
nonVoidFuncs[newAliasName] = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const(Token)[] tokens = fce.unaryExpression
|
||||
? fce.unaryExpression.tokens
|
||||
: fce.type
|
||||
? fce.type.tokens
|
||||
: fce.tokens;
|
||||
private extern (D) string buildFullyQualifiedName(string funcName)
|
||||
{
|
||||
import std.algorithm : fold;
|
||||
|
||||
addErrorMessage(tokens, KEY, MSG);
|
||||
}
|
||||
if (aggregateStack.length == 0)
|
||||
return funcName;
|
||||
|
||||
return aggregateStack.fold!((a, b) => a ~ "." ~ b) ~ "." ~ funcName;
|
||||
}
|
||||
|
||||
mixin VisitInstructionBlock!(AST.WhileStatement);
|
||||
mixin VisitInstructionBlock!(AST.ForStatement);
|
||||
mixin VisitInstructionBlock!(AST.DoStatement);
|
||||
mixin VisitInstructionBlock!(AST.ForeachRangeStatement);
|
||||
mixin VisitInstructionBlock!(AST.ForeachStatement);
|
||||
mixin VisitInstructionBlock!(AST.SwitchStatement);
|
||||
mixin VisitInstructionBlock!(AST.SynchronizedStatement);
|
||||
mixin VisitInstructionBlock!(AST.WithStatement);
|
||||
mixin VisitInstructionBlock!(AST.TryCatchStatement);
|
||||
mixin VisitInstructionBlock!(AST.TryFinallyStatement);
|
||||
|
||||
override void visit(AST.CompoundStatement compoundStatement)
|
||||
{
|
||||
foreach (statement; *compoundStatement.statements)
|
||||
{
|
||||
if (hasUnusedResult(statement))
|
||||
{
|
||||
auto lineNum = cast(ulong) statement.loc.linnum;
|
||||
auto charNum = cast(ulong) statement.loc.charnum;
|
||||
addErrorMessage(lineNum, charNum, KEY, MSG);
|
||||
}
|
||||
|
||||
statement.accept(this);
|
||||
}
|
||||
}
|
||||
|
||||
override void visit(AST.IfStatement ifStatement)
|
||||
{
|
||||
if (hasUnusedResult(ifStatement.ifbody))
|
||||
{
|
||||
auto lineNum = cast(ulong) ifStatement.ifbody.loc.linnum;
|
||||
auto charNum = cast(ulong) ifStatement.ifbody.loc.charnum;
|
||||
addErrorMessage(lineNum, charNum, KEY, MSG);
|
||||
}
|
||||
|
||||
if (ifStatement.elsebody && hasUnusedResult(ifStatement.elsebody))
|
||||
{
|
||||
auto lineNum = cast(ulong) ifStatement.elsebody.loc.linnum;
|
||||
auto charNum = cast(ulong) ifStatement.elsebody.loc.charnum;
|
||||
addErrorMessage(lineNum, charNum, KEY, MSG);
|
||||
}
|
||||
|
||||
super.visit(ifStatement);
|
||||
}
|
||||
|
||||
private template VisitInstructionBlock(NodeType)
|
||||
{
|
||||
override void visit(NodeType statement)
|
||||
{
|
||||
if (hasUnusedResult(statement._body))
|
||||
{
|
||||
auto lineNum = cast(ulong) statement._body.loc.linnum;
|
||||
auto charNum = cast(ulong) statement._body.loc.charnum;
|
||||
addErrorMessage(lineNum, charNum, KEY, MSG);
|
||||
}
|
||||
|
||||
super.visit(statement);
|
||||
}
|
||||
}
|
||||
|
||||
private bool hasUnusedResult(AST.Statement statement)
|
||||
{
|
||||
import dmd.astenums : TY;
|
||||
|
||||
auto exprStatement = statement.isExpStatement();
|
||||
if (exprStatement is null)
|
||||
return false;
|
||||
|
||||
auto callExpr = exprStatement.exp.isCallExp();
|
||||
if (callExpr is null)
|
||||
return false;
|
||||
|
||||
string funcName = "";
|
||||
|
||||
if (auto identExpr = callExpr.e1.isIdentifierExp())
|
||||
funcName = cast(string) identExpr.ident.toString();
|
||||
else if (auto dotIdExpr = callExpr.e1.isDotIdExp())
|
||||
funcName = buildFullyQualifiedCallName(dotIdExpr);
|
||||
|
||||
return (funcName in nonVoidFuncs) !is null;
|
||||
}
|
||||
|
||||
private extern (D) string buildFullyQualifiedCallName(AST.DotIdExp dotIdExpr)
|
||||
{
|
||||
import std.algorithm : fold, reverse;
|
||||
|
||||
string[] nameStack;
|
||||
nameStack ~= cast(string) dotIdExpr.ident.toString();
|
||||
|
||||
auto lastExpr = dotIdExpr.e1;
|
||||
while (lastExpr.isDotIdExp())
|
||||
{
|
||||
auto current = lastExpr.isDotIdExp();
|
||||
nameStack ~= cast(string) current.ident.toString();
|
||||
lastExpr = current.e1;
|
||||
}
|
||||
|
||||
if (auto identExpr = lastExpr.isIdentifierExp())
|
||||
nameStack ~= cast(string) identExpr.ident.toString();
|
||||
|
||||
return nameStack.reverse.fold!((a, b) => a ~ "." ~ b);
|
||||
}
|
||||
}
|
||||
|
||||
unittest
|
||||
{
|
||||
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
|
||||
import dscanner.analysis.helpers : assertAnalyzerWarnings;
|
||||
import std.stdio : stderr;
|
||||
import std.format : format;
|
||||
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
|
||||
import dscanner.analysis.helpers : assertAnalyzerWarningsDMD;
|
||||
import std.stdio : stderr;
|
||||
import std.format : format;
|
||||
|
||||
StaticAnalysisConfig sac = disabledConfig();
|
||||
sac.unused_result = Check.enabled;
|
||||
enum string MSG = "Function return value is discarded";
|
||||
StaticAnalysisConfig sac = disabledConfig();
|
||||
sac.unused_result = Check.enabled;
|
||||
|
||||
assertAnalyzerWarnings(q{
|
||||
void fun() {}
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
void fun() {}
|
||||
void main()
|
||||
{
|
||||
fun();
|
||||
}
|
||||
}c, sac);
|
||||
|
||||
assertAnalyzerWarnings(q{
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
alias noreturn = typeof(*null);
|
||||
noreturn fun() { while (1) {} }
|
||||
noreturn main()
|
||||
|
|
@ -131,16 +230,15 @@ unittest
|
|||
}
|
||||
}c, sac);
|
||||
|
||||
assertAnalyzerWarnings(q{
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
int fun() { return 1; }
|
||||
void main()
|
||||
{
|
||||
fun(); /+
|
||||
^^^ [warn]: %s +/
|
||||
fun(); // [warn]: %s
|
||||
}
|
||||
}c.format(UnusedResultChecker.MSG), sac);
|
||||
}c.format(MSG), sac);
|
||||
|
||||
assertAnalyzerWarnings(q{
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
struct Foo
|
||||
{
|
||||
static bool get()
|
||||
|
|
@ -151,12 +249,12 @@ unittest
|
|||
alias Bar = Foo;
|
||||
void main()
|
||||
{
|
||||
Bar.get(); /+
|
||||
^^^^^^^ [warn]: %s +/
|
||||
Bar.get(); // [warn]: %s
|
||||
Foo.bar.get();
|
||||
}
|
||||
}c.format(UnusedResultChecker.MSG), sac);
|
||||
}c.format(MSG), sac);
|
||||
|
||||
assertAnalyzerWarnings(q{
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
void main()
|
||||
{
|
||||
void fun() {}
|
||||
|
|
@ -164,17 +262,15 @@ unittest
|
|||
}
|
||||
}c, sac);
|
||||
|
||||
version (none) // TODO: local functions
|
||||
assertAnalyzerWarnings(q{
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
void main()
|
||||
{
|
||||
int fun() { return 1; }
|
||||
fun(); /+
|
||||
^^^ [warn]: %s +/
|
||||
fun(); // [warn]: %s
|
||||
}
|
||||
}c.format(UnusedResultChecker.MSG), sac);
|
||||
}c.format(MSG), sac);
|
||||
|
||||
assertAnalyzerWarnings(q{
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
int fun() { return 1; }
|
||||
void main()
|
||||
{
|
||||
|
|
@ -182,7 +278,7 @@ unittest
|
|||
}
|
||||
}c, sac);
|
||||
|
||||
assertAnalyzerWarnings(q{
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
void fun() { }
|
||||
alias gun = fun;
|
||||
void main()
|
||||
|
|
@ -191,7 +287,42 @@ unittest
|
|||
}
|
||||
}c, sac);
|
||||
|
||||
import std.stdio: writeln;
|
||||
writeln("Unittest for UnusedResultChecker passed");
|
||||
}
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
int fun() { return 1; }
|
||||
void main()
|
||||
{
|
||||
if (true)
|
||||
fun(); // [warn]: %s
|
||||
else
|
||||
fun(); // [warn]: %s
|
||||
}
|
||||
}c.format(MSG, MSG), sac);
|
||||
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
int fun() { return 1; }
|
||||
void main()
|
||||
{
|
||||
while (true)
|
||||
fun(); // [warn]: %s
|
||||
}
|
||||
}c.format(MSG), sac);
|
||||
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
int fun() { return 1; }
|
||||
alias gun = fun;
|
||||
void main()
|
||||
{
|
||||
gun(); // [warn]: %s
|
||||
}
|
||||
}c.format(MSG), sac);
|
||||
|
||||
assertAnalyzerWarningsDMD(q{
|
||||
void main()
|
||||
{
|
||||
void fun() {}
|
||||
fun();
|
||||
}
|
||||
}c, sac);
|
||||
|
||||
stderr.writeln("Unittest for UnusedResultChecker passed");
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue