Replace libdparse with DMD in LabelVarNameCheck (#101)

* Replace libdparse with DMD in LabelVarNameCheck

* Disable check for local functions

src/dscanner/analysis/label_var_same_name_check.d

@@ -4,174 +4,282 @@
 
 module dscanner.analysis.label_var_same_name_check;
 
-import dparse.ast;
-import dparse.lexer;
-import dsymbol.scope_ : Scope;
 import dscanner.analysis.base;
-import dscanner.analysis.helpers;
+import dmd.cond : Include;
+import std.conv : to;
 
 /**
  * Checks for labels and variables that have the same name.
  */
-final class LabelVarNameCheck : ScopedBaseAnalyzer
+extern (C++) class LabelVarNameCheck(AST) : BaseAnalyzerDmd
 {
 	mixin AnalyzerInfo!"label_var_same_name_check";
+	alias visit = BaseAnalyzerDmd.visit;
 
-	this(BaseAnalyzerArguments args)
+	mixin ScopedVisit!(AST.Module);
+	mixin ScopedVisit!(AST.IfStatement);
+	mixin ScopedVisit!(AST.WithStatement);
+	mixin ScopedVisit!(AST.WhileStatement);
+	mixin ScopedVisit!(AST.DoStatement);
+	mixin ScopedVisit!(AST.ForStatement);
+	mixin ScopedVisit!(AST.CaseStatement);
+	mixin ScopedVisit!(AST.ForeachStatement);
+	mixin ScopedVisit!(AST.ForeachRangeStatement);
+	mixin ScopedVisit!(AST.ScopeStatement);
+	mixin ScopedVisit!(AST.FuncAliasDeclaration);
+	mixin ScopedVisit!(AST.CtorDeclaration);
+	mixin ScopedVisit!(AST.DtorDeclaration);
+	mixin ScopedVisit!(AST.InvariantDeclaration);
+
+	mixin FunctionVisit!(AST.FuncDeclaration);
+	mixin FunctionVisit!(AST.TemplateDeclaration);
+	mixin FunctionVisit!(AST.UnitTestDeclaration);
+	mixin FunctionVisit!(AST.FuncLiteralDeclaration);
+
+	mixin AggregateVisit!(AST.ClassDeclaration);
+	mixin AggregateVisit!(AST.StructDeclaration);
+	mixin AggregateVisit!(AST.InterfaceDeclaration);
+	mixin AggregateVisit!(AST.UnionDeclaration);
+
+	extern (D) this(string fileName, bool skipTests = false)
 	{
-		super(args);
+		super(fileName);
 	}
 
-	mixin AggregateVisit!ClassDeclaration;
+	override void visit(AST.VarDeclaration vd)
-	mixin AggregateVisit!StructDeclaration;
-	mixin AggregateVisit!InterfaceDeclaration;
-	mixin AggregateVisit!UnionDeclaration;
-
-	override void visit(const VariableDeclaration var)
 	{
-		foreach (dec; var.declarators)
+		import dmd.astenums : STC;
-			duplicateCheck(dec.name, false, conditionalDepth > 0);
+
+		if (!(vd.storage_class & STC.scope_) && !isInLocalFunction)
+		{
+			auto thing = Thing(to!string(vd.ident.toChars()), vd.loc.linnum, vd.loc.charnum);
+			duplicateCheck(thing, false, conditionalDepth > 0);
+		}
+
+		super.visit(vd);
 	}
 
-	override void visit(const LabeledStatement labeledStatement)
+	override void visit(AST.LabelStatement ls)
 	{
-		duplicateCheck(labeledStatement.identifier, true, conditionalDepth > 0);
+		auto thing = Thing(to!string(ls.ident.toChars()), ls.loc.linnum, ls.loc.charnum);
-		if (labeledStatement.declarationOrStatement !is null)
+		duplicateCheck(thing, true, conditionalDepth > 0);
-			labeledStatement.declarationOrStatement.accept(this);
+		super.visit(ls);
 	}
 
-	override void visit(const ConditionalDeclaration condition)
+	override void visit(AST.ConditionalDeclaration conditionalDeclaration)
 	{
-		if (condition.falseDeclarations.length > 0)
+		import dmd.root.array : peekSlice;
+
+		conditionalDeclaration.condition.accept(this);
+
+		if (conditionalDeclaration.condition.isVersionCondition())
 			++conditionalDepth;
-		condition.accept(this);
+
-		if (condition.falseDeclarations.length > 0)
+		if (conditionalDeclaration.elsedecl || conditionalDeclaration.condition.inc != Include.yes)
+			pushScope();
+
+		foreach (decl; conditionalDeclaration.decl.peekSlice())
+			decl.accept(this);
+
+		if (conditionalDeclaration.elsedecl || conditionalDeclaration.condition.inc != Include.yes)
+			popScope();
+
+		if (conditionalDeclaration.elsedecl)
+			foreach (decl; conditionalDeclaration.elsedecl.peekSlice())
+				decl.accept(this);
+
+		if (conditionalDeclaration.condition.isVersionCondition())
 			--conditionalDepth;
 	}
 
-	override void visit(const VersionCondition condition)
+	override void visit(AST.ConditionalStatement conditionalStatement)
 	{
-		++conditionalDepth;
+		conditionalStatement.condition.accept(this);
-		condition.accept(this);
+
-		--conditionalDepth;
+		if (conditionalStatement.condition.isVersionCondition)
+			++conditionalDepth;
+
+		if (conditionalStatement.ifbody)
+		{
+			if (conditionalStatement.elsebody)
+				pushScope();
+
+			conditionalStatement.ifbody.accept(this);
+
+			if (conditionalStatement.elsebody)
+				popScope();
+		}
+
+		if (conditionalStatement.elsebody)
+		{
+			if (conditionalStatement.condition.inc == Include.no)
+				pushScope();
+
+			conditionalStatement.elsebody.accept(this);
+
+			if (conditionalStatement.condition.inc == Include.no)
+				popScope();
+		}
+
+		if (conditionalStatement.condition.isVersionCondition)
+			--conditionalDepth;
 	}
 
-	alias visit = ScopedBaseAnalyzer.visit;
+	override void visit(AST.AnonDeclaration ad)
+	{
+		pushScope();
+		pushAggregateName("", ad.loc.linnum, ad.loc.charnum);
+		super.visit(ad);
+		popScope();
+		popAggregateName();
+	}
 
 private:
-
+	extern (D) Thing[string][] stack;
+	int conditionalDepth;
+	extern (D) Thing[] parentAggregates;
+	extern (D) string parentAggregateText;
+	bool isInFunction;
+	bool isInLocalFunction;
 	enum string KEY = "dscanner.suspicious.label_var_same_name";
 
-	Thing[string][] stack;
+	template FunctionVisit(NodeType)
+	{
+		override void visit(NodeType n)
+		{
+			auto oldIsInFunction = isInFunction;
+			auto oldIsInLocalFunction = isInLocalFunction;
+
+			pushScope();
+
+			if (isInFunction)
+				isInLocalFunction = true;
+			else
+				isInFunction = true;
+
+			super.visit(n);
+			popScope();
+
+			isInFunction = oldIsInFunction;
+			isInLocalFunction = oldIsInLocalFunction;
+		}
+	}
 
 	template AggregateVisit(NodeType)
 	{
-		override void visit(const NodeType n)
+		override void visit(NodeType n)
 		{
-			pushAggregateName(n.name);
+			pushScope();
-			n.accept(this);
+			pushAggregateName(to!string(n.ident.toString()), n.loc.linnum, n.loc.charnum);
+			super.visit(n);
+			popScope();
 			popAggregateName();
 		}
 	}
 
-	void duplicateCheck(const Token name, bool fromLabel, bool isConditional)
+	template ScopedVisit(NodeType)
+	{
+		override void visit(NodeType n)
+		{
+			pushScope();
+			super.visit(n);
+			popScope();
+		}
+	}
+
+	extern (D) void duplicateCheck(const Thing id, bool fromLabel, bool isConditional)
 	{
-		import std.conv : to;
 		import std.range : retro;
 
 		size_t i;
 		foreach (s; retro(stack))
 		{
-			string fqn = parentAggregateText ~ name.text;
+			string fqn = parentAggregateText ~ id.name;
 			const(Thing)* thing = fqn in s;
 			if (thing is null)
-				currentScope[fqn] = Thing(fqn, name.line, name.column, !fromLabel /+, isConditional+/ );
+			{
+				currentScope[fqn] = Thing(fqn, id.line, id.column, !fromLabel);
+			}
 			else if (i != 0 || !isConditional)
 			{
 				immutable thisKind = fromLabel ? "Label" : "Variable";
 				immutable otherKind = thing.isVar ? "variable" : "label";
-				addErrorMessage(name, KEY,
+				auto msg = thisKind ~ " \"" ~ fqn ~ "\" has the same name as a "
-						thisKind ~ " \"" ~ fqn ~ "\" has the same name as a "
+					~ otherKind ~ " defined on line " ~ to!string(thing.line) ~ ".";
-						~ otherKind ~ " defined on line " ~ to!string(thing.line) ~ ".");
+				addErrorMessage(id.line, id.column, KEY, msg);
 			}
 			++i;
 		}
 	}
 
-	static struct Thing
+	extern (D) static struct Thing
 	{
 		string name;
 		size_t line;
 		size_t column;
 		bool isVar;
-		//bool isConditional;
 	}
 
-	ref currentScope() @property
+	extern (D) ref currentScope() @property
 	{
 		return stack[$ - 1];
 	}
 
-	protected override void pushScope()
+	extern (D) void pushScope()
 	{
 		stack.length++;
 	}
 
-	protected override void popScope()
+	extern (D) void popScope()
 	{
 		stack.length--;
 	}
 
-	int conditionalDepth;
+	extern (D) void pushAggregateName(string name, size_t line, size_t column)
-
-	void pushAggregateName(Token name)
 	{
-		parentAggregates ~= name;
+		parentAggregates ~= Thing(name, line, column);
 		updateAggregateText();
 	}
 
-	void popAggregateName()
+	extern (D) void popAggregateName()
 	{
 		parentAggregates.length -= 1;
 		updateAggregateText();
 	}
 
-	void updateAggregateText()
+	extern (D) void updateAggregateText()
 	{
 		import std.algorithm : map;
 		import std.array : join;
 
 		if (parentAggregates.length)
-			parentAggregateText = parentAggregates.map!(a => a.text).join(".") ~ ".";
+			parentAggregateText = parentAggregates.map!(a => a.name).join(".") ~ ".";
 		else
 			parentAggregateText = "";
 	}
-
-	Token[] parentAggregates;
-	string parentAggregateText;
 }
 
 unittest
 {
 	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarningsDMD;
 	import std.stdio : stderr;
 
 	StaticAnalysisConfig sac = disabledConfig();
 	sac.label_var_same_name_check = Check.enabled;
-	assertAnalyzerWarnings(q{
+
+	assertAnalyzerWarningsDMD(q{
 unittest
 {
 blah:
-	int blah; /+
+	int blah; // [warn]: Variable "blah" has the same name as a label defined on line 4.
-	    ^^^^ [warn]: Variable "blah" has the same name as a label defined on line 4. +/
 }
 int blah;
 unittest
 {
 	static if (stuff)
 		int a;
-	int a; /+
+	int a; // [warn]: Variable "a" has the same name as a variable defined on line 11.
-	    ^ [warn]: Variable "a" has the same name as a variable defined on line 12. +/
 }
 
 unittest
@@ -188,8 +296,7 @@ unittest
 		int a = 10;
 	else
 		int a = 20;
-	int a; /+
+	int a; // [warn]: Variable "a" has the same name as a variable defined on line 28.
-	    ^ [warn]: Variable "a" has the same name as a variable defined on line 30. +/
 }
 template T(stuff)
 {
@@ -212,8 +319,7 @@ unittest
 		int c = 10;
 	else
 		int c = 20;
-	int c; /+
+	int c; // [warn]: Variable "c" has the same name as a variable defined on line 51.
-	    ^ [warn]: Variable "c" has the same name as a variable defined on line 54. +/
 }
 
 unittest
@@ -251,8 +357,7 @@ unittest
 	interface A
 	{
 		int a;
-		int a; /+
+		int a; // [warn]: Variable "A.a" has the same name as a variable defined on line 89.
-		    ^ [warn]: Variable "A.a" has the same name as a variable defined on line 93. +/
 	}
 }
 
@@ -276,7 +381,6 @@ unittest
 		break;
 	}
 }
-
 }c, sac);
 	stderr.writeln("Unittest for LabelVarNameCheck passed.");
 }

src/dscanner/analysis/run.d

@@ -843,10 +843,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
 		checks ~= new FunctionAttributeCheck(args.setSkipTests(
 		analysisConfig.function_attribute_check == Check.skipTests && !ut));
 
-	if (moduleName.shouldRun!LabelVarNameCheck(analysisConfig))
-		checks ~= new LabelVarNameCheck(args.setSkipTests(
-		analysisConfig.label_var_same_name_check == Check.skipTests && !ut));
-
 	if (moduleName.shouldRun!MismatchedArgumentCheck(analysisConfig))
 		checks ~= new MismatchedArgumentCheck(args.setSkipTests(
 		analysisConfig.mismatched_args_check == Check.skipTests && !ut));
@@ -1351,6 +1347,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN
 			config.max_cyclomatic_complexity.to!int
 		);
 
+	if (moduleName.shouldRunDmd!(LabelVarNameCheck!ASTCodegen)(config))
+		visitors ~= new LabelVarNameCheck!ASTCodegen(
+			fileName,
+			config.label_var_same_name_check == Check.skipTests && !ut
+		);
+
 	foreach (visitor; visitors)
 	{
 		m.accept(visitor);