From e61ce458564d89068328c4375dd34185869cc927 Mon Sep 17 00:00:00 2001 From: Vladimir Panteleev Date: Fri, 21 Aug 2020 09:04:16 +0000 Subject: [PATCH] Add unused_result check --- .dscanner.ini | 2 + src/dscanner/analysis/config.d | 3 + src/dscanner/analysis/run.d | 5 + src/dscanner/analysis/unused_result.d | 157 ++++++++++++++++++++++++++ 4 files changed, 167 insertions(+) create mode 100644 src/dscanner/analysis/unused_result.d diff --git a/.dscanner.ini b/.dscanner.ini index ab5a27b..0b8ac4f 100644 --- a/.dscanner.ini +++ b/.dscanner.ini @@ -89,3 +89,5 @@ useless_initializer="disabled" allman_braces_check="disabled" ; Check for redundant attributes redundant_attributes_check="enabled" +; Check for unused function return values +unused_result="enabled" diff --git a/src/dscanner/analysis/config.d b/src/dscanner/analysis/config.d index f2f377c..c917e66 100644 --- a/src/dscanner/analysis/config.d +++ b/src/dscanner/analysis/config.d @@ -203,6 +203,9 @@ struct StaticAnalysisConfig @INI("Check for redundant storage classes on variable declarations") string redundant_storage_classes = Check.enabled; + @INI("Check for unused function return values") + string unused_result = Check.enabled; + @INI("Module-specific filters") ModuleFilters filters; } diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index a8d9ec6..83cd7ba 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -79,6 +79,7 @@ import dscanner.analysis.assert_without_msg; import dscanner.analysis.if_constraints_indent; import dscanner.analysis.trust_too_much; import dscanner.analysis.redundant_storage_class; +import dscanner.analysis.unused_result; import dsymbol.string_interning : internString; import dsymbol.scope_; @@ -583,6 +584,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a checks ~= new RedundantStorageClassCheck(fileName, analysisConfig.redundant_storage_classes == Check.skipTests && !ut); + if (moduleName.shouldRun!UnusedResultChecker(analysisConfig)) + checks ~= new UnusedResultChecker(fileName, moduleScope, + analysisConfig.unused_result == Check.skipTests && !ut); + version (none) if (moduleName.shouldRun!IfStatementCheck(analysisConfig)) checks ~= new IfStatementCheck(fileName, moduleScope, diff --git a/src/dscanner/analysis/unused_result.d b/src/dscanner/analysis/unused_result.d new file mode 100644 index 0000000..b1e987b --- /dev/null +++ b/src/dscanner/analysis/unused_result.d @@ -0,0 +1,157 @@ +// Copyright Vladimir Panteleev 2020 +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) +module dscanner.analysis.unused_result; + +import dscanner.analysis.base; +import dscanner.analysis.mismatched_args : resolveSymbol, IdentVisitor; +import dscanner.utils; +import dsymbol.scope_; +import dsymbol.symbol; +import dparse.ast, dparse.lexer; +import std.algorithm.searching : canFind; +import std.range: retro; + +/** + * Checks for function call statements which call non-void functions. + * + * In case the function returns a value indicating success/failure, + * ignoring this return value and continuing execution can lead to + * undesired results. + * + * When the return value is intentionally discarded, `cast(void)` can + * be prepended to silence the check. + */ +final class UnusedResultChecker : BaseAnalyzer +{ + 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_; + + /// + this(string fileName, const(Scope)* sc, bool skipTests = false) + { + super(fileName, sc, skipTests); + void_ = sc.getSymbolsByName(internString("void"))[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; + } + + addErrorMessage(decl.expression.line, decl.expression.column, KEY, MSG); + } +} + +unittest +{ + import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; + import dscanner.analysis.helpers : assertAnalyzerWarnings; + import std.stdio : stderr; + import std.format : format; + + StaticAnalysisConfig sac = disabledConfig(); + sac.unused_result = Check.enabled; + + assertAnalyzerWarnings(q{ + void fun() {} + void main() + { + fun(); + } + }c, sac); + + assertAnalyzerWarnings(q{ + int fun() { return 1; } + void main() + { + fun(); // [warn]: %s + } + }c.format(UnusedResultChecker.MSG), sac); + + assertAnalyzerWarnings(q{ + void main() + { + void fun() {} + fun(); + } + }c, sac); + + version (none) // TODO: local functions + assertAnalyzerWarnings(q{ + void main() + { + int fun() { return 1; } + fun(); // [warn]: %s + } + }c.format(UnusedResultChecker.MSG), sac); + + assertAnalyzerWarnings(q{ + int fun() { return 1; } + void main() + { + cast(void) fun(); + } + }c, sac); + + assertAnalyzerWarnings(q{ + void fun() { } + alias gun = fun; + void main() + { + gun(); + } + }c, sac); + + import std.stdio: writeln; + writeln("Unittest for UnusedResultChecker passed"); +} +