diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index 5e46a8b9..7e48c6b3 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -917,10 +917,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)); - if (moduleName.shouldRun!CyclomaticComplexityCheck(analysisConfig)) checks ~= new CyclomaticComplexityCheck(args.setSkipTests( analysisConfig.cyclomatic_complexity == Check.skipTests && !ut), @@ -1350,6 +1346,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN config.redundant_storage_classes == Check.skipTests && !ut ); + if (moduleName.shouldRunDmd!(UnusedResultChecker!ASTCodegen)(config)) + visitors ~= new UnusedResultChecker!ASTCodegen( + fileName, + config.unused_result == Check.skipTests && !ut + ); + foreach (visitor; visitors) { m.accept(visitor); diff --git a/src/dscanner/analysis/unused_result.d b/src/dscanner/analysis/unused_result.d index 83e4c646..998aaaa9 100644 --- a/src/dscanner/analysis/unused_result.d +++ b/src/dscanner/analysis/unused_result.d @@ -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,174 +16,186 @@ 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; - - mixin AnalyzerInfo!"unused_result"; - -private: - - enum string KEY = "dscanner.unused_result"; - enum string MSG = "Function return value is discarded"; - -public: - - const(DSymbol)* void_; - const(DSymbol)* noreturn_; - - /// - 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(const(ExpressionStatement) decl) - { - import std.typecons : scoped; - - 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; - - auto identVisitor = scoped!IdentVisitor; - if (fce.unaryExpression !is null) - identVisitor.visit(fce.unaryExpression); - else if (fce.type !is null) - identVisitor.visit(fce.type); - - if (!identVisitor.names.length) - return; - - const(DSymbol)*[] symbols = resolveSymbol(sc, identVisitor.names); - - if (!symbols.length) - return; - - 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; - } - - const(Token)[] tokens = fce.unaryExpression - ? fce.unaryExpression.tokens - : fce.type - ? fce.type.tokens - : fce.tokens; - - addErrorMessage(tokens, KEY, MSG); - } + alias visit = BaseAnalyzerDmd.visit; + mixin AnalyzerInfo!"unused_result"; + private enum KEY = "dscanner.performance.enum_array_literal"; + private enum string MSG = "Function return value is discarded"; + + extern (D) this(string fileName, bool skipTests = false) + { + super(fileName, skipTests); + } + + 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 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 || callExpr.f is null) + return false; + + auto typeFunction = callExpr.f.type.isTypeFunction(); + if (typeFunction is null || typeFunction.next.ty == TY.Tvoid) + return false; + + return true; + } + + private template VisitInstructionBlock(T) + { + override void visit(T 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); + } + } } 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{ + int fun() { return 1; } void main() { - fun(); + fun(); // [warn]: %s } - }c, sac); + }c.format(MSG), sac, 1); - assertAnalyzerWarnings(q{ - alias noreturn = typeof(*null); - noreturn fun() { while (1) {} } - noreturn main() + assertAnalyzerWarningsDMD(q{ + int fun() { return 1; } + void main() { - fun(); + if (true) + fun(); // [warn]: %s + else + fun(); // [warn]: %s } - }c, sac); + }c.format(MSG, MSG), sac, 1); - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ int fun() { return 1; } void main() { - fun(); /+ - ^^^ [warn]: %s +/ + while (true) + fun(); // [warn]: %s } - }c.format(UnusedResultChecker.MSG), sac); + }c.format(MSG), sac, 1); - assertAnalyzerWarnings(q{ - struct Foo + assertAnalyzerWarningsDMD(q{ + int fun() { return 1; } + alias gun = fun; + void main() { - static bool get() - { - return false; - } + gun(); // [warn]: %s } - alias Bar = Foo; + }c.format(MSG), sac, 1); + + assertAnalyzerWarningsDMD(q{ void main() { - Bar.get(); /+ - ^^^^^^^ [warn]: %s +/ + int fun() { return 1; } + fun(); // [warn]: %s } - }c.format(UnusedResultChecker.MSG), sac); + }c.format(MSG), sac, 1); - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ + void fun() {} void main() { - void fun() {} fun(); } - }c, sac); + }c, sac, 1); - version (none) // TODO: local functions - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ void main() { - int fun() { return 1; } - fun(); /+ - ^^^ [warn]: %s +/ + void fun() {} + fun(); } - }c.format(UnusedResultChecker.MSG), sac); + }c, sac, 1); - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ int fun() { return 1; } void main() { cast(void) fun(); } - }c, sac); + }c, sac, 1); - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ void fun() { } alias gun = fun; void main() { gun(); } - }c, sac); + }c, sac, 1); - import std.stdio: writeln; - writeln("Unittest for UnusedResultChecker passed"); + stderr.writeln("Unittest for UnusedResultChecker passed"); } -