Replace libdparse with DMD in UnusedParameterCheck (#116)

* Replace libdparse with DMD in UnusedParameterCheck

* Add workaround for gdc-12 compilation

src/dscanner/analysis/run.d

@@ -849,10 +849,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
 		checks ~= new UnusedVariableCheck(args.setSkipTests(
 		analysisConfig.unused_variable_check == Check.skipTests && !ut));
 
-	if (moduleName.shouldRun!UnusedParameterCheck(analysisConfig))
-		checks ~= new UnusedParameterCheck(args.setSkipTests(
-		analysisConfig.unused_parameter_check == Check.skipTests && !ut));
-
 	if (moduleName.shouldRun!LineLengthCheck(analysisConfig))
 		checks ~= new LineLengthCheck(args.setSkipTests(
 		analysisConfig.long_line_check == Check.skipTests && !ut),
@@ -1350,6 +1346,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN
 			config.auto_function_check == Check.skipTests && !ut
 		);
 
+	if (moduleName.shouldRunDmd!(UnusedParameterCheck!ASTCodegen)(config))
+		visitors ~= new UnusedParameterCheck!ASTCodegen(
+			fileName,
+			config.unused_parameter_check == Check.skipTests && !ut
+		);
+
 	foreach (visitor; visitors)
 	{
 		m.accept(visitor);

src/dscanner/analysis/unused_parameter.d

@@ -4,64 +4,172 @@
 //          http://www.boost.org/LICENSE_1_0.txt)
 module dscanner.analysis.unused_parameter;
 
-import dparse.ast;
-import dparse.lexer;
 import dscanner.analysis.base;
-import dscanner.analysis.unused;
+import dmd.astenums : STC;
-import dsymbol.scope_ : Scope;
+import std.algorithm : all, canFind, each, filter, map;
+import std.conv : to;
 
 /**
- * Checks for unused variables.
+ * Checks for unused function parameters.
  */
-final class UnusedParameterCheck : UnusedStorageCheck
+extern (C++) class UnusedParameterCheck(AST) : BaseAnalyzerDmd
 {
-	alias visit = UnusedStorageCheck.visit;
+	alias visit = BaseAnalyzerDmd.visit;
-
 	mixin AnalyzerInfo!"unused_parameter_check";
 
-	/**
+	private enum KEY = "dscanner.suspicious.unused_parameter";
-	 * Params:
+	private enum MSG = "Parameter %s is never used.";
-	 *     fileName = the name of the file being analyzed
+
-	 */
+	private static struct ParamInfo
-	this(BaseAnalyzerArguments args)
 	{
-		super(args, "Parameter", "unused_parameter");
+		string name;
+		ulong lineNum;
+		ulong charNum;
+		bool isUsed = false;
 	}
 
-	override void visit(const Parameter parameter)
+	private alias ParamSet = ParamInfo[string];
+	private ParamSet[] usedParams;
+	private bool inMixin;
+
+	extern (D) this(string fileName, bool skipTests = false)
 	{
-		import std.algorithm : among;
+		super(fileName, skipTests);
-		import std.algorithm.iteration : filter;
+		pushScope();
-		import std.range : empty;
+	}
 
-		if (parameter.name != tok!"")
+	override void visit(AST.FuncDeclaration funcDeclaration)
+	{
+		import std.format : format;
+
+		pushScope();
+		super.visit(funcDeclaration);
+
+		bool shouldIgnoreWarns = funcDeclaration.fbody is null || funcDeclaration.storage_class & STC.override_;
+		if (!shouldIgnoreWarns)
+			currentScope.byValue
+				.filter!(param => !param.isUsed)
+				.each!(param => addErrorMessage(param.lineNum, param.charNum, KEY, MSG.format(param.name)));
+
+		popScope();
+	}
+
+	override void visit(AST.Parameter parameter)
+	{
+		import dmd.astenums : TY;
+
+		if (parameter.ident is null)
+			return;
+
+		auto varName = cast(string) parameter.ident.toString();
+		bool shouldBeIgnored = varName.all!(c => c == '_') || parameter.storageClass & STC.ref_
+			|| parameter.storageClass & STC.out_ || parameter.type.ty == TY.Tpointer;
+		if (!shouldBeIgnored)
+			currentScope[varName] = ParamInfo(varName, parameter.loc.linnum, parameter.loc.charnum);
+	}
+
+	override void visit(AST.TypeSArray newExp)
+	{
+		if (auto identifierExpression = newExp.dim.isIdentifierExp())
+			identifierExpression.accept(this);
+	}
+
+	override void visit(AST.IdentifierExp identifierExp)
+	{
+		if (identifierExp.ident)
+			markAsUsed(cast(string) identifierExp.ident.toString());
+
+		super.visit(identifierExp);
+	}
+
+	mixin VisitMixin!(AST.MixinExp);
+	mixin VisitMixin!(AST.MixinStatement);
+
+	private template VisitMixin(NodeType)
+	{
+		override void visit(NodeType node)
 		{
-			immutable bool isRef = !parameter.parameterAttributes
+			inMixin = true;
-				.filter!(a => a.idType.among(tok!"ref", tok!"out")).empty;
+			super.visit(node);
-			immutable bool isPtr = parameter.type && !parameter.type
+			inMixin = false;
-				.typeSuffixes.filter!(a => a.star != tok!"").empty;
+		}
+	}
 
-			variableDeclared(parameter.name.text, parameter.name, isRef | isPtr);
+	override void visit(AST.StringExp stringExp)
+	{
+		if (!inMixin)
+			return;
 
-			if (parameter.default_ !is null)
+		string str = cast(string) stringExp.toStringz();
+		currentScope.byKey
+			.filter!(param => canFind(str, param))
+			.each!(param => markAsUsed(param));
+	}
+
+	override void visit(AST.TraitsExp traitsExp)
+	{
+		import dmd.dtemplate : isType;
+
+		super.visit(traitsExp);
+
+		if (traitsExp.args is null)
+			return;
+
+		(*traitsExp.args).opSlice()
+			.map!(arg => isType(arg))
+			.filter!(type => type !is null)
+			.map!(type => type.isTypeIdentifier())
+			.filter!(typeIdentifier => typeIdentifier !is null)
+			.each!(typeIdentifier => markAsUsed(cast(string) typeIdentifier.toString()));
+	}
+
+	private extern (D) void markAsUsed(string varName)
+	{
+		import std.range : retro;
+
+		foreach (funcScope; usedParams.retro())
+		{
+			if (varName in funcScope)
 			{
-				interestDepth++;
+				funcScope[varName].isUsed = true;
-				parameter.default_.accept(this);
+				break;
-				interestDepth--;
 			}
 		}
 	}
+
+	@property private extern (D) ParamSet currentScope()
+	{
+		return usedParams[$ - 1];
+	}
+
+	private void pushScope()
+	{
+		// Error with gdc-12
+		//usedParams ~= new ParamSet;
+
+		// Workaround for gdc-12
+		ParamSet newScope;
+		newScope["test"] = ParamInfo("test", 0, 0);
+		usedParams ~= newScope;
+		currentScope.remove("test");
+	}
+
+	private void popScope()
+	{
+		usedParams.length--;
+	}
 }
 
 @system unittest
 {
 	import std.stdio : stderr;
 	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
-	import dscanner.analysis.helpers : assertAnalyzerWarnings;
+	import dscanner.analysis.helpers : assertAnalyzerWarningsDMD;
 
 	StaticAnalysisConfig sac = disabledConfig();
 	sac.unused_parameter_check = Check.enabled;
-	assertAnalyzerWarnings(q{
+
+	assertAnalyzerWarningsDMD(q{
 
 	// bug encountered after correct DIP 1009 impl in dparse
 	version (StdDdoc)
@@ -71,11 +179,9 @@ final class UnusedParameterCheck : UnusedStorageCheck
 			is(StringTypeOf!R));
 	}
 
-	void inPSC(in int a){} /+
+	void inPSC(in int a){} // [warn]: Parameter a is never used.
-	                  ^ [warn]: Parameter a is never used. +/
 
-	void doStuff(int a, int b) /+
+	void doStuff(int a, int b) // [warn]: Parameter b is never used.
-	                        ^ [warn]: Parameter b is never used. +/
 	{
 		return a;
 	}
@@ -96,8 +202,7 @@ final class UnusedParameterCheck : UnusedStorageCheck
 	{
 		auto cb1 = delegate(size_t _) {};
 		cb1(3);
-		auto cb2 = delegate(size_t a) {}; /+
+		auto cb2 = delegate(size_t a) {}; // [warn]: Parameter a is never used.
-		                           ^ [warn]: Parameter a is never used. +/
 		cb2(3);
 	}
 
@@ -118,5 +223,68 @@ final class UnusedParameterCheck : UnusedStorageCheck
 	}
 
 	}c, sac);
+
+	assertAnalyzerWarningsDMD(q{
+		void testMixinExpression(const Declaration decl)
+		{
+			foreach (property; possibleDeclarations)
+				if (auto fn = mixin("decl." ~ property))
+					addMessage(fn.name.type ? [fn.name] : fn.tokens, fn.name.text);
+		}
+	}c, sac);
+
+	assertAnalyzerWarningsDMD(q{
+		void testNestedFunction(int a)
+		{
+			int nestedFunction(int b)
+			{
+				return a + b;
+			}
+
+			nestedFunction(5);
+		}
+
+		void testNestedFunctionShadowing(int a) // [warn]: Parameter a is never used.
+		{
+			int nestedFunctionShadowing(int a)
+			{
+				return a + 5;
+			}
+
+			nestedFunction(5);
+		}
+	}c, sac);
+
+	assertAnalyzerWarningsDMD(q{
+		override protected void testOverrideFunction(int a)
+		{
+			return;
+		}
+	}c, sac);
+
+	assertAnalyzerWarningsDMD(q{
+		void testRefParam(ref LogEntry payload)
+		{
+			return;
+		}
+
+		void testOutParam(out LogEntry payload)
+		{
+			return;
+		}
+
+		void testPointerParam(LogEntry* payload)
+		{
+			return;
+		}
+	}c, sac);
+
+	assertAnalyzerWarningsDMD(q{
+		private char[] testStaticArray(size_t size) @safe pure nothrow
+		{
+    		return new char[size];
+		}
+	}c, sac);
+
 	stderr.writeln("Unittest for UnusedParameterCheck passed.");
 }