Merge pull request #389 from wilzbach/explicitly_annotated_unittest

Add check for explictly annotated unittests

src/analysis/config.d

@@ -150,4 +150,7 @@ struct StaticAnalysisConfig
 
 	@INI("Check for sortedness of imports")
 	string imports_sortedness = Check.disabled;
+
+	@INI("Check for explicitly annotated unittests")
+	string explicitly_annotated_unittests = Check.disabled;
 }

src/analysis/explicitly_annotated_unittests.d

@@ -0,0 +1,93 @@
+// 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.explicitly_annotated_unittests;
+
+import dparse.lexer;
+import dparse.ast;
+import analysis.base : BaseAnalyzer;
+
+import std.stdio;
+
+/**
+ * Requires unittests to be explicitly annotated with either @safe or @system
+ */
+class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer
+{
+	enum string KEY = "dscanner.style.explicitly_annotated_unittest";
+	enum string MESSAGE = "A unittest should be annotated with at least @safe or @system";
+
+	///
+	this(string fileName, bool skipTests = false)
+	{
+		super(fileName, null, skipTests);
+	}
+
+	override void visit(const Declaration decl)
+	{
+		if (decl.unittest_ !is null)
+		{
+			bool isSafeOrSystem;
+			if (decl.attributes !is null)
+			foreach (attribute; decl.attributes)
+			{
+				if (attribute.atAttribute !is null)
+				{
+					const token = attribute.atAttribute.identifier.text;
+					if (token == "safe" || token == "system")
+					{
+						isSafeOrSystem = true;
+						break;
+					}
+				}
+			}
+			if (!isSafeOrSystem)
+				addErrorMessage(decl.unittest_.line, decl.unittest_.column, KEY, MESSAGE);
+		}
+		decl.accept(this);
+	}
+
+	alias visit = BaseAnalyzer.visit;
+
+}
+
+unittest
+{
+	import std.stdio : stderr;
+	import std.format : format;
+	import analysis.config : StaticAnalysisConfig, Check;
+	import analysis.helpers : assertAnalyzerWarnings;
+
+	StaticAnalysisConfig sac;
+	sac.explicitly_annotated_unittests = Check.enabled;
+
+	assertAnalyzerWarnings(q{
+		@safe unittest {}
+		@system unittest {}
+		pure nothrow @system @nogc unittest {}
+
+		unittest {} // [warn]: %s
+		pure nothrow @nogc unittest {} // [warn]: %s
+	}c.format(
+		ExplicitlyAnnotatedUnittestCheck.MESSAGE,
+		ExplicitlyAnnotatedUnittestCheck.MESSAGE,
+	), sac);
+
+	// nested
+	assertAnalyzerWarnings(q{
+		struct Foo
+		{
+			@safe unittest {}
+			@system unittest {}
+
+			unittest {} // [warn]: %s
+			pure nothrow @nogc unittest {} // [warn]: %s
+		}
+	}c.format(
+		ExplicitlyAnnotatedUnittestCheck.MESSAGE,
+		ExplicitlyAnnotatedUnittestCheck.MESSAGE,
+	), sac);
+
+	stderr.writeln("Unittest for ExplicitlyAnnotatedUnittestCheck passed.");
+}

src/analysis/run.d

@@ -61,6 +61,7 @@ import analysis.static_if_else;
 import analysis.lambda_return_check;
 import analysis.auto_function;
 import analysis.imports_sortedness;
+import analysis.explicitly_annotated_unittests;
 
 import dsymbol.string_interning : internString;
 import dsymbol.scope_;
@@ -364,6 +365,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
 		checks ~= new ImportSortednessCheck(fileName,
 		analysisConfig.imports_sortedness == Check.skipTests && !ut);
 
+	if (analysisConfig.explicitly_annotated_unittests != Check.disabled)
+		checks ~= new ExplicitlyAnnotatedUnittestCheck(fileName,
+		analysisConfig.explicitly_annotated_unittests == Check.skipTests && !ut);
+
 	version (none)
 		if (analysisConfig.redundant_if_check != Check.disabled)
 			checks ~= new IfStatementCheck(fileName, moduleScope,