From 1e8f1ec9e6fc7928f9594a4c61aeb69b548781e2 Mon Sep 17 00:00:00 2001 From: ricardaxel <46921637+ricardaxel@users.noreply.github.com> Date: Fri, 13 Oct 2023 02:45:59 +0200 Subject: [PATCH] Allow skipping checks with @("nolint(...)") and @nolint("...") (#936) Co-authored-by: Axel Ricard Co-authored-by: WebFreak001 --- src/dscanner/analysis/base.d | 51 +++- src/dscanner/analysis/nolint.d | 271 ++++++++++++++++++++ src/dscanner/analysis/style.d | 4 + src/dscanner/analysis/useless_initializer.d | 45 +++- 4 files changed, 368 insertions(+), 3 deletions(-) create mode 100644 src/dscanner/analysis/nolint.d diff --git a/src/dscanner/analysis/base.d b/src/dscanner/analysis/base.d index daa0736..110f902 100644 --- a/src/dscanner/analysis/base.d +++ b/src/dscanner/analysis/base.d @@ -1,7 +1,8 @@ module dscanner.analysis.base; import dparse.ast; -import dparse.lexer : IdType, str, Token; +import dparse.lexer : IdType, str, Token, tok; +import dscanner.analysis.nolint; import dsymbol.scope_ : Scope; import std.array; import std.container; @@ -405,6 +406,35 @@ public: unittest_.accept(this); } + /** + * Visits a module declaration. + * + * When overriden, make sure to keep this structure + */ + override void visit(const(Module) mod) + { + if (mod.moduleDeclaration !is null) + { + with (noLint.push(NoLintFactory.fromModuleDeclaration(mod.moduleDeclaration))) + mod.accept(this); + } + else + { + mod.accept(this); + } + } + + /** + * Visits a declaration. + * + * When overriden, make sure to disable and reenable error messages + */ + override void visit(const(Declaration) decl) + { + with (noLint.push(NoLintFactory.fromDeclaration(decl))) + decl.accept(this); + } + AutoFix.CodeReplacement[] resolveAutoFix( const Module mod, scope const(Token)[] tokens, @@ -423,6 +453,7 @@ protected: bool inAggregate; bool skipTests; + NoLint noLint; template visitTemplate(T) { @@ -437,42 +468,58 @@ protected: deprecated("Use the overload taking start and end locations or a Node instead") void addErrorMessage(size_t line, size_t column, string key, string message) { + if (noLint.containsCheck(key)) + return; _messages.insert(Message(fileName, line, column, key, message, getName())); } void addErrorMessage(const BaseNode node, string key, string message, AutoFix[] autofixes = null) { + if (noLint.containsCheck(key)) + return; addErrorMessage(Message.Diagnostic.from(fileName, node, message), key, autofixes); } void addErrorMessage(const Token token, string key, string message, AutoFix[] autofixes = null) { + if (noLint.containsCheck(key)) + return; addErrorMessage(Message.Diagnostic.from(fileName, token, message), key, autofixes); } void addErrorMessage(const Token[] tokens, string key, string message, AutoFix[] autofixes = null) { + if (noLint.containsCheck(key)) + return; addErrorMessage(Message.Diagnostic.from(fileName, tokens, message), key, autofixes); } void addErrorMessage(size_t[2] index, size_t line, size_t[2] columns, string key, string message, AutoFix[] autofixes = null) { + if (noLint.containsCheck(key)) + return; addErrorMessage(index, [line, line], columns, key, message, autofixes); } void addErrorMessage(size_t[2] index, size_t[2] lines, size_t[2] columns, string key, string message, AutoFix[] autofixes = null) { + if (noLint.containsCheck(key)) + return; auto d = Message.Diagnostic.from(fileName, index, lines, columns, message); _messages.insert(Message(d, key, getName(), autofixes)); } void addErrorMessage(Message.Diagnostic diagnostic, string key, AutoFix[] autofixes = null) { + if (noLint.containsCheck(key)) + return; _messages.insert(Message(diagnostic, key, getName(), autofixes)); } void addErrorMessage(Message.Diagnostic diagnostic, Message.Diagnostic[] supplemental, string key, AutoFix[] autofixes = null) { + if (noLint.containsCheck(key)) + return; _messages.insert(Message(diagnostic, supplemental, key, getName(), autofixes)); } @@ -756,7 +803,7 @@ unittest testScopes(q{ auto isNewScope = void; auto depth = 1; - + void foo() { isNewScope(); isOldScope(); diff --git a/src/dscanner/analysis/nolint.d b/src/dscanner/analysis/nolint.d new file mode 100644 index 0000000..4d2ab41 --- /dev/null +++ b/src/dscanner/analysis/nolint.d @@ -0,0 +1,271 @@ +module dscanner.analysis.nolint; + +@safe: + +import dparse.ast; +import dparse.lexer; + +import std.algorithm : canFind; +import std.regex : matchAll, regex; +import std.string : lastIndexOf, strip; +import std.typecons; + +struct NoLint +{ + bool containsCheck(scope const(char)[] check) const + { + while (true) + { + if (disabledChecks.get((() @trusted => cast(string) check)(), 0) > 0) + return true; + + auto dot = check.lastIndexOf('.'); + if (dot == -1) + break; + check = check[0 .. dot]; + } + return false; + } + + // automatic pop when returned value goes out of scope + Poppable push(in Nullable!NoLint other) scope + { + if (other.isNull) + return Poppable(null); + + foreach (key, value; other.get.getDisabledChecks) + this.disabledChecks[key] += value; + + return Poppable(() => this.pop(other)); + } + +package: + const(int[string]) getDisabledChecks() const + { + return this.disabledChecks; + } + + void pushCheck(in string check) + { + disabledChecks[check]++; + } + + void merge(in Nullable!NoLint other) + { + if (other.isNull) + return; + + foreach (key, value; other.get.getDisabledChecks) + this.disabledChecks[key] += value; + } + +private: + void pop(in Nullable!NoLint other) + { + if (other.isNull) + return; + + foreach (key, value; other.get.getDisabledChecks) + { + assert(this.disabledChecks.get(key, 0) >= value); + + this.disabledChecks[key] -= value; + } + } + + static struct Poppable + { + ~this() + { + if (onPop) + onPop(); + onPop = null; + } + + private: + void delegate() onPop; + } + + int[string] disabledChecks; +} + +struct NoLintFactory +{ + static Nullable!NoLint fromModuleDeclaration(in ModuleDeclaration moduleDeclaration) + { + NoLint noLint; + + foreach (atAttribute; moduleDeclaration.atAttributes) + noLint.merge(NoLintFactory.fromAtAttribute(atAttribute)); + + if (!noLint.getDisabledChecks.length) + return nullNoLint; + + return noLint.nullable; + } + + static Nullable!NoLint fromDeclaration(in Declaration declaration) + { + NoLint noLint; + foreach (attribute; declaration.attributes) + noLint.merge(NoLintFactory.fromAttribute(attribute)); + + if (!noLint.getDisabledChecks.length) + return nullNoLint; + + return noLint.nullable; + } + +private: + static Nullable!NoLint fromAttribute(const(Attribute) attribute) + { + if (attribute is null) + return nullNoLint; + + return NoLintFactory.fromAtAttribute(attribute.atAttribute); + + } + + static Nullable!NoLint fromAtAttribute(const(AtAttribute) atAttribute) + { + if (atAttribute is null) + return nullNoLint; + + auto ident = atAttribute.identifier; + auto argumentList = atAttribute.argumentList; + + if (argumentList !is null) + { + if (ident.text.length) + return NoLintFactory.fromStructUda(ident, argumentList); + else + return NoLintFactory.fromStringUda(argumentList); + + } + else + return nullNoLint; + } + + // @nolint("..") + static Nullable!NoLint fromStructUda(in Token ident, in ArgumentList argumentList) + in (ident.text.length && argumentList !is null) + { + if (ident.text != "nolint") + return nullNoLint; + + NoLint noLint; + + foreach (nodeExpr; argumentList.items) + { + if (auto unaryExpr = cast(const UnaryExpression) nodeExpr) + { + auto primaryExpression = unaryExpr.primaryExpression; + if (primaryExpression is null) + continue; + + if (primaryExpression.primary != tok!"stringLiteral") + continue; + + noLint.pushCheck(primaryExpression.primary.text.strip("\"")); + } + } + + if (!noLint.getDisabledChecks().length) + return nullNoLint; + + return noLint.nullable; + } + + // @("nolint(..)") + static Nullable!NoLint fromStringUda(in ArgumentList argumentList) + in (argumentList !is null) + { + NoLint noLint; + + foreach (nodeExpr; argumentList.items) + { + if (auto unaryExpr = cast(const UnaryExpression) nodeExpr) + { + auto primaryExpression = unaryExpr.primaryExpression; + if (primaryExpression is null) + continue; + + if (primaryExpression.primary != tok!"stringLiteral") + continue; + + auto str = primaryExpression.primary.text.strip("\""); + Nullable!NoLint currNoLint = NoLintFactory.fromString(str); + noLint.merge(currNoLint); + } + } + + if (!noLint.getDisabledChecks().length) + return nullNoLint; + + return noLint.nullable; + + } + + // Transform a string with form "nolint(abc, efg)" + // into a NoLint struct + static Nullable!NoLint fromString(in string str) + { + static immutable re = regex(`[\w-_.]+`, "g"); + auto matches = matchAll(str, re); + + if (!matches) + return nullNoLint; + + const udaName = matches.hit; + if (udaName != "nolint") + return nullNoLint; + + matches.popFront; + + NoLint noLint; + + while (matches) + { + noLint.pushCheck(matches.hit); + matches.popFront; + } + + if (!noLint.getDisabledChecks.length) + return nullNoLint; + + return noLint.nullable; + } + + static nullNoLint = Nullable!NoLint.init; +} + +unittest +{ + const s1 = "nolint(abc)"; + const s2 = "nolint(abc, efg, hij)"; + const s3 = " nolint ( abc , efg ) "; + const s4 = "nolint(dscanner.style.abc_efg-ijh)"; + const s5 = "OtherUda(abc)"; + const s6 = "nolint(dscanner)"; + + assert(NoLintFactory.fromString(s1).get.containsCheck("abc")); + + assert(NoLintFactory.fromString(s2).get.containsCheck("abc")); + assert(NoLintFactory.fromString(s2).get.containsCheck("efg")); + assert(NoLintFactory.fromString(s2).get.containsCheck("hij")); + + assert(NoLintFactory.fromString(s3).get.containsCheck("abc")); + assert(NoLintFactory.fromString(s3).get.containsCheck("efg")); + + assert(NoLintFactory.fromString(s4).get.containsCheck("dscanner.style.abc_efg-ijh")); + + assert(NoLintFactory.fromString(s5).isNull); + + assert(NoLintFactory.fromString(s6).get.containsCheck("dscanner")); + assert(!NoLintFactory.fromString(s6).get.containsCheck("dscanner2")); + assert(NoLintFactory.fromString(s6).get.containsCheck("dscanner.foo")); + + import std.stdio : stderr, writeln; + + (() @trusted => stderr.writeln("Unittest for NoLint passed."))(); +} diff --git a/src/dscanner/analysis/style.d b/src/dscanner/analysis/style.d index 2ffbbcb..b02e11c 100644 --- a/src/dscanner/analysis/style.d +++ b/src/dscanner/analysis/style.d @@ -14,6 +14,7 @@ import std.conv; import std.format; import dscanner.analysis.helpers; import dscanner.analysis.base; +import dscanner.analysis.nolint; import dsymbol.scope_ : Scope; final class StyleChecker : BaseAnalyzer @@ -33,6 +34,9 @@ final class StyleChecker : BaseAnalyzer override void visit(const ModuleDeclaration dec) { + with (noLint.push(NoLintFactory.fromModuleDeclaration(dec))) + dec.accept(this); + foreach (part; dec.moduleName.identifiers) { if (part.text.matchFirst(moduleNameRegex).length == 0) diff --git a/src/dscanner/analysis/useless_initializer.d b/src/dscanner/analysis/useless_initializer.d index c482db2..723f69e 100644 --- a/src/dscanner/analysis/useless_initializer.d +++ b/src/dscanner/analysis/useless_initializer.d @@ -5,6 +5,7 @@ module dscanner.analysis.useless_initializer; import dscanner.analysis.base; +import dscanner.analysis.nolint; import dscanner.utils : safeAccess; import containers.dynamicarray; import containers.hashmap; @@ -92,7 +93,10 @@ public: override void visit(const(Declaration) decl) { _inStruct.insert(decl.structDeclaration !is null); - decl.accept(this); + + with (noLint.push(NoLintFactory.fromDeclaration(decl))) + decl.accept(this); + if (_inStruct.length > 1 && _inStruct[$-2] && decl.constructor && ((decl.constructor.parameters && decl.constructor.parameters.parameters.length == 0) || !decl.constructor.parameters)) @@ -361,6 +365,45 @@ public: NotKnown nk = NotKnown.init; }, sac); + // passes + assertAnalyzerWarnings(q{ + @("nolint(dscanner.useless-initializer)") + int a = 0; + int a = 0; /+ + ^ [warn]: X +/ + + @("nolint(dscanner.useless-initializer)") + int f() { + int a = 0; + } + + struct nolint { string s; } + + @nolint("dscanner.useless-initializer") + int a = 0; + int a = 0; /+ + ^ [warn]: X +/ + + @("nolint(other_check, dscanner.useless-initializer, another_one)") + int a = 0; + + @nolint("other_check", "another_one", "dscanner.useless-initializer") + int a = 0; + + }, sac); + + // passes (disable check at module level) + assertAnalyzerWarnings(q{ + @("nolint(dscanner.useless-initializer)") + module my_module; + + int a = 0; + + int f() { + int a = 0; + } + }, sac); + stderr.writeln("Unittest for UselessInitializerChecker passed."); }