From 0943820ebecc903f86a95f4a69278a5562fc4553 Mon Sep 17 00:00:00 2001
From: dhasenan <dhasenan@gmail.com>
Date: Fri, 11 Mar 2016 17:59:27 +0000
Subject: [PATCH] Added a test for static if...else if mistakes.

If you write your code as:
static if (foo)
  doFoo();
else if (bar)
  doBar();

This check will catch it. If you write it as:
static if (foo)
  doFoo();
else {
  if (bar)
    doBar();
}

This isn't a bug and we won't warn about it.

If you write your code in an intermediate style, we'll still warn about it:
static if (foo)
  doFoo();
else
  if (bar)
    doBar();

Which probably isn't optimal, but what the hey.
---
 src/analysis/config.d         |  3 ++
 src/analysis/run.d            |  3 ++
 src/analysis/static_if_else.d | 95 +++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 src/analysis/static_if_else.d

diff --git a/src/analysis/config.d b/src/analysis/config.d
index 961b5cd..2331646 100644
--- a/src/analysis/config.d
+++ b/src/analysis/config.d
@@ -113,4 +113,7 @@ struct StaticAnalysisConfig
 
 	@INI("Check for uses of the old-style alias syntax")
 	bool alias_syntax_check;
+
+	@INI("Checks for else if that should be else static if")
+	bool static_if_else_check;
 }
diff --git a/src/analysis/run.d b/src/analysis/run.d
index b2e2de0..bf83753 100644
--- a/src/analysis/run.d
+++ b/src/analysis/run.d
@@ -56,6 +56,7 @@ import analysis.auto_ref_assignment;
 import analysis.incorrect_infinite_range;
 import analysis.useless_assert;
 import analysis.alias_syntax_check;
+import analysis.static_if_else;
 
 import dsymbol.string_interning : internString;
 import dsymbol.scope_;
@@ -269,6 +270,8 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
 		checks ~= new UselessAssertCheck(fileName);
 	if (analysisConfig.alias_syntax_check)
 		checks ~= new AliasSyntaxCheck(fileName);
+	if (analysisConfig.static_if_else_check)
+		checks ~= new StaticIfElse(fileName);
 	version (none)
 		if (analysisConfig.redundant_if_check)
 			checks ~= new IfStatementCheck(fileName, moduleScope);
diff --git a/src/analysis/static_if_else.d b/src/analysis/static_if_else.d
new file mode 100644
index 0000000..00d5880
--- /dev/null
+++ b/src/analysis/static_if_else.d
@@ -0,0 +1,95 @@
+// Copyright Chris Wright (dhasenan) 2016.
+// 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.static_if_else;
+
+import dparse.ast;
+import dparse.lexer;
+import analysis.base;
+
+/**
+ * Checks for potentially mistaken static if / else if.
+ *
+ * It's potentially valid to write:
+ * ---
+ * static if (foo) {
+ * } else if (bar) {
+ * }
+ * ---
+ * 
+ * However, it's more likely that this is a mistake.
+ */
+class StaticIfElse : BaseAnalyzer
+{
+	alias visit = BaseAnalyzer.visit;
+
+	this(string filename)
+	{
+		super(filename, null);
+	}
+
+	override void visit(const ConditionalStatement cc)
+	{
+		cc.accept(this);
+		if (cc.falseStatement is null)
+		{
+			return;
+		}
+		const(IfStatement) ifStmt = getIfStatement(cc);
+		if (!ifStmt)
+		{
+			return;
+		}
+		addErrorMessage(ifStmt.line, ifStmt.column, KEY, "Mismatched static if. Use 'else static if' here.");
+	}
+
+	const(IfStatement) getIfStatement(const ConditionalStatement cc)
+	{
+		if (cc.falseStatement.statement)
+		{
+			if (cc.falseStatement.statement.statementNoCaseNoDefault)
+			{
+				if (cc.falseStatement.statement.statementNoCaseNoDefault.ifStatement)
+				{
+					return cc.falseStatement.statement.statementNoCaseNoDefault.ifStatement;
+				}
+			}
+		}
+		return null;
+	}
+
+	enum KEY = "dscanner.suspicious.static_if_else";
+}
+
+unittest
+{
+	import analysis.helpers : assertAnalyzerWarnings;
+	import analysis.config : StaticAnalysisConfig;
+	import std.stdio : stderr;
+
+	StaticAnalysisConfig sac;
+	sac.static_if_else_check = true;
+	assertAnalyzerWarnings(q{
+		void foo() {
+			static if (false)
+				auto a = 0;
+			else if (true) // [warn]: Mismatched static if. Use 'else static if' here.
+				auto b = 1;
+		}
+	}c, sac);
+	// Explicit braces, so no warning.
+	assertAnalyzerWarnings(q{
+		void foo() {
+			static if (false)
+				auto a = 0;
+			else {
+				if (true)
+					auto b = 1;
+			}
+		}
+	}c, sac);
+
+	stderr.writeln("Unittest for StaticIfElse passed.");
+}