add end line/column to warnings

dub.json

@@ -11,7 +11,7 @@
     "built_with_dub"
   ],
   "dependencies": {
-    "libdparse": ">=0.23.0 <0.24.0",
+    "libdparse": ">=0.23.1 <0.24.0",
     "dcd:dsymbol": ">=0.16.0-beta.2 <0.17.0",
     "inifiled": "~>1.3.1",
     "emsi_containers": "~>0.9.0",

dub.selections.json

@@ -6,7 +6,7 @@
 		"emsi_containers": "0.9.0",
 		"inifiled": "1.3.3",
 		"libddoc": "0.8.0",
-		"libdparse": "0.23.0",
+		"libdparse": "0.23.1",
 		"stdx-allocator": "2.77.5"
 	}
 }

src/dscanner/analysis/alias_syntax_check.d

@@ -29,8 +29,7 @@ final class AliasSyntaxCheck : BaseAnalyzer
 			return;
 		assert(ad.declaratorIdentifierList.identifiers.length > 0,
 				"Identifier list length is zero, libdparse has a bug");
-		addErrorMessage(ad.declaratorIdentifierList.identifiers[0].line,
-				ad.declaratorIdentifierList.identifiers[0].column, KEY,
+		addErrorMessage(ad, KEY,
 				"Prefer the new \"'alias' identifier '=' type ';'\" syntax"
 				~ " to the  old \"'alias' type identifier ';'\" syntax.");
 	}
@@ -48,7 +47,8 @@ unittest
 	StaticAnalysisConfig sac = disabledConfig();
 	sac.alias_syntax_check = Check.enabled;
 	assertAnalyzerWarnings(q{
-		alias int abcde; // [warn]: Prefer the new "'alias' identifier '=' type ';'" syntax to the  old "'alias' type identifier ';'" syntax.
+		alias int abcde; /+
+		^^^^^^^^^^^^^^^^ [warn]: Prefer the new "'alias' identifier '=' type ';'" syntax to the  old "'alias' type identifier ';'" syntax.+/
 		alias abcde = int;
 	}c, sac);
 

src/dscanner/analysis/allman.d

@@ -47,7 +47,7 @@ final class AllManCheck : BaseAnalyzer
 					continue;
 				// ignore inline { } braces
 				if (curLine != tokens[i + 1].line)
-					addErrorMessage(tokens[i].line, tokens[i].column, KEY, MESSAGE);
+					addErrorMessage(tokens[i], KEY, MESSAGE);
 			}
 			if (tokens[i].type == tok!"}" && curLine == prevTokenLine)
 			{
@@ -56,7 +56,7 @@ final class AllManCheck : BaseAnalyzer
 					continue;
 				// ignore inline { } braces
 				if (!tokens[0 .. i].retro.until!(t => t.line != curLine).canFind!(t => t.type == tok!"{"))
-					addErrorMessage(tokens[i].line, tokens[i].column, KEY, MESSAGE);
+					addErrorMessage(tokens[i], KEY, MESSAGE);
 			}
 		}
 	}
@@ -79,7 +79,8 @@ unittest
 	assertAnalyzerWarnings(q{
 		void testAllman()
 		{
-			while (true) { // [warn]: %s
+			while (true) { /+
+			             ^ [warn]: %s +/
 				auto f = 1;
 			}
 

src/dscanner/analysis/asm_style.d

@@ -32,7 +32,7 @@ final class AsmStyleCheck : BaseAnalyzer
 		if (brExp.asmBrExp !is null && brExp.asmBrExp.asmUnaExp !is null
 				&& brExp.asmBrExp.asmUnaExp.asmPrimaryExp !is null)
 		{
-			addErrorMessage(brExp.line, brExp.column, "dscanner.confusing.brexp",
+			addErrorMessage(brExp, "dscanner.confusing.brexp",
 					"This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.");
 		}
 		brExp.accept(this);
@@ -50,7 +50,8 @@ unittest
 		{
 			asm
 			{
-				mov a, someArray[1]; // [warn]: This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.
+				mov a, someArray[1]; /+
+				       ^^^^^^^^^^^^ [warn]: This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify. +/
 				add near ptr [EAX], 3;
 			}
 		}

src/dscanner/analysis/assert_without_msg.d

@@ -41,7 +41,7 @@ final class AssertWithoutMessageCheck : BaseAnalyzer
 				&& expr.assertArguments.message !is null;
 
 		if (!hasMessage)
-			addErrorMessage(expr.line, expr.column, KEY, MESSAGE);
+			addErrorMessage(expr, KEY, MESSAGE);
 	}
 
 	override void visit(const FunctionCallExpression expr)
@@ -55,7 +55,7 @@ final class AssertWithoutMessageCheck : BaseAnalyzer
 			auto ident = iot.identifier;
 			if (ident.text == "enforce" && expr.arguments !is null && expr.arguments.namedArgumentList !is null &&
 					expr.arguments.namedArgumentList.items.length < 2)
-				addErrorMessage(ident.line, ident.column, KEY, MESSAGE);
+				addErrorMessage(expr, KEY, MESSAGE);
 		}
 	}
 
@@ -112,7 +112,8 @@ unittest
 	assertAnalyzerWarnings(q{
 	unittest {
 		assert(0, "foo bar");
-		assert(0); // [warn]: %s
+		assert(0); /+
+		^^^^^^^^^ [warn]: %s +/
 	}
 	}c.format(
 		AssertWithoutMessageCheck.MESSAGE,
@@ -121,7 +122,8 @@ unittest
 	assertAnalyzerWarnings(q{
 	unittest {
 		static assert(0, "foo bar");
-		static assert(0); // [warn]: %s
+		static assert(0); /+
+		       ^^^^^^^^^ [warn]: %s +/
 	}
 	}c.format(
 		AssertWithoutMessageCheck.MESSAGE,
@@ -133,7 +135,8 @@ unittest
 		enforce(0); // std.exception not imported yet - could be a user-defined symbol
 		import std.exception;
 		enforce(0, "foo bar");
-		enforce(0); // [warn]: %s
+		enforce(0); /+
+		^^^^^^^^^^ [warn]: %s +/
 	}
 	}c.format(
 		AssertWithoutMessageCheck.MESSAGE,

src/dscanner/analysis/auto_function.d

@@ -11,7 +11,7 @@ import dparse.ast;
 import dparse.lexer;
 
 import std.stdio;
-import std.algorithm.searching : any;
+import std.algorithm : map, filter;
 
 /**
  * Checks for auto functions without return statement.
@@ -26,7 +26,8 @@ final class AutoFunctionChecker : BaseAnalyzer
 private:
 
 	enum string KEY = "dscanner.suspicious.missing_return";
-	enum string MESSAGE = "Auto function without return statement, prefer an explicit void";
+	enum string MESSAGE = "Auto function without return statement, prefer replacing auto with void";
+	enum string MESSAGE_INSERT = "Auto function without return statement, prefer inserting void to be explicit";
 
 	bool[] _returns;
 	size_t _mixinDepth;
@@ -44,19 +45,41 @@ public:
 		super(fileName, null, skipTests);
 	}
 
+	package static const(Token)[] findAutoReturnType(const(FunctionDeclaration) decl)
+	{
+		auto autoFunTokens = decl.storageClasses
+			.map!(a => a.token.type == tok!"auto"
+					? [a.token]
+					: a.atAttribute
+						? a.atAttribute.tokens
+						: null)
+			.filter!(a => a.length > 0);
+		return autoFunTokens.empty ? null : autoFunTokens.front;
+	}
+
 	override void visit(const(FunctionDeclaration) decl)
 	{
 		_returns.length += 1;
 		scope(exit) _returns.length -= 1;
 		_returns[$-1] = false;
 
-		const bool autoFun = decl.storageClasses
-			.any!(a => a.token.type == tok!"auto" || a.atAttribute !is null);
+		auto autoTokens = findAutoReturnType(decl);
+		bool isAtAttribute = autoTokens.length > 1;
 
 		decl.accept(this);
 
-		if (decl.functionBody.specifiedFunctionBody && autoFun && !_returns[$-1])
-			addErrorMessage(decl.name.line, decl.name.column, KEY, MESSAGE);
+		if (decl.functionBody.specifiedFunctionBody && autoTokens.length && !_returns[$-1])
+		{
+			if (isAtAttribute)
+			{
+				// highlight on the whitespace between attribute and function name
+				auto tok = autoTokens[$ - 1];
+				auto whitespace = tok.column + (tok.text.length ? tok.text.length : str(tok.type).length);
+				addErrorMessage(tok.line, whitespace, whitespace + 1, KEY, MESSAGE_INSERT);
+			}
+			else
+				addErrorMessage(autoTokens, KEY, MESSAGE);
+		}
 	}
 
 	override void visit(const(ReturnStatement) rst)
@@ -165,9 +188,12 @@ unittest
 	StaticAnalysisConfig sac = disabledConfig();
 	sac.auto_function_check = Check.enabled;
 	assertAnalyzerWarnings(q{
-		auto ref doStuff(){} // [warn]: %s
-		auto doStuff(){} // [warn]: %s
-		int doStuff(){auto doStuff(){}} // [warn]: %s
+		auto ref doStuff(){} /+
+		^^^^ [warn]: %s +/
+		auto doStuff(){} /+
+		^^^^ [warn]: %s +/
+		int doStuff(){auto doStuff(){}} /+
+		              ^^^^ [warn]: %s +/
 		auto doStuff(){return 0;}
 		int doStuff(){/*error but not the aim*/}
 	}c.format(
@@ -177,55 +203,63 @@ unittest
 	), sac);
 
 	assertAnalyzerWarnings(q{
-		auto doStuff(){assert(true);} // [warn]: %s
+		auto doStuff(){assert(true);} /+
+		^^^^ [warn]: %s +/
 		auto doStuff(){assert(false);}
 	}c.format(
 		AutoFunctionChecker.MESSAGE,
 	), sac);
 
 	assertAnalyzerWarnings(q{
-		auto doStuff(){assert(1);} // [warn]: %s
+		auto doStuff(){assert(1);} /+
+		^^^^ [warn]: %s +/
 		auto doStuff(){assert(0);}
 	}c.format(
 		AutoFunctionChecker.MESSAGE,
 	), sac);
 
 	assertAnalyzerWarnings(q{
-		auto doStuff(){mixin("0+0");} // [warn]: %s
+		auto doStuff(){mixin("0+0");} /+
+		^^^^ [warn]: %s +/
 		auto doStuff(){mixin("return 0;");}
 	}c.format(
 		AutoFunctionChecker.MESSAGE,
 	), sac);
 
 	assertAnalyzerWarnings(q{
-		auto doStuff(){mixin("0+0");} // [warn]: %s
+		auto doStuff(){mixin("0+0");} /+
+		^^^^ [warn]: %s +/
 		auto doStuff(){mixin("static if (true)" ~ "  return " ~ 0.stringof ~ ";");}
 	}c.format(
 		AutoFunctionChecker.MESSAGE,
 	), sac);
 
 	assertAnalyzerWarnings(q{
-		auto doStuff(){} // [warn]: %s
+		auto doStuff(){} /+
+		^^^^ [warn]: %s +/
 		extern(C) auto doStuff();
 	}c.format(
 		AutoFunctionChecker.MESSAGE,
 	), sac);
 
 	assertAnalyzerWarnings(q{
-		auto doStuff(){} // [warn]: %s
+		auto doStuff(){} /+
+		^^^^ [warn]: %s +/
 		@disable auto doStuff();
 	}c.format(
 		AutoFunctionChecker.MESSAGE,
 	), sac);
 
 	assertAnalyzerWarnings(q{
-		@property doStuff(){} // [warn]: %s
-		@safe doStuff(){} // [warn]: %s
+		@property doStuff(){} /+
+		         ^ [warn]: %s +/
+		@safe doStuff(){} /+
+		     ^ [warn]: %s +/
 		@disable doStuff();
 		@safe void doStuff();
 	}c.format(
-		AutoFunctionChecker.MESSAGE,
-		AutoFunctionChecker.MESSAGE,
+		AutoFunctionChecker.MESSAGE_INSERT,
+		AutoFunctionChecker.MESSAGE_INSERT,
 	), sac);
 
 	assertAnalyzerWarnings(q{

src/dscanner/analysis/auto_ref_assignment.d

@@ -54,29 +54,29 @@ final class AutoRefAssignmentCheck : BaseAnalyzer
 	{
 		if (assign.operator == tok!"" || scopes.length == 0)
 			return;
-		interest++;
+		interest ~= assign;
 		assign.ternaryExpression.accept(this);
-		interest--;
+		interest.length--;
 	}
 
 	override void visit(const IdentifierOrTemplateInstance ioti)
 	{
 		import std.algorithm.searching : canFind;
 
-		if (ioti.identifier == tok!"" || interest <= 0)
+		if (ioti.identifier == tok!"" || !interest.length)
 			return;
 		if (scopes[$ - 1].canFind(ioti.identifier.text))
-			addErrorMessage(ioti.identifier.line, ioti.identifier.column, KEY, MESSAGE);
+			addErrorMessage(interest[$ - 1], KEY, MESSAGE);
 	}
 
 	override void visit(const IdentifierChain ic)
 	{
 		import std.algorithm.searching : canFind;
 
-		if (ic.identifiers.length == 0 || interest <= 0)
+		if (ic.identifiers.length == 0 || !interest.length)
 			return;
 		if (scopes[$ - 1].canFind(ic.identifiers[0].text))
-			addErrorMessage(ic.identifiers[0].line, ic.identifiers[0].column, KEY, MESSAGE);
+			addErrorMessage(interest[$ - 1], KEY, MESSAGE);
 	}
 
 	alias visit = BaseAnalyzer.visit;
@@ -86,12 +86,7 @@ private:
 	enum string MESSAGE = "Assignment to auto-ref function parameter.";
 	enum string KEY = "dscanner.suspicious.auto_ref_assignment";
 
-	invariant
-	{
-		assert(interest >= 0);
-	}
-
-	int interest;
+	const(AssignExpression)[] interest;
 
 	void addSymbol(string symbolName)
 	{
@@ -123,7 +118,8 @@ unittest
 	assertAnalyzerWarnings(q{
 		int doStuff(T)(auto ref int a)
 		{
-			a = 10; // [warn]: %s
+			a = 10; /+
+			^^^^^^ [warn]: %s +/
 		}
 
 		int doStuff(T)(ref int a)