From 9e04e595f78967c5dbbab912f233f0bd5c89810c Mon Sep 17 00:00:00 2001 From: Vladiwostok Date: Wed, 1 May 2024 15:08:23 +0300 Subject: [PATCH] Replace libdparse with DMD in BodyOnDisabledFuncsCheck --- .../analysis/body_on_disabled_funcs.d | 186 +++++------------- src/dscanner/analysis/run.d | 10 +- 2 files changed, 60 insertions(+), 136 deletions(-) diff --git a/src/dscanner/analysis/body_on_disabled_funcs.d b/src/dscanner/analysis/body_on_disabled_funcs.d index c6476a84..a21ff7a5 100644 --- a/src/dscanner/analysis/body_on_disabled_funcs.d +++ b/src/dscanner/analysis/body_on_disabled_funcs.d @@ -1,143 +1,78 @@ module dscanner.analysis.body_on_disabled_funcs; import dscanner.analysis.base; -import dparse.ast; -import dparse.lexer; -import dsymbol.scope_; -import std.meta : AliasSeq; +import dmd.astenums : STC; -final class BodyOnDisabledFuncsCheck : BaseAnalyzer +extern (C++) class BodyOnDisabledFuncsCheck(AST) : BaseAnalyzerDmd { - alias visit = BaseAnalyzer.visit; - + alias visit = BaseAnalyzerDmd.visit; mixin AnalyzerInfo!"body_on_disabled_func_check"; - this(BaseAnalyzerArguments args) - { - super(args); - } + private enum string KEY = "dscanner.confusing.disabled_function_with_body"; + private enum FUNC_MSG = "Function marked with '@disabled' should not have a body"; + private enum CTOR_MSG = "Constructor marked with '@disabled' should not have a body"; + private enum DTOR_MSG = "Destructor marked with '@disabled' should not have a body"; - 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; - } + private bool isDisabled; - override void visit(const Declaration dec) + extern (D) this(string fileName, bool skipTests = false) { - foreach (attr; dec.attributes) - { - if (attr.atAttribute !is null && attr.atAttribute.identifier.text == "disable") { - // found attr block w. disable: dec.constructor - scope wasDisabled = isDisabled; - isDisabled = true; - visitDeclarationInner(dec); - dec.accept(this); - isDisabled = wasDisabled; - return; - } - } + super(fileName, skipTests); + } - visitDeclarationInner(dec); - scope wasDisabled = isDisabled; - dec.accept(this); + override void visit(AST.StorageClassDeclaration storageClassDecl) + { + bool wasDisabled = isDisabled; + isDisabled = (storageClassDecl.stc & STC.disable) != 0; + super.visit(storageClassDecl); isDisabled = wasDisabled; } -private: - bool isDisabled = false; + mixin VisitAggregate!(AST.ClassDeclaration); + mixin VisitAggregate!(AST.InterfaceDeclaration); + mixin VisitAggregate!(AST.StructDeclaration); + mixin VisitAggregate!(AST.UnionDeclaration); - bool isDeclDisabled(T)(const T dec) + private template VisitAggregate(NodeType) { - // `@disable { ... }` - if (isDisabled) - return true; - - static if (__traits(hasMember, T, "storageClasses")) + override void visit(NodeType node) { - // `@disable doThing() {}` - if (hasDisabledStorageclass(dec.storageClasses)) - return true; - } - - // `void doThing() @disable {}` - return hasDisabledMemberFunctionAttribute(dec.memberFunctionAttributes); - } + bool isCurrentDisabled = isDisabled || (node.storage_class & STC.disable) != 0; + if (isCurrentDisabled) + return; - void visitDeclarationInner(const Declaration dec) - { - if (dec.attributeDeclaration !is null - && dec.attributeDeclaration.attribute - && dec.attributeDeclaration.attribute.atAttribute - && dec.attributeDeclaration.attribute.atAttribute.identifier.text == "disable") - { - // 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.missingFunctionBody is null) - { - addErrorMessage(dec.functionDeclaration.functionBody, - 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.missingFunctionBody is null) - { - addErrorMessage(dec.constructor.functionBody, - 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.missingFunctionBody is null) - { - addErrorMessage(dec.destructor.functionBody, - KEY, "Destructor marked with '@disabled' should not have a body"); + bool wasDisabled = isDisabled; + isDisabled = false; + super.visit(node); + isDisabled = wasDisabled; } } - bool hasDisabledStorageclass(const(StorageClass[]) storageClasses) - { - foreach (sc; storageClasses) - { - if (sc.atAttribute !is null && sc.atAttribute.identifier.text == "disable") - return true; - } - return false; - } + mixin VisitFunction!(AST.FuncDeclaration, FUNC_MSG); + mixin VisitFunction!(AST.CtorDeclaration, CTOR_MSG); + mixin VisitFunction!(AST.DtorDeclaration, DTOR_MSG); - bool hasDisabledMemberFunctionAttribute(const(MemberFunctionAttribute[]) memberFunctionAttributes) + private template VisitFunction(NodeType, string MSG) { - foreach (attr; memberFunctionAttributes) + override void visit(NodeType node) { - if (attr.atAttribute !is null && attr.atAttribute.identifier.text == "disable") - return true; + bool isCurrentDisabled = isDisabled || (node.storage_class & STC.disable) != 0; + if (isCurrentDisabled && node.fbody !is null) + addErrorMessage(cast(ulong) node.loc.linnum, cast(ulong) node.loc.charnum, KEY, MSG); } - return false; } - - enum string KEY = "dscanner.confusing.disabled_function_with_body"; } unittest { import std.stdio : stderr; - import std.format : format; import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - import dscanner.analysis.helpers : assertAnalyzerWarnings; + import dscanner.analysis.helpers : assertAnalyzerWarningsDMD; StaticAnalysisConfig sac = disabledConfig(); sac.body_on_disabled_func_check = Check.enabled; - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ class C1 { this() {} @@ -159,12 +94,9 @@ unittest } } - 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() {} // [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(); @@ -173,28 +105,18 @@ unittest 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() {} // [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(); } diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index be19b786..eac39600 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -870,10 +870,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName, checks ~= new UnusedResultChecker(args.setSkipTests( analysisConfig.unused_result == Check.skipTests && !ut)); - if (moduleName.shouldRun!BodyOnDisabledFuncsCheck(analysisConfig)) - checks ~= new BodyOnDisabledFuncsCheck(args.setSkipTests( - analysisConfig.body_on_disabled_func_check == Check.skipTests && !ut)); - return checks; } @@ -1356,6 +1352,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN config.could_be_immutable_check == Check.skipTests && !ut ); + if (moduleName.shouldRunDmd!(BodyOnDisabledFuncsCheck!ASTCodegen)(config)) + visitors ~= new BodyOnDisabledFuncsCheck!ASTCodegen( + fileName, + config.body_on_disabled_func_check == Check.skipTests && !ut + ); + foreach (visitor; visitors) { m.accept(visitor);