Replace libdparse in IfElseSameCheck

src/dscanner/analysis/ifelsesame.d

@@ -5,111 +5,162 @@
 
 module dscanner.analysis.ifelsesame;
 
-import std.stdio;
-import dparse.ast;
-import dparse.lexer;
 import dscanner.analysis.base;
-import dscanner.analysis.helpers;
-import dsymbol.scope_ : Scope;
+import dmd.hdrgen : toChars;
+import dmd.tokens : EXP;
+import std.conv : to;
+import std.string : format;
+import std.typecons : Tuple, tuple;
 
 /**
  * Checks for duplicated code in conditional and logical expressions.
  * $(UL
  * $(LI If statements whose "then" block is the same as the "else" block)
  * $(LI || and && expressions where the left and right are the same)
- * $(LI == expressions where the left and right are the same)
+ * $(LI == and != expressions where the left and right are the same)
+ * $(LI >, <, >=, and <= expressions where the left and right are the same)
+ * $(LI Assignments where the left and right are the same)
  * )
  */
-final class IfElseSameCheck : BaseAnalyzer
+extern (C++) class IfElseSameCheck(AST) : BaseAnalyzerDmd
 {
-	alias visit = BaseAnalyzer.visit;
-
+	alias visit = BaseAnalyzerDmd.visit;
 	mixin AnalyzerInfo!"if_else_same_check";
 
-	this(BaseAnalyzerArguments args)
+	private enum IF_KEY = "dscanner.bugs.if_else_same";
+	private enum IF_MESSAGE = "'Else' branch is identical to 'Then' branch.";
+
+	private enum LOGICAL_EXP_KEY = "dscanner.bugs.logic_operator_operands";
+	private enum LOGICAL_EXP_MESSAGE = "Left side of logical %s is identical to right side.";
+
+	private enum ASSIGN_KEY = "dscanner.bugs.self_assignment";
+	private enum ASSIGN_MESSAGE = "Left side of assignment operation is identical to the right side.";
+
+	private bool inAssignment = false;
+
+	extern (D) this(string fileName, bool skipTests = false)
 	{
-		super(args);
+		super(fileName, skipTests);
 	}
 
-	override void visit(const IfStatement ifStatement)
+	override void visit(AST.IfStatement ifStatement)
 	{
-		if (ifStatement.thenStatement && (ifStatement.thenStatement == ifStatement.elseStatement))
+		super.visit(ifStatement);
+
+		if (ifStatement.ifbody is null || ifStatement.elsebody is null)
+			return;
+
+		auto thenBody = to!string(toChars(ifStatement.ifbody));
+		auto elseBody = to!string(toChars(ifStatement.elsebody));
+
+		if (thenBody == elseBody)
 		{
-			const(Token)[] tokens = ifStatement.elseStatement.tokens;
-			// extend 1 past, so we include the `else` token
-			tokens = (tokens.ptr - 1)[0 .. tokens.length + 1];
-			addErrorMessage(tokens,
-					"dscanner.bugs.if_else_same", "'Else' branch is identical to 'Then' branch.");
+			auto lineNum = cast(ulong) ifStatement.loc.linnum;
+			auto charNum = cast(ulong) ifStatement.loc.charnum;
+			addErrorMessage(lineNum, charNum, IF_KEY, IF_MESSAGE);
 		}
-		ifStatement.accept(this);
 	}
 
-	override void visit(const AssignExpression assignExpression)
+	override void visit(AST.AssignExp assignExp)
 	{
-		auto e = cast(const AssignExpression) assignExpression.expression;
-		if (e !is null && assignExpression.operator == tok!"="
-				&& e.ternaryExpression == assignExpression.ternaryExpression)
-		{
-			addErrorMessage(assignExpression, "dscanner.bugs.self_assignment",
-					"Left side of assignment operatior is identical to the right side.");
-		}
-		assignExpression.accept(this);
+		bool oldInAssignment = inAssignment;
+		inAssignment = true;
+		super.visit(assignExp);
+		inAssignment = oldInAssignment;
 	}
 
-	override void visit(const AndAndExpression andAndExpression)
+	override void visit(AST.CondExp condExp)
 	{
-		if (andAndExpression.left !is null && andAndExpression.right !is null
-				&& andAndExpression.left == andAndExpression.right)
+		super.visit(condExp);
+		if (inAssignment)
+			handleBinaryExpression(condExp);
+	}
+
+	override void visit(AST.LogicalExp logicalExpr)
+	{
+		super.visit(logicalExpr);
+		handleBinaryExpression(logicalExpr);
+	}
+
+	private void handleBinaryExpression(AST.BinExp expr)
+	{
+		auto expr1 = to!string(toChars(expr.e1));
+		auto expr2 = to!string(toChars(expr.e2));
+
+		if (expr1 == expr2)
 		{
-			addErrorMessage(andAndExpression.right,
-					"dscanner.bugs.logic_operator_operands",
-					"Left side of logical and is identical to right side.");
+			auto lineNum = cast(ulong) expr.loc.linnum;
+			auto charNum = cast(ulong) expr.loc.charnum;
+			auto errorInfo = getErrorInfo(expr.op);
+			addErrorMessage(lineNum, charNum, errorInfo[0], errorInfo[1]);
 		}
-		andAndExpression.accept(this);
 	}
 
-	override void visit(const OrOrExpression orOrExpression)
+	private extern (D) Tuple!(string, string) getErrorInfo(EXP op)
 	{
-		if (orOrExpression.left !is null && orOrExpression.right !is null
-				&& orOrExpression.left == orOrExpression.right)
+		switch (op)
 		{
-			addErrorMessage(orOrExpression.right,
-					"dscanner.bugs.logic_operator_operands",
-					"Left side of logical or is identical to right side.");
+		case EXP.orOr:
+			return tuple(LOGICAL_EXP_KEY, LOGICAL_EXP_MESSAGE.format("or"));
+		case EXP.andAnd:
+			return tuple(LOGICAL_EXP_KEY, LOGICAL_EXP_MESSAGE.format("and"));
+		case EXP.question:
+			return tuple(ASSIGN_KEY, ASSIGN_MESSAGE);
+		default:
+			assert(0);
 		}
-		orOrExpression.accept(this);
+
 	}
 }
 
 unittest
 {
 	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarningsDMD;
+	import std.stdio : stderr;
 
 	StaticAnalysisConfig sac = disabledConfig();
 	sac.if_else_same_check = Check.enabled;
-	assertAnalyzerWarnings(q{
-		void testSizeT()
+
+	assertAnalyzerWarningsDMD(q{
+		void testThenElseSame()
 		{
 			string person = "unknown";
-			if (person == "unknown")
-				person = "bobrick"; /* same */
+			if (person == "unknown") // [warn]: 'Else' branch is identical to 'Then' branch.
+				person = "bobrick";
 			else
-				person = "bobrick"; /* same */ /+
-^^^^^^^^^^^^^^^^^^^^^^^ [warn]: 'Else' branch is identical to 'Then' branch. +/
-			// note: above ^^^ line spans over multiple lines, so it starts at start of line, since we don't have any way to test this here
-			// look at the tests using 1-wide tab width for accurate visuals.
+				person = "bobrick";
 
-			if (person == "unknown") // ok
-				person = "ricky"; // not same
+			if (person == "unknown")
+				person = "ricky";
 			else
-				person = "bobby"; // not same
+				person = "bobby";
+		}
+	}c, sac);
+
+	assertAnalyzerWarningsDMD(q{
+		void testLogicalExp()
+		{
+			int a = 5, b = 5;
+			if (a == b || a == b) // [warn]: Left side of logical or is identical to right side.
+				a = 6;
+			if (a == b && a == b) // [warn]: Left side of logical and is identical to right side.
+				a = 6;
+		}
+	}c, sac);
+
+	assertAnalyzerWarningsDMD(q{
+		void testAssignExp()
+		{
+			int a = 5, b = 5;
+			a = b > 5 ? b : b; // [warn]: Left side of assignment operation is identical to the right side.
 		}
 	}c, sac);
 
-	assertAnalyzerWarnings(q{
+	assertAnalyzerWarningsDMD(q{
 		void foo()
 		{
-			if (auto stuff = call())
+			if (auto stuff = call()) {}
 		}
 	}c, sac);
 

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!IfElseSameCheck(analysisConfig))
-		checks ~= new IfElseSameCheck(args.setSkipTests(
-		analysisConfig.if_else_same_check == Check.skipTests&& !ut));
-
 	if (moduleName.shouldRun!LabelVarNameCheck(analysisConfig))
 		checks ~= new LabelVarNameCheck(args.setSkipTests(
 		analysisConfig.label_var_same_name_check == Check.skipTests && !ut));
@@ -1347,6 +1343,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN
 			config.number_style_check == Check.skipTests && !ut
 		);
 
+	if (moduleName.shouldRunDmd!(IfElseSameCheck!ASTCodegen)(config))
+		visitors ~= new IfElseSameCheck!ASTCodegen(
+			fileName,
+			config.if_else_same_check == Check.skipTests && !ut
+		);
+
 	foreach (visitor; visitors)
 	{
 		m.accept(visitor);