Adds an check for `@disable`d functions that have an body; closes #897

.dscanner.ini

@@ -97,3 +97,5 @@ unused_result="enabled"
 cyclomatic_complexity="disabled"
 ; Maximum cyclomatic complexity after which to issue warnings
 max_cyclomatic_complexity="50"
+; Check for function bodies on disabled functions
+body_on_disabled_func_check="enabled"

src/dscanner/analysis/body_on_disabled_funcs.d

@@ -0,0 +1,108 @@
+module dscanner.analysis.body_on_disabled_funcs;
+
+import dscanner.analysis.base;
+import dparse.ast;
+import dparse.lexer;
+import std.stdio;
+import dsymbol.scope_;
+
+final class BodyOnDisabledFuncsCheck : BaseAnalyzer
+{
+	alias visit = BaseAnalyzer.visit;
+
+	mixin AnalyzerInfo!"body_on_disabled_func_check";
+
+	this(string fileName, const(Scope)* sc, bool skipTests = false)
+	{
+		super(fileName, sc, skipTests);
+	}
+
+	override void visit(const Declaration dec)
+	{
+		foreach (attr; dec.attributes)
+		{
+			if (attr.atAttribute !is null && attr.atAttribute.identifier.text == "disable") {
+				writeln("found attr block w. disable: ", dec.constructor);
+				isDisabled = true;
+				visitDeclarationInner(dec);
+				dec.accept(this);
+				isDisabled = false;
+				return;
+			}
+		}
+
+		visitDeclarationInner(dec);
+		dec.accept(this);
+	}
+
+private:
+	bool isDisabled = false;
+
+	bool isDeclDisabled(T)(const T dec)
+	{
+		// `@disable { ... }`
+		if (isDisabled)
+			return true;
+
+		static if (__traits(hasMember, T, "storageClasses"))
+		{
+			// `@disable doThing() {}`
+			if (hasDisabledStorageclass(dec.storageClasses))
+				return true;
+		}
+
+		// `void doThing() @disable {}`
+		return hasDisabledMemberFunctionAttribute(dec.memberFunctionAttributes);
+	}
+
+	void visitDeclarationInner(const Declaration dec)
+	{
+		if (dec.attributeDeclaration !is null)
+		{
+			writeln("found attrdecl: ", dec.attributeDeclaration);
+		}
+		else if (dec.functionDeclaration !is null
+					&& isDeclDisabled(dec.functionDeclaration)
+					&& dec.functionDeclaration.functionBody !is null)
+		{
+			addErrorMessage(dec.functionDeclaration.name.line, dec.functionDeclaration.name.column,
+					KEY, "Function marked with '@disabled' should not have a body");
+		}
+		else if (dec.constructor !is null
+					&& isDeclDisabled(dec.constructor)
+					&& dec.constructor.functionBody !is null)
+		{
+			addErrorMessage(dec.constructor.line, dec.constructor.column,
+					KEY, "Constructor marked with '@disabled' should not have a body");
+		}
+		else if (dec.destructor !is null
+					&& isDeclDisabled(dec.destructor)
+					&& dec.destructor.functionBody !is null)
+		{
+			addErrorMessage(dec.destructor.line, dec.destructor.column,
+					KEY, "Destructor marked with '@disabled' should not have a body");
+		}
+	}
+
+	bool hasDisabledStorageclass(const(StorageClass[]) storageClasses)
+	{
+		foreach (sc; storageClasses)
+		{
+			if (sc.atAttribute !is null && sc.atAttribute.identifier.text == "disable")
+				return true;
+		}
+		return false;
+	}
+
+	bool hasDisabledMemberFunctionAttribute(const(MemberFunctionAttribute[]) memberFunctionAttributes)
+	{
+		foreach (attr; memberFunctionAttributes)
+		{
+			if (attr.atAttribute !is null && attr.atAttribute.identifier.text == "disable")
+				return true;
+		}
+		return false;
+	}
+
+	enum string KEY = "dscanner.confusing.disabled_function_with_body";
+}

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 function bodies on disabled functions")
+	string body_on_disabled_func_check = Check.enabled;
+
 	@INI("Module-specific filters")
 	ModuleFilters filters;
 }

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.body_on_disabled_funcs;
 
 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!BodyOnDisabledFuncsCheck(analysisConfig))
+		checks ~= new BodyOnDisabledFuncsCheck(fileName, moduleScope,
+		analysisConfig.body_on_disabled_func_check == Check.skipTests && !ut);
+
 	version (none)
 		if (moduleName.shouldRun!IfStatementCheck(analysisConfig))
 			checks ~= new IfStatementCheck(fileName, moduleScope,