From 5ba4a7bffa4e820de26804bb21f51286b6cb81e2 Mon Sep 17 00:00:00 2001
From: Sebastian Wilzbach <seb@wilzba.ch>
Date: Sat, 8 Jul 2017 03:28:08 +0200
Subject: [PATCH] Add check for asserts without an explantory message

---
 README.md                         |  1 +
 src/analysis/assert_without_msg.d | 68 +++++++++++++++++++++++++++++++
 src/analysis/config.d             |  3 ++
 src/analysis/run.d                |  5 +++
 4 files changed, 77 insertions(+)
 create mode 100644 src/analysis/assert_without_msg.d

diff --git a/README.md b/README.md
index ac69b98..7e7b9ae 100644
--- a/README.md
+++ b/README.md
@@ -134,6 +134,7 @@ Note that the "--skipTests" option is the equivalent of changing each
 * Allman brace style
 * Redundant visibility attributes
 * Public declarations without a documented unittest. By default disabled.
+* Asserts without an explanatory message. By default disabled.
 
 #### Wishlist
 
diff --git a/src/analysis/assert_without_msg.d b/src/analysis/assert_without_msg.d
new file mode 100644
index 0000000..5a69a37
--- /dev/null
+++ b/src/analysis/assert_without_msg.d
@@ -0,0 +1,68 @@
+// 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.assert_without_msg;
+
+import analysis.base : BaseAnalyzer;
+import dsymbol.scope_ : Scope;
+import dparse.lexer;
+import dparse.ast;
+
+import std.stdio;
+
+/**
+ * Check that all asserts have an explanatory message.
+ */
+class AssertWithoutMessageCheck : BaseAnalyzer
+{
+	enum string KEY = "dscanner.style.assert_without_msg";
+	enum string MESSAGE = "An assert should have an explanatory message";
+
+	///
+	this(string fileName, const(Scope)* sc, bool skipTests = false)
+	{
+		super(fileName, sc, skipTests);
+	}
+
+	override void visit(const AssertExpression expr)
+	{
+		if (expr.message is null)
+			addErrorMessage(expr.line, expr.column, KEY, MESSAGE);
+	}
+
+	alias visit = BaseAnalyzer.visit;
+
+}
+
+unittest
+{
+	import std.stdio : stderr;
+	import std.format : format;
+	import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
+	import analysis.helpers : assertAnalyzerWarnings;
+
+	StaticAnalysisConfig sac = disabledConfig();
+	sac.assert_without_msg = Check.enabled;
+
+	assertAnalyzerWarnings(q{
+	unittest {
+		assert(0, "foo bar");
+		assert(0); // [warn]: %s
+	}
+	}c.format(
+		AssertWithoutMessageCheck.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+	unittest {
+		static assert(0, "foo bar");
+		static assert(0); // [warn]: %s
+	}
+	}c.format(
+		AssertWithoutMessageCheck.MESSAGE,
+	), sac);
+
+
+	stderr.writeln("Unittest for AssertWithoutMessageCheck passed.");
+}
diff --git a/src/analysis/config.d b/src/analysis/config.d
index 1966e98..ebf811b 100644
--- a/src/analysis/config.d
+++ b/src/analysis/config.d
@@ -189,6 +189,9 @@ struct StaticAnalysisConfig
 	@INI("Check public declarations without a documented unittest")
 	string has_public_example = Check.disabled;
 
+	@INI("Check for asserts without an explanatory message")
+	string assert_without_msg = Check.disabled;
+
 	@INI("Module-specific filters")
 	ModuleFilters filters;
 }
diff --git a/src/analysis/run.d b/src/analysis/run.d
index 16f7e67..847da2f 100644
--- a/src/analysis/run.d
+++ b/src/analysis/run.d
@@ -69,6 +69,7 @@ import analysis.useless_initializer;
 import analysis.allman;
 import analysis.redundant_attributes;
 import analysis.has_public_example;
+import analysis.assert_without_msg;
 
 import dsymbol.string_interning : internString;
 import dsymbol.scope_;
@@ -481,6 +482,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
 		checks ~= new HasPublicExampleCheck(fileName, moduleScope,
 		analysisConfig.has_public_example == Check.skipTests && !ut);
 
+	if (moduleName.shouldRun!"assert_without_msg"(analysisConfig))
+		checks ~= new AssertWithoutMessageCheck(fileName, moduleScope,
+		analysisConfig.assert_without_msg == Check.skipTests && !ut);
+
 	version (none)
 		if (moduleName.shouldRun!"redundant_if_check"(analysisConfig))
 			checks ~= new IfStatementCheck(fileName, moduleScope,