diff --git a/.dscanner.ini b/.dscanner.ini index 46cf36f..f352f6b 100644 --- a/.dscanner.ini +++ b/.dscanner.ini @@ -98,4 +98,4 @@ 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" \ No newline at end of file +body_on_disabled_func_check="enabled" diff --git a/src/dscanner/analysis/body_on_disabled_funcs.d b/src/dscanner/analysis/body_on_disabled_funcs.d index db20c4a..6d4248c 100644 --- a/src/dscanner/analysis/body_on_disabled_funcs.d +++ b/src/dscanner/analysis/body_on_disabled_funcs.d @@ -3,8 +3,8 @@ module dscanner.analysis.body_on_disabled_funcs; import dscanner.analysis.base; import dparse.ast; import dparse.lexer; -import std.stdio; import dsymbol.scope_; +import std.meta : AliasSeq; final class BodyOnDisabledFuncsCheck : BaseAnalyzer { @@ -17,22 +17,35 @@ final class BodyOnDisabledFuncsCheck : BaseAnalyzer super(fileName, sc, skipTests); } + static foreach (AggregateType; AliasSeq!(InterfaceDeclaration, ClassDeclaration, + StructDeclaration, UnionDeclaration, FunctionDeclaration)) + override void visit(const AggregateType t) + { + scope wasDisabled = isDisabled; + isDisabled = false; + t.accept(this); + isDisabled = wasDisabled; + } + 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); + // found attr block w. disable: dec.constructor + scope wasDisabled = isDisabled; isDisabled = true; visitDeclarationInner(dec); dec.accept(this); - isDisabled = false; + isDisabled = wasDisabled; return; } } visitDeclarationInner(dec); + scope wasDisabled = isDisabled; dec.accept(this); + isDisabled = wasDisabled; } private: @@ -57,27 +70,34 @@ private: void visitDeclarationInner(const Declaration dec) { - if (dec.attributeDeclaration !is null) + if (dec.attributeDeclaration !is null + && dec.attributeDeclaration.attribute + && dec.attributeDeclaration.attribute.atAttribute + && dec.attributeDeclaration.attribute.atAttribute.identifier.text == "disable") { - writeln("found attrdecl: ", dec.attributeDeclaration); + // found `@disable:`, so all code in this block is now disabled + isDisabled = true; } else if (dec.functionDeclaration !is null && isDeclDisabled(dec.functionDeclaration) - && dec.functionDeclaration.functionBody !is null) + && dec.functionDeclaration.functionBody !is null + && dec.functionDeclaration.functionBody.missingFunctionBody 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) + && dec.constructor.functionBody !is null + && dec.constructor.functionBody.missingFunctionBody 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) + && dec.destructor.functionBody !is null + && dec.destructor.functionBody.missingFunctionBody is null) { addErrorMessage(dec.destructor.line, dec.destructor.column, KEY, "Destructor marked with '@disabled' should not have a body"); @@ -105,4 +125,78 @@ private: } enum string KEY = "dscanner.confusing.disabled_function_with_body"; -} \ No newline at end of file +} + +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.body_on_disabled_func_check = Check.enabled; + + assertAnalyzerWarnings(q{ + class C1 + { + this() {} + void doThing() {} + ~this() {} + + this(); + void doThing(); + ~this(); + + @disable: + @disable + { + class UnaffectedSubClass + { + this() {} + void doThing() {} + ~this() {} + } + } + + this() {} // [warn]: Constructor marked with '@disabled' should not have a body + void doThing() {} // [warn]: Function marked with '@disabled' should not have a body + ~this() {} // [warn]: Destructor marked with '@disabled' should not have a body + + this(); + void doThing(); + ~this(); + } + + class C2 + { + @disable this() {} // [warn]: Constructor marked with '@disabled' should not have a body + @disable { this() {} } // [warn]: Constructor marked with '@disabled' should not have a body + this() @disable {} // [warn]: Constructor marked with '@disabled' should not have a body + + @disable void doThing() {} // [warn]: Function marked with '@disabled' should not have a body + @disable doThing() {} // [warn]: Function marked with '@disabled' should not have a body + @disable { void doThing() {} } // [warn]: Function marked with '@disabled' should not have a body + void doThing() @disable {} // [warn]: Function marked with '@disabled' should not have a body + + @disable ~this() {} // [warn]: Destructor marked with '@disabled' should not have a body + @disable { ~this() {} } // [warn]: Destructor marked with '@disabled' should not have a body + ~this() @disable {} // [warn]: Destructor marked with '@disabled' should not have a body + + @disable this(); + @disable { this(); } + this() @disable; + + @disable void doThing(); + // @disable doThing(); // this is invalid grammar! + @disable { void doThing(); } + void doThing() @disable; + + @disable ~this(); + @disable { ~this(); } + ~this() @disable; + } + }c, sac); + + stderr.writeln("Unittest for BodyOnDisabledFuncsCheck passed."); +}