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

src/dscanner/analysis/run.d

@@ -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);

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,97 +16,204 @@ 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";
 
-private:
+	private enum KEY = "dscanner.performance.enum_array_literal";
+	private enum string MSG = "Function return value is discarded";
 
-    enum string KEY = "dscanner.unused_result";
+	private bool[string] nonVoidFuncs;
-    enum string MSG = "Function return value is discarded";
+	private string[] aggregateStack;
 
-public:
+	extern (D) this(string fileName, bool skipTests = false)
-
-    const(DSymbol)* void_;
-    const(DSymbol)* noreturn_;
-
-    ///
-    this(BaseAnalyzerArguments args)
 	{
-        super(args);
+		super(fileName, skipTests);
-        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)
+	private template VisitAggregate(NodeType)
 	{
-        import std.typecons : scoped;
+		override void visit(NodeType aggregate)
-
-        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)
+			string name = cast(string) aggregate.ident.toString();
-                return;
+			aggregateStack ~= name;
-            if (!sym.type)
+			super.visit(aggregate);
-                return;
+			aggregateStack.length -= 1;
-            if (sym.kind != CompletionKind.functionName)
+		}
-                return;
-            if (sym.type is void_)
-                return;
-            if (noreturn_ && sym.type is noreturn_)
-                return;
 	}
 
-        const(Token)[] tokens = fce.unaryExpression
+	mixin VisitAggregate!(AST.StructDeclaration);
-            ? fce.unaryExpression.tokens
+	mixin VisitAggregate!(AST.ClassDeclaration);
-            : fce.type
-            ? fce.type.tokens
-            : fce.tokens;
 
-        addErrorMessage(tokens, KEY, MSG);
+	override void visit(AST.FuncDeclaration funcDeclaration)
+	{
+		import dmd.astenums : TY;
+
+		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";
+
+			if (!isNoReturn)
+			{
+				string funcName = buildFullyQualifiedName(cast(string) funcDeclaration.ident.toString());
+				nonVoidFuncs[funcName] = true;
+			}
+		}
+
+		super.visit(funcDeclaration);
+	}
+
+	override void visit(AST.AliasDeclaration aliasDecl)
+	{
+		import std.algorithm : canFind, endsWith;
+		import std.array : replace;
+
+		auto typeIdent = aliasDecl.type.isTypeIdentifier();
+		if (typeIdent is null)
+			return;
+
+		string aliasName = cast(string) aliasDecl.ident.toString();
+		string targetName = cast(string) typeIdent.ident.toString();
+
+		foreach(func; nonVoidFuncs.byKey())
+		{
+			if (func.endsWith(targetName) || func.canFind(targetName ~ "."))
+			{
+				string newAliasName = func.replace(targetName, aliasName);
+				nonVoidFuncs[newAliasName] = true;
+			}
+		}
+	}
+
+	private extern (D) string buildFullyQualifiedName(string funcName)
+	{
+		import std.algorithm : fold;
+
+		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 dscanner.analysis.helpers : assertAnalyzerWarningsDMD;
 	import std.stdio : stderr;
 	import std.format : format;
 
+	enum string MSG = "Function return value is discarded";
 	StaticAnalysisConfig sac = disabledConfig();
 	sac.unused_result = Check.enabled;
 
-    assertAnalyzerWarnings(q{
+	assertAnalyzerWarningsDMD(q{
 		void fun() {}
         void main()
         {
@@ -122,7 +221,7 @@ unittest
         }
     }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(); /+
+            fun(); // [warn]: %s
-            ^^^ [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(); /+
+            Bar.get(); // [warn]: %s
-            ^^^^^^^ [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
+	assertAnalyzerWarningsDMD(q{
-    assertAnalyzerWarnings(q{
         void main()
         {
             int fun() { return 1; }
-            fun(); /+
+            fun(); // [warn]: %s
-            ^^^ [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;
+	assertAnalyzerWarningsDMD(q{
-    writeln("Unittest for UnusedResultChecker passed");
+        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");
+}