Always Check Curly

Check that if|else|for|foreach|while|do|try|catch
are always followed by a BlockStatement aka. { }

closer

can not get the test to work

try to get the AutoFix in place

maybe a fix

nicer messages

some formatting

more tinkering

still nothing

autofix work now

AutoFix name

message to message_postfix

src/dscanner/analysis/always_curly.d

@@ -0,0 +1,227 @@
+// 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 dscanner.analysis.always_curly;
+
+import dparse.lexer;
+import dparse.ast;
+import dscanner.analysis.base;
+import dsymbol.scope_ : Scope;
+
+import std.array : back, front;
+import std.algorithm;
+import std.range;
+import std.stdio;
+
+final class AlwaysCurlyCheck : BaseAnalyzer
+{
+	mixin AnalyzerInfo!"always_curly_check";
+
+	alias visit = BaseAnalyzer.visit;
+
+	///
+	this(string fileName, const(Token)[] tokens, bool skipTests = false)
+	{
+		super(fileName, null, skipTests);
+	}
+
+	void test(L, B)(L loc, B s, string stmtKind)
+	{
+		if (!is(s == BlockStatement))
+		{
+			if (!s.tokens.empty)
+			{
+				AutoFix af = AutoFix.insertionBefore(s.tokens.front, " { ")
+					.concat(AutoFix.insertionAfter(s.tokens.back, " } "));
+				af.name = "Wrap in braces";
+
+				addErrorMessage(loc, KEY, stmtKind ~ MESSAGE_POSTFIX, [af]);
+			}
+			else
+			{
+				addErrorMessage(loc, KEY, stmtKind ~ MESSAGE_POSTFIX);
+			}
+		}
+	}
+
+	override void visit(const(IfStatement) stmt)
+	{
+		auto s = stmt.thenStatement.statement;
+		this.test(stmt.thenStatement, s, "if");
+		if (stmt.elseStatement !is null)
+		{
+			auto e = stmt.elseStatement.statement;
+			this.test(stmt.elseStatement, e, "else");
+		}
+	}
+
+	override void visit(const(ForStatement) stmt)
+	{
+		auto s = stmt.declarationOrStatement;
+		if (s.statement !is null)
+		{
+			this.test(s, s, "for");
+		}
+	}
+
+	override void visit(const(ForeachStatement) stmt)
+	{
+		auto s = stmt.declarationOrStatement;
+		if (s.statement !is null)
+		{
+			this.test(s, s, "foreach");
+		}
+	}
+
+	override void visit(const(TryStatement) stmt)
+	{
+		auto s = stmt.declarationOrStatement;
+		if (s.statement !is null)
+		{
+			this.test(s, s, "try");
+		}
+
+		if (stmt.catches !is null)
+		{
+			foreach (const(Catch) ct; stmt.catches.catches)
+			{
+				this.test(ct, ct.declarationOrStatement, "catch");
+			}
+			if (stmt.catches.lastCatch !is null)
+			{
+				auto sncnd = stmt.catches.lastCatch.statementNoCaseNoDefault;
+				if (sncnd !is null)
+				{
+					this.test(stmt.catches.lastCatch, sncnd, "finally");
+				}
+			}
+		}
+	}
+
+	override void visit(const(WhileStatement) stmt)
+	{
+		auto s = stmt.declarationOrStatement;
+		if (s.statement !is null)
+		{
+			this.test(s, s, "while");
+		}
+	}
+
+	override void visit(const(DoStatement) stmt)
+	{
+		auto s = stmt.statementNoCaseNoDefault;
+		if (s !is null)
+		{
+			this.test(s, s, "do");
+		}
+	}
+
+	enum string KEY = "dscanner.style.always_curly";
+	enum string MESSAGE_POSTFIX = " must be follow by a BlockStatement aka. { }";
+}
+
+unittest
+{
+	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
+	import std.stdio : stderr;
+
+	StaticAnalysisConfig sac = disabledConfig();
+	sac.always_curly_check = Check.enabled;
+
+	assertAnalyzerWarnings(q{
+		void testIf()
+		{
+			if(true) return; // [warn]: if must be follow by a BlockStatement aka. { }
+		}
+	}, sac);
+
+	assertAnalyzerWarnings(q{
+		void testIf()
+		{
+			if(true) return; /+
+			         ^^^^^^^ [warn]: if must be follow by a BlockStatement aka. { } +/
+		}
+	}, sac);
+
+	assertAnalyzerWarnings(q{
+		void testIf()
+		{
+			for(int i = 0; i < 10; ++i) return; // [warn]: for must be follow by a BlockStatement aka. { }
+		}
+	}, sac);
+
+	assertAnalyzerWarnings(q{
+		void testIf()
+		{
+			foreach(it; 0 .. 10) return; // [warn]: foreach must be follow by a BlockStatement aka. { }
+		}
+	}, sac);
+
+	assertAnalyzerWarnings(q{
+		void testIf()
+		{
+			while(true) return; // [warn]: while must be follow by a BlockStatement aka. { }
+		}
+	}, sac);
+
+	assertAnalyzerWarnings(q{
+		void testIf()
+		{
+			do return; while(true); return; // [warn]: do must be follow by a BlockStatement aka. { }
+		}
+	}, sac);
+}
+
+unittest {
+	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
+	import std.stdio : stderr;
+
+	StaticAnalysisConfig sac = disabledConfig();
+	sac.always_curly_check = Check.enabled;
+
+	assertAutoFix(q{
+		void test() {
+			if(true) return; // fix:0
+		}
+	}c, q{
+		void test() {
+			if(true) { return; } // fix:0
+		}
+	}c, sac);
+
+	assertAutoFix(q{
+		void test() {
+			foreach(_; 0 .. 10 ) return; // fix:0
+		}
+	}c, q{
+		void test() {
+			foreach(_; 0 .. 10 ) { return; } // fix:0
+		}
+	}c, sac);
+
+	assertAutoFix(q{
+		void test() {
+			for(int i = 0; i < 10; ++i) return; // fix:0
+		}
+	}c, q{
+		void test() {
+			for(int i = 0; i < 10; ++i) { return; } // fix:0
+		}
+	}c, sac);
+
+	assertAutoFix(q{
+		void test() {
+			do return; while(true) // fix:0
+		}
+	}c, q{
+		void test() {
+			do { return; } while(true) // fix:0
+		}
+	}c, sac);
+
+
+	stderr.writeln("Unittest for AlwaysCurly passed.");
+}

src/dscanner/analysis/config.d

@@ -167,6 +167,9 @@ struct StaticAnalysisConfig
 	@INI("Check for auto function without return statement")
 	string auto_function_check = Check.disabled;
 
+	@INI("Check that if|else|for|foreach|while|do|try|catch are always followed by a BlockStatement { }")
+	string always_curly_check = Check.disabled;
+
 	@INI("Check for sortedness of imports")
 	string imports_sortedness = Check.disabled;
 

src/dscanner/analysis/run.d

@@ -73,6 +73,7 @@ import dscanner.analysis.final_attribute;
 import dscanner.analysis.vcall_in_ctor;
 import dscanner.analysis.useless_initializer;
 import dscanner.analysis.allman;
+import dscanner.analysis.always_curly;
 import dscanner.analysis.redundant_attributes;
 import dscanner.analysis.has_public_example;
 import dscanner.analysis.assert_without_msg;
@@ -917,6 +918,10 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
 		checks ~= new AllManCheck(fileName, tokens,
 		analysisConfig.allman_braces_check == Check.skipTests && !ut);
 
+	if (moduleName.shouldRun!AlwaysCurlyCheck(analysisConfig))
+		checks ~= new AlwaysCurlyCheck(fileName, tokens,
+		analysisConfig.always_curly_check == Check.skipTests && !ut);
+
 	if (moduleName.shouldRun!RedundantAttributesCheck(analysisConfig))
 		checks ~= new RedundantAttributesCheck(fileName, moduleScope,
 		analysisConfig.redundant_attributes_check == Check.skipTests && !ut);