Merge pull request #819 from CyberShadow/unused_result

Add unused_result check

Author: Hackerpilot

.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"

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;
 }

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,

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");
+}
+