Merge e73c5aea3a into 5f1cf31ee0

src/dscanner/analysis/config.d

@@ -215,6 +215,9 @@ struct StaticAnalysisConfig
 	@INI("Maximum cyclomatic complexity after which to issue warnings")
 	int max_cyclomatic_complexity = 50;
 
+	@INI("Check for ignored unittests in templates or functions")
+	string ignored_unittest = Check.enabled;
+
 	@INI("Module-specific filters")
 	ModuleFilters filters;
 }

src/dscanner/analysis/ignored_unittest.d

@@ -0,0 +1,188 @@
+module dscanner.analysis.ignored_unittest;
+
+import dparse.lexer;
+import dparse.ast;
+import dscanner.analysis.base;
+
+import std.stdio;
+import std.meta : AliasSeq;
+
+private string scoped(string v, string val)
+{
+	if (__ctfe)
+	{
+		return "auto _old_" ~ v ~ " = " ~ v ~ "; " ~ v ~ " = " ~ val ~ "; scope (exit) " ~ v ~ " = _old_" ~ v ~ ";";
+	}
+	else
+	{
+		return null;
+	}
+}
+
+/**
+ * unittests in methods are never run, unittests in templates may only serve for documentation
+ */
+final class IgnoredUnittestCheck : BaseAnalyzer
+{
+	enum string KEY_IGNORED = "dscanner.bugs.ignored_unittest";
+	enum string MESSAGE_IGNORED = "unittest blocks can only be run in global scope or in types";
+	enum string KEY_TEMPLATED = "dscanner.suspicious.templated_unittest";
+	enum string MESSAGE_TEMPLATED = "unittest blocks in templates may not be run and can only serve for documentation purpose. Document with a ddoc-comment or move unittest block out of template.";
+	mixin AnalyzerInfo!"ignored_unittest";
+
+	private bool inValidScope = true;
+	private bool inUndocumentable;
+	private bool inTemplate;
+
+	///
+	this(string fileName, bool skipTests = false)
+	{
+		super(fileName, null, skipTests);
+	}
+
+	static foreach (T; AliasSeq!(
+		ClassDeclaration,
+		InterfaceDeclaration,
+		StructDeclaration,
+		UnionDeclaration
+	))
+		override void visit(const T b)
+		{
+			mixin(scoped("inValidScope", "true"));
+			mixin(scoped("inTemplate", "inTemplate || b.templateParameters !is null"));
+			b.accept(this);
+		}
+
+	override void visit(const MixinTemplateDeclaration m)
+	{
+		// ignore mixin templates as they might be used for proper unittests
+		if (m.templateDeclaration)
+		{
+			mixin(scoped("inValidScope", "true"));
+			mixin(scoped("inTemplate", "false"));
+			m.templateDeclaration.accept(this);
+		}
+	}
+
+	override void visit(const FunctionDeclaration b)
+	{
+		mixin(scoped("inTemplate", "inTemplate || b.templateParameters !is null"));
+		b.accept(this);
+	}
+
+	override void visit(const FunctionBody b)
+	{
+		mixin(scoped("inValidScope", "false"));
+		mixin(scoped("inUndocumentable", "true"));
+		b.accept(this);
+	}
+
+	override void visit(const TemplateDeclaration decl)
+	{
+		mixin(scoped("inTemplate", "true"));
+		decl.accept(this);
+	}
+
+	override void visit(const Unittest test)
+	{
+		if (!inValidScope)
+		{
+			addErrorMessage(test.line, test.column, KEY_IGNORED, MESSAGE_IGNORED);
+		}
+		else if (inTemplate && inUndocumentable)
+		{
+			addErrorMessage(test.line, test.column, KEY_IGNORED, MESSAGE_IGNORED);
+		}
+		else if (inTemplate)
+		{
+			if (test.comment is null)
+				addErrorMessage(test.line, test.column, KEY_TEMPLATED, MESSAGE_TEMPLATED);
+		}
+
+		if (!skipTests)
+		{
+			mixin(scoped("inValidScope", "false"));
+			mixin(scoped("inUndocumentable", "true"));
+			test.accept(this);
+		}
+	}
+
+	alias visit = BaseAnalyzer.visit;
+}
+
+unittest
+{
+	import std.stdio : stderr;
+	import std.format : format;
+	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarnings;
+
+	StaticAnalysisConfig sac = disabledConfig();
+	sac.ignored_unittest = Check.enabled;
+
+	assertAnalyzerWarnings(q{
+		unittest {}
+
+		void foo() {
+			unittest {} // [warn]: %s
+		}
+
+		void bar()() {
+			unittest {} // [warn]: %s
+		}
+
+		class T1() {
+			unittest {} // [warn]: %s
+		}
+
+		class T2() {
+			///
+			unittest {}
+		}
+
+		class C {
+			unittest {}
+		}
+
+		unittest {
+			unittest {} // [warn]: %s
+		}
+
+		void test1() {
+			struct S {
+				unittest {} // surprisingly, this gets called
+			}
+		}
+
+		void test2()() {
+			struct S {
+				unittest {} // [warn]: %s
+			}
+		}
+
+		void test2() {
+			struct S() {
+				unittest {} // [warn]: %s
+			}
+		}
+
+		mixin template MT()
+		{
+			unittest { /* ok */ }
+		}
+
+		struct MixedStruct
+		{
+			mixin MT;
+		}
+	}c.format(
+		IgnoredUnittestCheck.MESSAGE_IGNORED,
+		IgnoredUnittestCheck.MESSAGE_IGNORED,
+		IgnoredUnittestCheck.MESSAGE_TEMPLATED,
+		IgnoredUnittestCheck.MESSAGE_IGNORED,
+		IgnoredUnittestCheck.MESSAGE_IGNORED,
+		IgnoredUnittestCheck.MESSAGE_IGNORED,
+	), sac);
+
+	stderr.writeln("Unittest for IgnoredUnittestCheck passed.");
+}

src/dscanner/analysis/run.d

@@ -81,6 +81,7 @@ import dscanner.analysis.trust_too_much;
 import dscanner.analysis.redundant_storage_class;
 import dscanner.analysis.unused_result;
 import dscanner.analysis.cyclomatic_complexity;
+import dscanner.analysis.ignored_unittest;
 
 import dsymbol.string_interning : internString;
 import dsymbol.scope_;
@@ -593,6 +594,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
 		analysisConfig.cyclomatic_complexity == Check.skipTests && !ut,
 		analysisConfig.max_cyclomatic_complexity.to!int);
 
+	if (moduleName.shouldRun!IgnoredUnittestCheck(analysisConfig))
+		checks ~= new IgnoredUnittestCheck(fileName,
+		analysisConfig.unused_result == Check.skipTests && !ut);
+
 	version (none)
 		if (moduleName.shouldRun!IfStatementCheck(analysisConfig))
 			checks ~= new IfStatementCheck(fileName, moduleScope,