Merge pull request #797 from MoonlightSentinel/unused

Fix issue #794: False positive for parameter used in __traits(...)

src/dscanner/analysis/unused.d

@@ -393,3 +393,65 @@ private:
 
 	bool blockStatementIntroducesScope = true;
 }
+
+/// Base class for unused parameter/variables checks
+abstract class UnusedStorageCheck : UnusedIdentifierCheck
+{
+	alias visit = UnusedIdentifierCheck.visit;
+
+	/**
+	Ignore declarations which are allowed to be unused, e.g. inside of a
+	speculative compilation: __traits(compiles, { S s = 0; })
+	**/
+	uint ignoreDeclarations = 0;
+
+	/// Kind of declaration for error messages e.g. "Variable"
+	const string publicType;
+
+	/// Kind of declaration for error reports e.g. "unused_variable"
+	const string reportType;
+
+	/**
+	 * Params:
+	 *      fileName	= the name of the file being analyzed
+	 *		sc			= the scope
+	 *		skipTest	= whether tests should be analyzed
+	 *		publicType	= declaration kind used in error messages, e.g. "Variable"s
+	 *		reportType	= declaration kind used in error reports, e.g. "unused_variable"
+	 */
+	this(string fileName, const(Scope)* sc, bool skipTests = false, string publicType = null, string reportType = null)
+	{
+		super(fileName, sc, skipTests);
+		this.publicType = publicType;
+		this.reportType = reportType;
+	}
+
+	override void visit(const TraitsExpression traitsExp)
+	{
+		// issue #788: Enum values might be used inside of `__traits` expressions, e.g.:
+		// enum name = "abc";
+		// __traits(hasMember, S, name);
+		ignoreDeclarations++;
+		traitsExp.templateArgumentList.accept(this);
+		ignoreDeclarations--;
+	}
+
+	override final protected void popScope()
+	{
+		if (!ignoreDeclarations)
+		{
+			foreach (uu; tree[$ - 1])
+			{
+				if (!uu.isRef && tree.length > 1)
+				{
+					if (uu.uncertain)
+						continue;
+					immutable string errorMessage = publicType ~ ' ' ~ uu.name ~ " is never used.";
+					addErrorMessage(uu.line, uu.column,
+							"dscanner.suspicious." ~ reportType, errorMessage);
+				}
+			}
+		}
+		tree = tree[0 .. $ - 1];
+	}
+}

src/dscanner/analysis/unused_parameter.d

@@ -13,9 +13,9 @@ import dsymbol.scope_ : Scope;
 /**
  * Checks for unused variables.
  */
-final class UnusedParameterCheck : UnusedIdentifierCheck
+final class UnusedParameterCheck : UnusedStorageCheck
 {
-	alias visit = UnusedIdentifierCheck.visit;
+	alias visit = UnusedStorageCheck.visit;
 
 	mixin AnalyzerInfo!"unused_parameter_check";
 
@@ -25,7 +25,7 @@ final class UnusedParameterCheck : UnusedIdentifierCheck
 	 */
 	this(string fileName, const(Scope)* sc, bool skipTests = false)
 	{
-		super(fileName, sc, skipTests);
+		super(fileName, sc, skipTests, "Parameter", "unused_parameter");
 	}
 
 	override void visit(const Parameter parameter)
@@ -52,22 +52,6 @@ final class UnusedParameterCheck : UnusedIdentifierCheck
 			}
 		}
 	}
-
-	override protected void popScope()
-	{
-		foreach (uu; tree[$ - 1])
-		{
-			if (!uu.isRef && tree.length > 1)
-			{
-			 	if (uu.uncertain)
-					continue;
-				immutable string errorMessage = "Parameter " ~ uu.name ~ " is never used.";
-				addErrorMessage(uu.line, uu.column,
-						"dscanner.suspicious.unused_parameter", errorMessage);
-			}
-		}
-		tree = tree[0 .. $ - 1];
-	}
 }
 
 @system unittest
@@ -114,12 +98,23 @@ final class UnusedParameterCheck : UnusedIdentifierCheck
 		auto cb2 = delegate(size_t a) {}; // [warn]: Parameter a is never used.
 		cb2(3);
 	}
-	
+
 	bool hasDittos(int decl)
 	{
 		mixin("decl++;");
 	}
 
+	// https://github.com/dlang-community/D-Scanner/issues/794
+	void traits()
+	{
+		struct S { int i; }
+
+		static foo(S s)
+		{
+			__traits(getMember, s, "i") = 99;
+		}
+	}
+
 	}c, sac);
 	stderr.writeln("Unittest for UnusedParameterCheck passed.");
 }

src/dscanner/analysis/unused_variable.d

@@ -13,25 +13,19 @@ import std.algorithm.iteration : map;
 /**
  * Checks for unused variables.
  */
-final class UnusedVariableCheck : UnusedIdentifierCheck
+final class UnusedVariableCheck : UnusedStorageCheck
 {
-	alias visit = UnusedIdentifierCheck.visit;
+	alias visit = UnusedStorageCheck.visit;
 
 	mixin AnalyzerInfo!"unused_variable_check";
 
-	/**
-	Ignore declarations which are allowed to be unused, e.g. inside of a
-	speculative compilation: __traits(compiles, { S s = 0; })
-	**/
-	uint ignoreDeclarations = 0;
-
 	/**
 	 * Params:
 	 *     fileName = the name of the file being analyzed
 	 */
 	this(string fileName, const(Scope)* sc, bool skipTests = false)
 	{
-		super(fileName, sc, skipTests);
+		super(fileName, sc, skipTests, "Variable", "unused_variable");
 	}
 
 	override void visit(const VariableDeclaration variableDeclaration)
@@ -47,35 +41,6 @@ final class UnusedVariableCheck : UnusedIdentifierCheck
 			this.variableDeclared(t.text, t.line, t.column, false);
 		autoDeclaration.accept(this);
 	}
-
-	override void visit(const TraitsExpression traitsExp)
-	{
-		// issue #788: Enum values might be used inside of `__traits` expressions, e.g.:
-		// enum name = "abc";
-		// __traits(hasMember, S, name);
-		ignoreDeclarations++;
-		traitsExp.templateArgumentList.accept(this);
-		ignoreDeclarations--;
-	}
-
-	override protected void popScope()
-	{
-		if (!ignoreDeclarations)
-		{
-			foreach (uu; tree[$ - 1])
-			{
-				if (!uu.isRef && tree.length > 1)
-				{
-					if (uu.uncertain)
-						continue;
-					immutable string errorMessage = "Variable " ~ uu.name ~ " is never used.";
-					addErrorMessage(uu.line, uu.column,
-							"dscanner.suspicious.unused_variable", errorMessage);
-				}
-			}
-		}
-		tree = tree[0 .. $ - 1];
-	}
 }
 
 @system unittest