Allow skipping checks with @("nolint(...)") and @nolint("...") (#936)

Co-authored-by: Axel Ricard <contact@axelricard.fr>
Co-authored-by: WebFreak001 <gh@webfreak.org>

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));
 	}
 

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."))();
+}

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)

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.");
 }