forks
/
D-Scanner
mirror of https://github.com/dlang-community/D-Scanner.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Replace libdparse with DMD in BodyOnDisabledFuncsCheck (#127) 

* Replace libdparse with DMD in BodyOnDisabledFuncsCheck

* Address feedback
This commit is contained in:
Vladiwostok 2024-08-08 12:09:28 +03:00 committed by Vladiwostok
parent c167ff0695
commit 8b5bc9fd9d
2 changed files with 57 additions and 135 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

178
src/dscanner/analysis/body_on_disabled_funcs.d
View File

@ -1,143 +1,76 @@
module dscanner.analysis.body_on_disabled_funcs; module dscanner.analysis.body_on_disabled_funcs;
import dscanner.analysis.base; import dscanner.analysis.base;
import dparse.ast; import dmd.astenums : STC;
import dparse.lexer;
import dsymbol.scope_;
import std.meta : AliasSeq;
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"; mixin AnalyzerInfo!"body_on_disabled_func_check";
this(BaseAnalyzerArguments 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";
private bool isDisabled;
extern (D) this(string fileName, bool skipTests = false)
{ {
super(args); super(fileName, skipTests);
} }
static foreach (AggregateType; AliasSeq!(InterfaceDeclaration, ClassDeclaration, override void visit(AST.StorageClassDeclaration storageClassDecl)
StructDeclaration, UnionDeclaration, FunctionDeclaration))
override void visit(const AggregateType t)
{ {
scope wasDisabled = isDisabled; bool wasDisabled = isDisabled;
isDisabled = false; isDisabled = (storageClassDecl.stc & STC.disable) != 0;
t.accept(this); super.visit(storageClassDecl);
isDisabled = wasDisabled; isDisabled = wasDisabled;
} }
override void visit(const Declaration dec) mixin VisitAggregate!(AST.ClassDeclaration);
mixin VisitAggregate!(AST.InterfaceDeclaration);
mixin VisitAggregate!(AST.StructDeclaration);
mixin VisitAggregate!(AST.UnionDeclaration);
private template VisitAggregate(NodeType)
{ {
foreach (attr; dec.attributes) override void visit(NodeType node)
{ {
if (attr.atAttribute !is null && attr.atAttribute.identifier.text == "disable") { if (isDisabled || (node.storage_class & STC.disable) != 0)
// found attr block w. disable: dec.constructor
scope wasDisabled = isDisabled;
isDisabled = true;
visitDeclarationInner(dec);
dec.accept(this);
isDisabled = wasDisabled;
return; return;
}
}
visitDeclarationInner(dec); bool wasDisabled = isDisabled;
scope wasDisabled = isDisabled; isDisabled = false;
dec.accept(this); super.visit(node);
isDisabled = wasDisabled; isDisabled = wasDisabled;
} }
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 {}` mixin VisitFunction!(AST.FuncDeclaration, FUNC_MSG);
return hasDisabledMemberFunctionAttribute(dec.memberFunctionAttributes); mixin VisitFunction!(AST.CtorDeclaration, CTOR_MSG);
} mixin VisitFunction!(AST.DtorDeclaration, DTOR_MSG);
void visitDeclarationInner(const Declaration dec) private template VisitFunction(NodeType, string MSG)
{ {
if (dec.attributeDeclaration !is null override void visit(NodeType node)
&& 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 if ((isDisabled || (node.storage_class & STC.disable) != 0) && node.fbody !is null)
isDisabled = true; addErrorMessage(cast(ulong) node.loc.linnum, cast(ulong) node.loc.charnum, KEY, MSG);
}
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 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";
} }
unittest unittest
{ {
import std.stdio : stderr; import std.stdio : stderr;
import std.format : format;
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings; import dscanner.analysis.helpers : assertAnalyzerWarningsDMD;
StaticAnalysisConfig sac = disabledConfig(); StaticAnalysisConfig sac = disabledConfig();
sac.body_on_disabled_func_check = Check.enabled; sac.body_on_disabled_func_check = Check.enabled;
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
class C1 class C1
{ {
this() {} this() {}
@ -159,12 +92,9 @@ unittest
} }
} }
this() {} /+ this() {} // [warn]: Constructor marked with '@disabled' should not have a body
^^ [warn]: Constructor marked with '@disabled' should not have a body +/ void doThing() {} // [warn]: Function marked with '@disabled' should not have a body
void doThing() {} /+ ~this() {} // [warn]: Destructor marked with '@disabled' should not have a body
^^ [warn]: Function marked with '@disabled' should not have a body +/
~this() {} /+
^^ [warn]: Destructor marked with '@disabled' should not have a body +/
this(); this();
void doThing(); void doThing();
@ -173,28 +103,18 @@ unittest
class C2 class C2
{ {
@disable this() {} /+ @disable this() {} // [warn]: Constructor marked with '@disabled' should not have a body
^^ [warn]: Constructor marked with '@disabled' should not have a body +/ @disable { this() {} } // [warn]: Constructor marked with '@disabled' should not have a body
@disable { this() {} } /+ this() @disable {} // [warn]: Constructor marked with '@disabled' should not have a body
^^ [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() {} /+ @disable void doThing() {} // [warn]: Function marked with '@disabled' should not have a body
^^ [warn]: Function marked with '@disabled' should not have a body +/ @disable doThing() {} // [warn]: Function marked with '@disabled' should not have a body
@disable doThing() {} /+ @disable { void doThing() {} } // [warn]: Function marked with '@disabled' should not have a body
^^ [warn]: Function marked with '@disabled' should not have a body +/ void doThing() @disable {} // [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() {} /+ @disable ~this() {} // [warn]: Destructor marked with '@disabled' should not have a body
^^ [warn]: Destructor marked with '@disabled' should not have a body +/ @disable { ~this() {} } // [warn]: Destructor marked with '@disabled' should not have a body
@disable { ~this() {} } /+ ~this() @disable {} // [warn]: Destructor marked with '@disabled' should not have a body
^^ [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();
@disable { this(); } @disable { this(); }

10
src/dscanner/analysis/run.d
View File

@ -870,10 +870,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
checks ~= new UnusedResultChecker(args.setSkipTests( checks ~= new UnusedResultChecker(args.setSkipTests(
analysisConfig.unused_result == Check.skipTests && !ut)); 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; return checks;
} }
@ -1356,6 +1352,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN
config.could_be_immutable_check == Check.skipTests && !ut 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) foreach (visitor; visitors)
{ {
m.accept(visitor); m.accept(visitor);