From e73a8a88605408b97df94867ea44e411c3f2c1a0 Mon Sep 17 00:00:00 2001 From: lucica28 <57060141+lucica28@users.noreply.github.com> Date: Mon, 29 May 2023 13:47:43 +0300 Subject: [PATCH] replace libdparse in trust_too_much visitor (#70) --- src/dscanner/analysis/run.d | 10 +-- src/dscanner/analysis/trust_too_much.d | 96 ++++++++------------------ 2 files changed, 33 insertions(+), 73 deletions(-) diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index 00f1831..97a5598 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -917,10 +917,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName, checks ~= new IfConstraintsIndentCheck(args.setSkipTests( analysisConfig.if_constraints_indent == Check.skipTests && !ut)); - if (moduleName.shouldRun!TrustTooMuchCheck(analysisConfig)) - checks ~= new TrustTooMuchCheck(args.setSkipTests( - analysisConfig.trust_too_much == Check.skipTests && !ut)); - if (moduleName.shouldRun!RedundantStorageClassCheck(analysisConfig)) checks ~= new RedundantStorageClassCheck(args.setSkipTests( analysisConfig.redundant_storage_classes == Check.skipTests && !ut)); @@ -1285,6 +1281,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN fileName, config.opequals_tohash_check == Check.skipTests && !ut ); + + if (moduleName.shouldRunDmd!(TrustTooMuchCheck!ASTCodegen)(config)) + visitors ~= new TrustTooMuchCheck!ASTCodegen( + fileName, + config.trust_too_much == Check.skipTests && !ut + ); if (moduleName.shouldRunDmd!(AutoRefAssignmentCheck!ASTCodegen)(config)) visitors ~= new AutoRefAssignmentCheck!ASTCodegen(fileName); diff --git a/src/dscanner/analysis/trust_too_much.d b/src/dscanner/analysis/trust_too_much.d index c964826..e589136 100644 --- a/src/dscanner/analysis/trust_too_much.d +++ b/src/dscanner/analysis/trust_too_much.d @@ -5,103 +5,65 @@ module dscanner.analysis.trust_too_much; -import std.stdio; -import dparse.ast; -import dparse.lexer; import dscanner.analysis.base; -import dsymbol.scope_; +import dmd.astenums : STC; /** * Checks that `@trusted` is only applied to a a single function */ -final class TrustTooMuchCheck : BaseAnalyzer +extern(C++) class TrustTooMuchCheck(AST) : BaseAnalyzerDmd { + mixin AnalyzerInfo!"trust_too_much"; + alias visit = BaseAnalyzerDmd.visit; + private: - - static immutable MESSAGE = "Trusting a whole scope is a bad idea, " ~ + extern(D) static immutable MESSAGE = "Trusting a whole scope is a bad idea, " ~ "`@trusted` should only be attached to the functions individually"; - static immutable string KEY = "dscanner.trust_too_much"; - - bool checkAtAttribute = true; + extern(D) static immutable string KEY = "dscanner.trust_too_much"; public: - - alias visit = BaseAnalyzer.visit; - - mixin AnalyzerInfo!"trust_too_much"; - /// - this(BaseAnalyzerArguments args) + extern(D) this(string fileName, bool skipTests = false) { - super(args); + super(fileName, skipTests); } - override void visit(const AtAttribute d) + override void visit(AST.StorageClassDeclaration scd) { - if (checkAtAttribute && d.identifier.text == "trusted") - addErrorMessage(d, KEY, MESSAGE); - d.accept(this); - } - - // always applied to function body, so OK - override void visit(const MemberFunctionAttribute d) - { - const oldCheckAtAttribute = checkAtAttribute; - checkAtAttribute = false; - d.accept(this); - checkAtAttribute = oldCheckAtAttribute; - } - - // handles `@trusted{}` and old style, leading, atAttribute for single funcs - override void visit(const Declaration d) - { - const oldCheckAtAttribute = checkAtAttribute; - - checkAtAttribute = d.functionDeclaration is null && d.unittest_ is null && - d.constructor is null && d.destructor is null && - d.staticConstructor is null && d.staticDestructor is null && - d.sharedStaticConstructor is null && d.sharedStaticDestructor is null; - d.accept(this); - checkAtAttribute = oldCheckAtAttribute; - } - - // issue #588 - override void visit(const AliasDeclaration d) - { - const oldCheckAtAttribute = checkAtAttribute; - checkAtAttribute = false; - d.accept(this); - checkAtAttribute = oldCheckAtAttribute; + if (scd.stc & STC.trusted) + addErrorMessage(cast(ulong) scd.loc.linnum, cast(ulong) scd.loc.charnum, + KEY, MESSAGE); + + super.visit(scd); } } unittest { import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - import dscanner.analysis.helpers : assertAnalyzerWarnings; + import dscanner.analysis.helpers : assertAnalyzerWarnings = assertAnalyzerWarningsDMD; import std.format : format; + import std.stdio : stderr; StaticAnalysisConfig sac = disabledConfig(); sac.trust_too_much = Check.enabled; - const msg = TrustTooMuchCheck.MESSAGE; + const msg = "Trusting a whole scope is a bad idea, " ~ + "`@trusted` should only be attached to the functions individually"; //--- fail cases ---// assertAnalyzerWarnings(q{ - @trusted: /+ - ^^^^^^^^ [warn]: %s +/ + @trusted: // [warn]: %s void test(); }c.format(msg), sac); assertAnalyzerWarnings(q{ - @trusted @nogc: /+ - ^^^^^^^^ [warn]: %s +/ + @trusted @nogc: // [warn]: %s void test(); }c.format(msg), sac); assertAnalyzerWarnings(q{ - @trusted { /+ - ^^^^^^^^ [warn]: %s +/ + @trusted { // [warn]: %s void test(); void test(); } @@ -109,31 +71,27 @@ unittest assertAnalyzerWarnings(q{ @safe { - @trusted @nogc { /+ - ^^^^^^^^ [warn]: %s +/ + @trusted @nogc { // [warn]: %s void test(); void test(); }} }c.format(msg), sac); assertAnalyzerWarnings(q{ - @nogc @trusted { /+ - ^^^^^^^^ [warn]: %s +/ + @nogc @trusted { // [warn]: %s void test(); void test(); } }c.format(msg), sac); assertAnalyzerWarnings(q{ - @trusted template foo(){ /+ - ^^^^^^^^ [warn]: %s +/ + @trusted template foo(){ // [warn]: %s } }c.format(msg), sac); assertAnalyzerWarnings(q{ struct foo{ - @trusted: /+ - ^^^^^^^^ [warn]: %s +/ + @trusted: // [warn]: %s } }c.format(msg), sac); //--- pass cases ---// @@ -161,4 +119,4 @@ unittest }c , sac); stderr.writeln("Unittest for TrustTooMuchCheck passed."); -} +} \ No newline at end of file