Add possible immutable variable detector

src/analysis/config.d

@@ -80,4 +80,7 @@ struct StaticAnalysisConfig
 
 	@INI("Checks for local imports that are too broad")
 	bool local_import_check;
+
+	@INI("Checks for variables that could be declared immutable")
+	bool could_be_immutable_check = false; // disabled by default for now
 }

src/analysis/immutable_finder.d

@@ -0,0 +1,212 @@
+//          Copyright Brian Schott (Hackerpilot) 2015.
+// 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 analysis.immutable_finder;
+
+import std.container;
+import std.d.ast;
+import std.d.lexer;
+import analysis.base;
+
+/**
+ * Checks for variables that could have been declared immutable
+ */
+class ImmutableFinder:BaseAnalyzer
+{
+    alias visit = BaseAnalyzer.visit;
+
+	///
+	this(string fileName)
+    {
+        super(fileName);
+    }
+
+	override void visit(const Module mod)
+	{
+		pushScope();
+		mod.accept(this);
+		popScope();
+	}
+
+	override void visit(const BlockStatement blockStatement)
+	{
+		pushScope();
+		blockStatementDepth++;
+		blockStatement.accept(this);
+		blockStatementDepth--;
+		popScope();
+	}
+
+	override void visit(const StructBody structBody)
+	{
+		pushScope();
+		auto oldBlockStatementDepth = blockStatementDepth;
+		blockStatementDepth = 0;
+		structBody.accept(this);
+		blockStatementDepth = oldBlockStatementDepth;
+		popScope();
+	}
+
+	override void visit(const VariableDeclaration dec)
+	{
+		if (dec.autoDeclaration is null && blockStatementDepth > 0 && isImmutable <= 0 && !canFindImmutable(dec))
+		{
+			foreach (d; dec.declarators)
+			{
+				tree[$ - 1].insert(new VariableInfo(d.name.text, d.name.line,
+					d.name.column));
+			}
+		}
+		dec.accept(this);
+	}
+
+	override void visit(const AutoDeclaration autoDeclaration)
+	{
+		if (blockStatementDepth > 0 && isImmutable <= 0
+			&& (autoDeclaration.storageClass !is null
+				&& autoDeclaration.storageClass.token != tok!"immutable"))
+		{
+			foreach (id; autoDeclaration.identifiers)
+				tree[$ - 1].insert(new VariableInfo(id.text, id.line,
+					id.column));
+		}
+		autoDeclaration.accept(this);
+	}
+
+	override void visit(const AssignExpression assignExpression)
+	{
+		if (assignExpression.operator != tok!"")
+		{
+			interest++;
+			assignExpression.ternaryExpression.accept(this);
+			interest--;
+			assignExpression.ternaryExpression.accept(this);
+		}
+		else
+			assignExpression.accept(this);
+	}
+
+	override void visit(const Declaration dec)
+	{
+		if (canFindImmutable(dec))
+		{
+			isImmutable++;
+			dec.accept(this);
+			isImmutable--;
+		}
+		else
+			dec.accept(this);
+	}
+
+	override void visit(const IdentifierOrTemplateInstance ioti)
+	{
+//		import std.stdio : stderr;
+//		stderr.writeln(ioti.identifier.text, " ", ioti.identifier.line);
+		if (ioti.identifier != tok!"" && interest > 0)
+		{
+			variableMightBeModified(ioti.identifier.text);
+		}
+		ioti.accept(this);
+	}
+
+	mixin PartsMightModify!IndexExpression;
+	mixin PartsMightModify!SliceExpression;
+	mixin PartsMightModify!FunctionCallExpression;
+	mixin PartsMightModify!IdentifierOrTemplateChain;
+	mixin PartsMightModify!ReturnStatement;
+
+	override void visit(const UnaryExpression unary)
+	{
+		if (unary.prefix == tok!"++" || unary.prefix == tok!"--"
+			|| unary.suffix == tok!"++" || unary.suffix == tok!"--")
+		{
+			interest++;
+			unary.accept(this);
+			interest--;
+		}
+		else
+			unary.accept(this);
+	}
+
+private:
+
+	template PartsMightModify(T)
+	{
+		override void visit(const T t)
+		{
+			interest++;
+			t.accept(this);
+			interest--;
+		}
+	}
+
+	void variableMightBeModified(string name)
+	{
+//		import std.stdio : stderr;
+//		stderr.writeln("Marking ", name, " as possibly modified");
+		size_t index = tree.length - 1;
+		auto vi = VariableInfo(name);
+		while (true)
+		{
+			if (tree[index].removeKey(&vi) != 0 || index == 0)
+				break;
+			index--;
+		}
+	}
+
+	bool canFindImmutable(const Declaration dec)
+	{
+		import std.algorithm : canFind, map;
+		return dec.attributes.map!(a => a.attribute).canFind(cast(IdType) tok!"immutable");
+	}
+
+	bool canFindImmutable(const VariableDeclaration dec)
+	{
+		import std.algorithm : canFind;
+		foreach (attr; dec.attributes)
+		{
+			if (attr.attribute.type == tok!"immutable")
+				return true;
+		}
+		if (dec.type !is null)
+		{
+			if (dec.type.typeConstructors.canFind(cast(IdType) tok!"immutable"))
+				return true;
+		}
+		return false;
+	}
+
+	static struct VariableInfo
+	{
+		string name;
+		size_t line;
+		size_t column;
+	}
+
+	void popScope()
+	{
+		foreach (vi; tree[$ - 1])
+		{
+			immutable string errorMessage = "Variable " ~ vi.name
+				~ " could have been declared immutable";
+			addErrorMessage(vi.line, vi.column, "dscanner.suspicious.could_be_immutable",
+				errorMessage);
+		}
+		tree = tree[0 .. $ - 1];
+	}
+
+	void pushScope()
+	{
+		tree ~= new RedBlackTree!(VariableInfo*, "a.name < b.name");
+	}
+
+	int blockStatementDepth;
+
+	int interest;
+
+	int isImmutable;
+
+	RedBlackTree!(VariableInfo*, "a.name < b.name")[] tree;
+}
+

src/analysis/run.d

@@ -39,6 +39,7 @@ import analysis.undocumented;
 import analysis.comma_expression;
 import analysis.function_attributes;
 import analysis.local_imports;
+import analysis.immutable_finder;
 
 bool first = true;
 
@@ -185,6 +186,7 @@ MessageSet analyze(string fileName, const Module m,
 	if (analysisConfig.function_attribute_check) checks ~= new FunctionAttributeCheck(fileName);
 	if (analysisConfig.comma_expression_check) checks ~= new CommaExpressionCheck(fileName);
 	if (analysisConfig.local_import_check) checks ~= new LocalImportCheck(fileName);
+	if (analysisConfig.could_be_immutable_check) checks ~= new ImmutableFinder(fileName);
 
 	foreach (check; checks)
 	{

src/analysis/unused.d

@@ -224,7 +224,7 @@ class UnusedVariableCheck : BaseAnalyzer
 			{
 				foreach (part; matchAll(primary.primary.text, re))
 				{
-					size_t treeIndex = tree.length - 1;
+					immutable size_t treeIndex = tree.length - 1;
 					auto uu = UnUsed(part.hit);
 					auto r = tree[treeIndex].equalRange(&uu);
 					if (!r.empty)
@@ -247,7 +247,7 @@ class UnusedVariableCheck : BaseAnalyzer
 
 	override void visit(const BlockStatement blockStatement)
 	{
-		bool sb = inAggregateScope;
+		immutable bool sb = inAggregateScope;
 		inAggregateScope = false;
 		if (blockStatementIntroducesScope)
 			pushScope();
@@ -281,12 +281,12 @@ class UnusedVariableCheck : BaseAnalyzer
 
 	override void visit(const Parameter parameter)
 	{
-		import std.algorithm;
+		import std.algorithm : canFind;
-		import std.array;
+		import std.array : array;
 		if (parameter.name != tok!"")
 		{
 //			stderr.writeln("Adding parameter ", parameter.name.text);
-			bool isRef = canFind(parameter.parameterAttributes, cast(IdType) tok!"ref")
+			immutable bool isRef = canFind(parameter.parameterAttributes, cast(IdType) tok!"ref")
 				|| canFind(parameter.parameterAttributes, cast(IdType) tok!"in")
 				|| canFind(parameter.parameterAttributes, cast(IdType) tok!"out");
 			variableDeclared(parameter.name.text, parameter.name.line,
@@ -302,7 +302,7 @@ class UnusedVariableCheck : BaseAnalyzer
 
 	override void visit(const StructBody structBody)
 	{
-		bool sb = inAggregateScope;
+		immutable bool sb = inAggregateScope;
 		inAggregateScope = true;
 		foreach (dec; structBody.declarations)
 			visit(dec);
@@ -311,7 +311,7 @@ class UnusedVariableCheck : BaseAnalyzer
 
 	override void visit(const ConditionalStatement conditionalStatement)
 	{
-		bool cs = blockStatementIntroducesScope;
+		immutable bool cs = blockStatementIntroducesScope;
 		blockStatementIntroducesScope = false;
 		conditionalStatement.accept(this);
 		blockStatementIntroducesScope = cs;
@@ -325,6 +325,8 @@ class UnusedVariableCheck : BaseAnalyzer
 			variableUsed(primary.identifierChain.identifiers[0].text);
 	}
 
+private:
+
 	void variableDeclared(string name, size_t line, size_t column,
 		bool isParameter, bool isRef)
 	{
@@ -353,8 +355,8 @@ class UnusedVariableCheck : BaseAnalyzer
 		{
 			if (!uu.isRef && tree.length > 1)
 			{
-				string certainty = uu.uncertain ? " might not be used." : " is never used.";
+				immutable string certainty = uu.uncertain ? " might not be used." : " is never used.";
-				string errorMessage = (uu.isParameter ? "Parameter " : "Variable ")
+				immutable string errorMessage = (uu.isParameter ? "Parameter " : "Variable ")
 						~ uu.name ~ certainty;
 				addErrorMessage(uu.line, uu.column,
 					uu.isParameter ? "dscanner.suspicious.unused_parameter"