add auto-fix API

src/dscanner/analysis/auto_function.d

@@ -76,10 +76,12 @@ public:
 				auto tok = autoTokens[$ - 1];
 				auto whitespace = tok.column + (tok.text.length ? tok.text.length : str(tok.type).length);
 				auto whitespaceIndex = tok.index + (tok.text.length ? tok.text.length : str(tok.type).length);
-				addErrorMessage([whitespaceIndex, whitespaceIndex + 1], tok.line, [whitespace, whitespace + 1], KEY, MESSAGE_INSERT);
+				addErrorMessage([whitespaceIndex, whitespaceIndex + 1], tok.line, [whitespace, whitespace + 1], KEY, MESSAGE_INSERT,
+					[AutoFix.insertionAt(whitespaceIndex + 1, "void ")]);
 			}
 			else
-				addErrorMessage(autoTokens, KEY, MESSAGE);
+				addErrorMessage(autoTokens, KEY, MESSAGE,
+					[AutoFix.replacement(autoTokens[0], "void")]);
 		}
 	}
 

src/dscanner/analysis/base.d

@@ -1,14 +1,159 @@
 module dscanner.analysis.base;
 
+import dparse.ast;
+import dparse.lexer : IdType, str, Token;
+import dsymbol.scope_ : Scope;
+import std.array;
 import std.container;
 import std.string;
-import dparse.ast;
+import std.sumtype;
-import std.array;
-import dsymbol.scope_ : Scope;
-import dparse.lexer : Token, str, IdType;
 
+///
+struct AutoFix
+{
+	///
+	struct CodeReplacement
+	{
+		/// Byte index `[start, end)` within the file what text to replace.
+		/// `start == end` if text is only getting inserted.
+		size_t[2] range;
+		/// The new text to put inside the range. (empty to delete text)
+		string newText;
+	}
+
+	/// Context that the analyzer resolve method can use to generate the
+	/// resolved `CodeReplacement` with.
+	struct ResolveContext
+	{
+		/// Arbitrary analyzer-defined parameters. May grow in the future with
+		/// more items.
+		ulong[3] params;
+		/// For dynamically sized data, may contain binary data.
+		string extraInfo;
+	}
+
+	/// Display name for the UI.
+	string name;
+	/// Either code replacements, sorted by range start, never overlapping, or a
+	/// context that can be passed to `BaseAnalyzer.resolveAutoFix` along with
+	/// the message key from the parent `Message` object.
+	///
+	/// `CodeReplacement[]` should be applied to the code in reverse, otherwise
+	/// an offset to the following start indices must be calculated and be kept
+	/// track of.
+	SumType!(CodeReplacement[], ResolveContext) autofix;
+
+	invariant
+	{
+		autofix.match!(
+			(const CodeReplacement[] replacement)
+			{
+				import std.algorithm : all, isSorted;
+
+				assert(replacement.all!"a.range[0] <= a.range[1]");
+				assert(replacement.isSorted!"a.range[0] < b.range[0]");
+			},
+			(_) {}
+		);
+	}
+
+	static AutoFix replacement(const Token token, string newText, string name = null)
+	{
+		if (!name.length)
+		{
+			auto text = token.text.length ? token.text : str(token.type);
+			if (newText.length)
+				name = "Replace `" ~ text ~ "` with `" ~ newText ~ "`";
+			else
+				name = "Remove `" ~ text ~ "`";
+		}
+		return replacement([token], newText, name);
+	}
+
+	static AutoFix replacement(const BaseNode node, string newText, string name)
+	{
+		return replacement(node.tokens, newText, name);
+	}
+
+	static AutoFix replacement(const Token[] tokens, string newText, string name)
+	in(tokens.length > 0, "must provide at least one token")
+	{
+		auto end = tokens[$ - 1].text.length ? tokens[$ - 1].text : str(tokens[$ - 1].type);
+		return replacement([tokens[0].index, tokens[$ - 1].index + end.length], newText, name);
+	}
+
+	static AutoFix replacement(size_t[2] range, string newText, string name)
+	{
+		AutoFix ret;
+		ret.name = name;
+		ret.autofix = [
+			AutoFix.CodeReplacement(range, newText)
+		];
+		return ret;
+	}
+
+	static AutoFix insertionBefore(const Token token, string content, string name = null)
+	{
+		return insertionAt(token.index, content, name);
+	}
+
+	static AutoFix insertionAfter(const Token token, string content, string name = null)
+	{
+		auto tokenText = token.text.length ? token.text : str(token.type);
+		return insertionAt(token.index + tokenText.length, content, name);
+	}
+
+	static AutoFix insertionAt(size_t index, string content, string name = null)
+	{
+		assert(content.length > 0, "generated auto fix inserting text without content");
+		AutoFix ret;
+		ret.name = name.length
+			? name
+			: content.strip.length
+				? "Insert `" ~ content.strip ~ "`"
+				: "Insert whitespace";
+		ret.autofix = [
+			AutoFix.CodeReplacement([index, index], content)
+		];
+		return ret;
+	}
+
+	AutoFix concat(AutoFix other) const
+	{
+		import std.algorithm : sort;
+
+		AutoFix ret;
+		ret.name = name;
+		CodeReplacement[] concatenated;
+		autofix.match!(
+			(const CodeReplacement[] replacement)
+			{
+				concatenated = replacement.dup;
+			},
+			_ => assert(false, "Cannot concatenate code replacement with late-resolve")
+		);
+		other.autofix.match!(
+			(const CodeReplacement[] concat)
+			{
+				concatenated ~= concat.dup;
+			},
+			_ => assert(false, "Cannot concatenate code replacement with late-resolve")
+		);
+		concatenated.sort!"a.range[0] < b.range[0]";
+		ret.autofix = concatenated;
+		return ret;
+	}
+}
+
+/// A diagnostic message. Each message defines one issue in the file, which
+/// consists of one or more squiggly line ranges within the code, as well as
+/// human readable descriptions and optionally also one or more automatic code
+/// fixes that can be applied.
 struct Message
 {
+	/// A squiggly line range within the code. May be the issue itself if it's
+	/// the `diagnostic` member or supplemental information that can aid the
+	/// user in resolving the issue.
 	struct Diagnostic
 	{
 		/// Name of the file where the warning was triggered.
@@ -22,8 +167,6 @@ struct Message
 		/// Warning message, may be null for supplemental diagnostics.
 		string message;
 
-		// TODO: add auto-fix suggestion API here
-
 		deprecated("Use startLine instead") alias line = startLine;
 		deprecated("Use startColumn instead") alias column = startColumn;
 
@@ -74,6 +217,10 @@ struct Message
 	/// Check name
 	string checkName;
 
+	/// Either immediate code changes that can be applied or context to call
+	/// the `BaseAnalyzer.resolveAutoFix` method with.
+	AutoFix[] autofixes;
+
 	deprecated this(string fileName, size_t line, size_t column, string key = null, string message = null, string checkName = null)
 	{
 		diagnostic.fileName = fileName;
@@ -84,19 +231,21 @@ struct Message
 		this.checkName = checkName;
 	}
 
-	this(Diagnostic diagnostic, string key = null, string checkName = null)
+	this(Diagnostic diagnostic, string key = null, string checkName = null, AutoFix[] autofixes = null)
 	{
 		this.diagnostic = diagnostic;
 		this.key = key;
 		this.checkName = checkName;
+		this.autofixes = autofixes;
 	}
 
-	this(Diagnostic diagnostic, Diagnostic[] supplemental, string key = null, string checkName = null)
+	this(Diagnostic diagnostic, Diagnostic[] supplemental, string key = null, string checkName = null, AutoFix[] autofixes = null)
 	{
 		this.diagnostic = diagnostic;
 		this.supplemental = supplemental;
 		this.key = key;
 		this.checkName = checkName;
+		this.autofixes = autofixes;
 	}
 
 	alias diagnostic this;
@@ -151,6 +300,20 @@ public:
 			unittest_.accept(this);
 	}
 
+	AutoFix.CodeReplacement[] resolveAutoFix(
+		const Module mod,
+		const(Token)[] tokens,
+		const Message message,
+		const AutoFix.ResolveContext context
+	)
+	{
+		cast(void) mod;
+		cast(void) tokens;
+		cast(void) message;
+		cast(void) context;
+		assert(0);
+	}
+
 protected:
 
 	bool inAggregate;
@@ -172,40 +335,40 @@ protected:
 		_messages.insert(Message(fileName, line, column, key, message, getName()));
 	}
 
-	void addErrorMessage(const BaseNode node, string key, string message)
+	void addErrorMessage(const BaseNode node, string key, string message, AutoFix[] autofixes = null)
 	{
-		addErrorMessage(Message.Diagnostic.from(fileName, node, message), key);
+		addErrorMessage(Message.Diagnostic.from(fileName, node, message), key, autofixes);
 	}
 
-	void addErrorMessage(const Token token, string key, string message)
+	void addErrorMessage(const Token token, string key, string message, AutoFix[] autofixes = null)
 	{
-		addErrorMessage(Message.Diagnostic.from(fileName, token, message), key);
+		addErrorMessage(Message.Diagnostic.from(fileName, token, message), key, autofixes);
 	}
 
-	void addErrorMessage(const Token[] tokens, string key, string message)
+	void addErrorMessage(const Token[] tokens, string key, string message, AutoFix[] autofixes = null)
 	{
-		addErrorMessage(Message.Diagnostic.from(fileName, tokens, message), key);
+		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)
+	void addErrorMessage(size_t[2] index, size_t line, size_t[2] columns, string key, string message, AutoFix[] autofixes = null)
 	{
-		addErrorMessage(index, [line, line], columns, key, message);
+		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)
+	void addErrorMessage(size_t[2] index, size_t[2] lines, size_t[2] columns, string key, string message, AutoFix[] autofixes = null)
 	{
 		auto d = Message.Diagnostic.from(fileName, index, lines, columns, message);
-		_messages.insert(Message(d, key, getName()));
+		_messages.insert(Message(d, key, getName(), autofixes));
 	}
 
-	void addErrorMessage(Message.Diagnostic diagnostic, string key)
+	void addErrorMessage(Message.Diagnostic diagnostic, string key, AutoFix[] autofixes = null)
 	{
-		_messages.insert(Message(diagnostic, key, getName()));
+		_messages.insert(Message(diagnostic, key, getName(), autofixes));
 	}
 
-	void addErrorMessage(Message.Diagnostic diagnostic, Message.Diagnostic[] supplemental, string key)
+	void addErrorMessage(Message.Diagnostic diagnostic, Message.Diagnostic[] supplemental, string key, AutoFix[] autofixes = null)
 	{
-		_messages.insert(Message(diagnostic, supplemental, key, getName()));
+		_messages.insert(Message(diagnostic, supplemental, key, getName(), autofixes));
 	}
 
 	/**

src/dscanner/analysis/del.d

@@ -28,7 +28,9 @@ final class DeleteCheck : BaseAnalyzer
 	override void visit(const DeleteExpression d)
 	{
 		addErrorMessage(d.tokens[0], "dscanner.deprecated.delete_keyword",
-				"Avoid using the 'delete' keyword.");
+				"Avoid using the 'delete' keyword.",
+				[AutoFix.replacement(d.tokens[0], `destroy(`, "Replace delete with destroy()")
+					.concat(AutoFix.insertionAfter(d.tokens[$ - 1], ")"))]);
 		d.accept(this);
 	}
 }

src/dscanner/analysis/duplicate_attribute.d

@@ -93,7 +93,8 @@ final class DuplicateAttributeCheck : BaseAnalyzer
 		if (hasAttribute)
 		{
 			string message = "Attribute '%s' is duplicated.".format(attributeName);
-			addErrorMessage(tokens, "dscanner.unnecessary.duplicate_attribute", message);
+			addErrorMessage(tokens, "dscanner.unnecessary.duplicate_attribute", message,
+				[AutoFix.replacement(tokens, "", "Remove second attribute " ~ attributeName)]);
 		}
 
 		// Mark it as having that attribute

src/dscanner/analysis/enumarrayliteral.d

@@ -8,7 +8,7 @@ module dscanner.analysis.enumarrayliteral;
 import dparse.ast;
 import dparse.lexer;
 import dscanner.analysis.base;
-import std.algorithm : canFind, map;
+import std.algorithm : find, map;
 import dsymbol.scope_ : Scope;
 
 void doNothing(string, size_t, size_t, string, bool)
@@ -35,7 +35,8 @@ final class EnumArrayLiteralCheck : BaseAnalyzer
 
 	override void visit(const AutoDeclaration autoDec)
 	{
-		if (autoDec.storageClasses.canFind!(a => a.token == tok!"enum"))
+		auto enumToken = autoDec.storageClasses.find!(a => a.token == tok!"enum");
+		if (enumToken.length)
 		{
 			foreach (part; autoDec.parts)
 			{
@@ -49,7 +50,10 @@ final class EnumArrayLiteralCheck : BaseAnalyzer
 						"dscanner.performance.enum_array_literal",
 						"This enum may lead to unnecessary allocation at run-time."
 						~ " Use 'static immutable "
-						~ part.identifier.text ~ " = [ ...' instead.");
+						~ part.identifier.text ~ " = [ ...' instead.",
+						[
+							AutoFix.replacement(enumToken[0].token, "static immutable")
+						]);
 			}
 		}
 		autoDec.accept(this);

src/dscanner/analysis/explicitly_annotated_unittests.d

@@ -44,7 +44,14 @@ final class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer
 				}
 			}
 			if (!isSafeOrSystem)
-				addErrorMessage(decl.unittest_.findTokenForDisplay(tok!"unittest"), KEY, MESSAGE);
+			{
+				auto token = decl.unittest_.findTokenForDisplay(tok!"unittest");
+				addErrorMessage(token, KEY, MESSAGE,
+					[
+						AutoFix.insertionBefore(token[0], "@safe ", "Mark unittest @safe"),
+						AutoFix.insertionBefore(token[0], "@system ", "Mark unittest @system")
+					]);
+			}
 		}
 		decl.accept(this);
 	}

src/dscanner/analysis/final_attribute.d

@@ -57,7 +57,8 @@ private:
 	void addError(T)(const Token finalToken, T t, string msg)
 	{
 		import std.format : format;
-		addErrorMessage(finalToken.type ? finalToken : t.name, KEY, MSGB.format(msg));
+		addErrorMessage(finalToken.type ? finalToken : t.name, KEY, MSGB.format(msg),
+				[AutoFix.replacement(finalToken, "")]);
 	}
 
 public:

src/dscanner/analysis/function_attributes.d

@@ -104,9 +104,15 @@ final class FunctionAttributeCheck : BaseAnalyzer
 			}
 			if (foundProperty && !foundConst)
 			{
+				auto paren = dec.parameters.tokens.length ? dec.parameters.tokens[$ - 1] : Token.init;
+				auto autofixes = paren is Token.init ? null : [
+					AutoFix.insertionAfter(paren, " const", "Mark function `const`"),
+					AutoFix.insertionAfter(paren, " inout", "Mark function `inout`"),
+					AutoFix.insertionAfter(paren, " immutable", "Mark function `immutable`"),
+				];
 				addErrorMessage(dec.name, KEY,
 						"Zero-parameter '@property' function should be"
-						~ " marked 'const', 'inout', or 'immutable'.");
+						~ " marked 'const', 'inout', or 'immutable'.", autofixes);
 			}
 		}
 		dec.accept(this);
@@ -123,7 +129,8 @@ final class FunctionAttributeCheck : BaseAnalyzer
 				continue;
 			if (attr.attribute == tok!"abstract" && inInterface)
 			{
-				addErrorMessage(attr.attribute, KEY, ABSTRACT_MESSAGE);
+				addErrorMessage(attr.attribute, KEY, ABSTRACT_MESSAGE,
+					[AutoFix.replacement(attr.attribute, "")]);
 				continue;
 			}
 			if (attr.attribute == tok!"static")
@@ -136,9 +143,21 @@ final class FunctionAttributeCheck : BaseAnalyzer
 				import std.string : format;
 
 				immutable string attrString = str(attr.attribute.type);
+				AutoFix[] autofixes;
+				if (dec.functionDeclaration.parameters)
+					autofixes ~= AutoFix.replacement(
+							attr.attribute, "",
+							"Move " ~ str(attr.attribute.type) ~ " after parameter list")
+						.concat(AutoFix.insertionAfter(
+							dec.functionDeclaration.parameters.tokens[$ - 1],
+							" " ~ str(attr.attribute.type)));
+				if (dec.functionDeclaration.returnType)
+					autofixes ~= AutoFix.insertionAfter(attr.attribute, "(", "Make return type const")
+							.concat(AutoFix.insertionAfter(dec.functionDeclaration.returnType.tokens[$ - 1], ")"));
 				addErrorMessage(attr.attribute, KEY, format(
-							"'%s' is not an attribute of the return type." ~ " Place it after the parameter list to clarify.",
+							"'%s' is not an attribute of the return type."
-							attrString));
+							~ " Place it after the parameter list to clarify.",
+							attrString), autofixes);
 			}
 		}
 	end:

src/dscanner/analysis/lambda_return_check.d

@@ -23,6 +23,8 @@ final class LambdaReturnCheck : BaseAnalyzer
 
 	override void visit(const FunctionLiteralExpression fLit)
 	{
+		import std.algorithm : find;
+
 		auto fe = safeAccess(fLit).assignExpression.as!UnaryExpression
 			.primaryExpression.functionLiteralExpression.unwrap;
 
@@ -35,7 +37,22 @@ final class LambdaReturnCheck : BaseAnalyzer
 		auto endIncl = &fe.specifiedFunctionBody.tokens[0];
 		assert(endIncl >= start);
 		auto tokens = start[0 .. endIncl - start + 1];
-		addErrorMessage(tokens, KEY, "This lambda returns a lambda. Add parenthesis to clarify.");
+		auto arrow = tokens.find!(a => a.type == tok!"=>");
+
+		AutoFix[] autofixes;
+		if (arrow.length)
+		{
+			if (fLit.tokens[0] == tok!"(")
+				autofixes ~= AutoFix.replacement(arrow[0], "", "Remove arrow (use function body)");
+			else
+				autofixes ~= AutoFix.insertionBefore(fLit.tokens[0], "(", "Remove arrow (use function body)")
+					.concat(AutoFix.insertionAfter(fLit.tokens[0], ")"))
+					.concat(AutoFix.replacement(arrow[0], ""));
+		}
+		autofixes ~= AutoFix.insertionBefore(*endIncl, "(", "Add parenthesis (return delegate)")
+			.concat(AutoFix.insertionAfter(fe.specifiedFunctionBody.tokens[$ - 1], ")"));
+		addErrorMessage(tokens, KEY, "This lambda returns a lambda. Add parenthesis to clarify.",
+			autofixes);
 	}
 
 private:

src/dscanner/analysis/length_subtraction.d

@@ -41,7 +41,10 @@ final class LengthSubtractionCheck : BaseAnalyzer
 					|| l.identifierOrTemplateInstance.identifier.text != "length")
 				goto end;
 			addErrorMessage(addExpression, "dscanner.suspicious.length_subtraction",
-					"Avoid subtracting from '.length' as it may be unsigned.");
+					"Avoid subtracting from '.length' as it may be unsigned.",
+					[
+						AutoFix.insertionBefore(l.tokens[0], "cast(ptrdiff_t) ", "Cast to ptrdiff_t")
+					]);
 		}
 	end:
 		addExpression.accept(this);

src/dscanner/analysis/static_if_else.d

@@ -48,7 +48,11 @@ final class StaticIfElse : BaseAnalyzer
 		auto tokens = ifStmt.tokens[0 .. 1];
 		// extend one token to include `else` before this if
 		tokens = (tokens.ptr - 1)[0 .. 2];
-		addErrorMessage(tokens, KEY, "Mismatched static if. Use 'else static if' here.");
+		addErrorMessage(tokens, KEY, "Mismatched static if. Use 'else static if' here.",
+			[
+				AutoFix.insertionBefore(tokens[$ - 1], "static "),
+				// TODO: make if explicit with block {}, using correct indentation
+			]);
 	}
 
 	const(IfStatement) getIfStatement(const ConditionalStatement cc)