add autofix testing API

src/dscanner/analysis/auto_function.d

@@ -270,5 +270,16 @@ unittest
 		auto doStuff(){ mixin(_genSave);}
 	}, sac);
 
+
+	assertAutoFix(q{
+		auto doStuff(){} // fix
+		@property doStuff(){} // fix
+		@safe doStuff(){} // fix
+	}c, q{
+		void doStuff(){} // fix
+		@property void doStuff(){} // fix
+		@safe void doStuff(){} // fix
+	}c, sac);
+
 	stderr.writeln("Unittest for AutoFunctionChecker passed.");
 }

src/dscanner/analysis/base.d

@@ -276,7 +276,7 @@ public:
 		_messages = new MessageSet;
 	}
 
-	protected string getName()
+	string getName()
 	{
 		assert(0);
 	}

src/dscanner/analysis/del.d

@@ -37,8 +37,8 @@ final class DeleteCheck : BaseAnalyzer
 
 unittest
 {
-	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
-	import dscanner.analysis.helpers : assertAnalyzerWarnings;
+	import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
 
 	StaticAnalysisConfig sac = disabledConfig();
 	sac.delete_check = Check.enabled;
@@ -55,5 +55,25 @@ unittest
 		}
 	}c, sac);
 
+	assertAutoFix(q{
+		void testDelete()
+		{
+			int[int] data = [1 : 2];
+			delete data[1]; // fix
+
+			auto a = new Class();
+			delete a; // fix
+		}
+	}c, q{
+		void testDelete()
+		{
+			int[int] data = [1 : 2];
+			destroy(data[1]); // fix
+
+			auto a = new Class();
+			destroy(a); // fix
+		}
+	}c, sac);
+
 	stderr.writeln("Unittest for DeleteCheck passed.");
 }

src/dscanner/analysis/duplicate_attribute.d

@@ -227,5 +227,40 @@ unittest
 		}
 	}c, sac);
 
+
+	assertAutoFix(q{
+		class ExampleAttributes
+		{
+			@property @property bool aaa() {} // fix
+			bool bbb() @safe @safe {} // fix
+			@system bool ccc() @system {} // fix
+			@trusted bool ddd() @trusted {} // fix
+		}
+
+		class ExamplePureNoThrow
+		{
+			pure pure bool bbb() {} // fix
+			bool ccc() pure pure {} // fix
+			nothrow nothrow bool ddd() {} // fix
+			bool eee() nothrow nothrow {} // fix
+		}
+	}c, q{
+		class ExampleAttributes
+		{
+			@property bool aaa() {} // fix
+			bool bbb() @safe {} // fix
+			@system bool ccc() {} // fix
+			@trusted bool ddd() {} // fix
+		}
+
+		class ExamplePureNoThrow
+		{
+			pure bool bbb() {} // fix
+			bool ccc() pure {} // fix
+			nothrow bool ddd() {} // fix
+			bool eee() nothrow {} // fix
+		}
+	}c, sac);
+
 	stderr.writeln("Unittest for DuplicateAttributeCheck passed.");
 }

src/dscanner/analysis/enumarrayliteral.d

@@ -59,3 +59,25 @@ final class EnumArrayLiteralCheck : BaseAnalyzer
 		autoDec.accept(this);
 	}
 }
+
+unittest
+{
+	import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
+	import std.stdio : stderr;
+
+	StaticAnalysisConfig sac = disabledConfig();
+	sac.enum_array_literal_check = Check.enabled;
+	assertAnalyzerWarnings(q{
+		enum x = [1, 2, 3]; /+
+		         ^^^^^^^^^ [warn]: This enum may lead to unnecessary allocation at run-time. Use 'static immutable x = [ ...' instead. +/
+	}c, sac);
+
+	assertAutoFix(q{
+		enum x = [1, 2, 3]; // fix
+	}c, q{
+		static immutable x = [1, 2, 3]; // fix
+	}c, sac);
+
+	stderr.writeln("Unittest for EnumArrayLiteralCheck passed.");
+}

src/dscanner/analysis/explicitly_annotated_unittests.d

@@ -62,10 +62,10 @@ final class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer
 
 unittest
 {
-	import std.stdio : stderr;
+	import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
 	import std.format : format;
-	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
-	import dscanner.analysis.helpers : assertAnalyzerWarnings;
+	import std.stdio : stderr;
 
 	StaticAnalysisConfig sac = disabledConfig();
 	sac.explicitly_annotated_unittests = Check.enabled;
@@ -101,5 +101,27 @@ unittest
 		ExplicitlyAnnotatedUnittestCheck.MESSAGE,
 	), sac);
 
+
+	// nested
+	assertAutoFix(q{
+		unittest {} // fix:0
+		pure nothrow @nogc unittest {} // fix:0
+
+		struct Foo
+		{
+			unittest {} // fix:1
+			pure nothrow @nogc unittest {} // fix:1
+		}
+	}c, q{
+		@safe unittest {} // fix:0
+		pure nothrow @nogc @safe unittest {} // fix:0
+
+		struct Foo
+		{
+			@system unittest {} // fix:1
+			pure nothrow @nogc @system unittest {} // fix:1
+		}
+	}c, sac);
+
 	stderr.writeln("Unittest for ExplicitlyAnnotatedUnittestCheck passed.");
 }

src/dscanner/analysis/final_attribute.d

@@ -254,10 +254,10 @@ public:
 
 @system unittest
 {
-	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
-	import dscanner.analysis.helpers : assertAnalyzerWarnings;
-	import std.stdio : stderr;
+	import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
 	import std.format : format;
+	import std.stdio : stderr;
 
 	StaticAnalysisConfig sac = disabledConfig();
 	sac.final_attribute_check = Check.enabled;
@@ -433,5 +433,62 @@ public:
 		}
 	}, sac);
 
+
+	assertAutoFix(q{
+		final void foo(){} // fix
+		void foo(){final void foo(){}} // fix
+		void foo()
+		{
+			static if (true)
+			final class A{ private: final protected void foo(){}} // fix
+		}
+		final struct Foo{} // fix
+		final union Foo{} // fix
+		class Foo{private final void foo(){}} // fix
+		class Foo{private: final void foo(){}} // fix
+		interface Foo{final void foo(T)(){}} // fix
+		final class Foo{final void foo(){}} // fix
+		private: final class Foo {public: private final void foo(){}} // fix
+		class Foo {final static void foo(){}} // fix
+		class Foo
+		{
+			void foo(){}
+			static: final void foo(){} // fix
+		}
+		class Foo
+		{
+			void foo(){}
+			static{ final void foo(){}} // fix
+			void foo(){}
+		}
+	}, q{
+		void foo(){} // fix
+		void foo(){ void foo(){}} // fix
+		void foo()
+		{
+			static if (true)
+			final class A{ private: protected void foo(){}} // fix
+		}
+		struct Foo{} // fix
+		union Foo{} // fix
+		class Foo{private void foo(){}} // fix
+		class Foo{private: void foo(){}} // fix
+		interface Foo{ void foo(T)(){}} // fix
+		final class Foo{ void foo(){}} // fix
+		private: final class Foo {public: private void foo(){}} // fix
+		class Foo { static void foo(){}} // fix
+		class Foo
+		{
+			void foo(){}
+			static: void foo(){} // fix
+		}
+		class Foo
+		{
+			void foo(){}
+			static{ void foo(){}} // fix
+			void foo(){}
+		}
+	}, sac);
+
 	stderr.writeln("Unittest for FinalAttributeChecker passed.");
 }

src/dscanner/analysis/function_attributes.d

@@ -223,5 +223,58 @@ unittest
 		}
 	}c, sac);
 
+
+	assertAutoFix(q{
+		int foo() @property { return 0; }
+
+		class ClassName {
+			const int confusingConst() { return 0; } // fix:0
+			const int confusingConst() { return 0; } // fix:1
+
+			int bar() @property { return 0; } // fix:0
+			int bar() @property { return 0; } // fix:1
+			int bar() @property { return 0; } // fix:2
+		}
+
+		struct StructName {
+			int bar() @property { return 0; } // fix:0
+		}
+
+		union UnionName {
+			int bar() @property { return 0; } // fix:0
+		}
+
+		interface InterfaceName {
+			int bar() @property; // fix:0
+
+			abstract int method(); // fix
+		}
+	}c, q{
+		int foo() @property { return 0; }
+
+		class ClassName {
+			int confusingConst() const { return 0; } // fix:0
+			const(int) confusingConst() { return 0; } // fix:1
+
+			int bar() const @property { return 0; } // fix:0
+			int bar() inout @property { return 0; } // fix:1
+			int bar() immutable @property { return 0; } // fix:2
+		}
+
+		struct StructName {
+			int bar() const @property { return 0; } // fix:0
+		}
+
+		union UnionName {
+			int bar() const @property { return 0; } // fix:0
+		}
+
+		interface InterfaceName {
+			int bar() const @property; // fix:0
+
+			int method(); // fix
+		}
+	}c, sac);
+
 	stderr.writeln("Unittest for FunctionAttributeCheck passed.");
 }

src/dscanner/analysis/helpers.d

@@ -6,9 +6,9 @@
 module dscanner.analysis.helpers;
 
 import core.exception : AssertError;
+import std.stdio;
 import std.string;
 import std.traits;
-import std.stdio;
 
 import dparse.ast;
 import dparse.rollback_allocator;
@@ -194,3 +194,124 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
 		throw new AssertError(message, file, line);
 	}
 }
+
+/**
+ * This assert function will analyze the passed in code, get the warnings, and
+ * apply all specified autofixes all at once.
+ *
+ * Indicate which autofix to apply by adding a line comment at the end of the
+ * line with the following content: `// fix:0`, where 0 is the index which
+ * autofix to apply. There may only be one diagnostic on a line with this fix
+ * comment. Alternatively you can also just write `// fix` to apply the only
+ * available suggestion.
+ */
+void assertAutoFix(string before, string after, const StaticAnalysisConfig config,
+		string file = __FILE__, size_t line = __LINE__)
+{
+	import dparse.lexer : StringCache, Token;
+	import dscanner.analysis.run : parseModule;
+	import std.algorithm : canFind, findSplit, map, sort;
+	import std.conv : to;
+	import std.sumtype : match;
+	import std.typecons : tuple, Tuple;
+
+	StringCache cache = StringCache(StringCache.defaultBucketCount);
+	RollbackAllocator r;
+	const(Token)[] tokens;
+	const(Module) m = parseModule(file, cast(ubyte[]) before, &r, defaultErrorFormat, cache, false, tokens);
+
+	ModuleCache moduleCache;
+
+	// Run the code and get any warnings
+	MessageSet rawWarnings = analyze("test", m, config, moduleCache, tokens);
+	string[] codeLines = before.splitLines();
+
+	Tuple!(Message, int)[] toApply;
+	int[] applyLines;
+
+	scope (failure)
+	{
+		if (toApply.length)
+			stderr.writefln("Would have applied these fixes:%(\n- %s%)",
+				toApply.map!"a[0].autofixes[a[1]].name");
+		else
+			stderr.writeln("Did not find any fixes at all up to this point.");
+		stderr.writeln("Found warnings on lines: ", rawWarnings[].map!(a
+			=> a.endLine == 0 ? 0 : a.endLine - 1 + line));
+	}
+
+	foreach (rawWarning; rawWarnings[])
+	{
+		// Skip the warning if it is on line zero
+		immutable size_t rawLine = rawWarning.endLine;
+		if (rawLine == 0)
+		{
+			stderr.writefln("!!! Skipping warning because it is on line zero:\n%s",
+					rawWarning.message);
+			continue;
+		}
+
+		auto fixComment = codeLines[rawLine - 1].findSplit("// fix");
+		if (fixComment[1].length)
+		{
+			applyLines ~= cast(int)rawLine - 1;
+			if (fixComment[2].startsWith(":"))
+			{
+				auto i = fixComment[2][1 .. $].to!int;
+				assert(i >= 0, "can't use negative autofix indices");
+				if (i >= rawWarning.autofixes.length)
+					throw new AssertError("autofix index out of range, diagnostic only has %s autofixes (%s)."
+						.format(rawWarning.autofixes.length, rawWarning.autofixes.map!"a.name"),
+							file, rawLine + line);
+				toApply ~= tuple(rawWarning, i);
+			}
+			else
+			{
+				if (rawWarning.autofixes.length != 1)
+					throw new AssertError("diagnostic has %s autofixes (%s), but expected exactly one."
+						.format(rawWarning.autofixes.length, rawWarning.autofixes.map!"a.name"),
+							file, rawLine + line);
+				toApply ~= tuple(rawWarning, 0);
+			}
+		}
+	}
+
+	foreach (i, codeLine; codeLines)
+	{
+		if (!applyLines.canFind(i) && codeLine.canFind("// fix"))
+			throw new AssertError("Missing expected warning for autofix on line %s"
+				.format(i + line), file, i + line);
+	}
+
+	AutoFix.CodeReplacement[] replacements;
+
+	foreach_reverse (pair; toApply)
+	{
+		Message message = pair[0];
+		AutoFix fix = message.autofixes[pair[1]];
+		replacements ~= fix.autofix.match!(
+			(AutoFix.CodeReplacement[] r) => r,
+			(AutoFix.ResolveContext context) => resolveAutoFix(message, context, "test", moduleCache, tokens, m, config)
+		);
+	}
+
+	replacements.sort!"a.range[0] < b.range[0]";
+
+	improveAutoFixWhitespace(before, replacements);
+
+	string newCode = before;
+	foreach_reverse (replacement; replacements)
+	{
+		newCode = newCode[0 .. replacement.range[0]] ~ replacement.newText
+			~ newCode[replacement.range[1] .. $];
+	}
+
+	if (newCode != after)
+	{
+		throw new AssertError("Applying autofix didn't yield expected results. Expected:\n"
+			~ after.lineSplitter!(KeepTerminator.yes).map!(a => "\t" ~ a).join
+			~ "\n\nActual:\n"
+			~ newCode.lineSplitter!(KeepTerminator.yes).map!(a => "\t" ~ a).join,
+			file, line);
+	}
+}

src/dscanner/analysis/lambda_return_check.d

@@ -62,14 +62,14 @@ private:
 version(Windows) {/*because of newline in code*/} else
 unittest
 {
-	import dscanner.analysis.helpers : assertAnalyzerWarnings;
-	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
+	import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
 	import std.stdio : stderr;
 
 	StaticAnalysisConfig sac = disabledConfig();
 	sac.lambda_return_check = Check.enabled;
 
-	auto code = `
+	assertAnalyzerWarnings(q{
 		void main()
 		{
 			int[] b;
@@ -81,7 +81,33 @@ unittest
 			                   ^^^^^^^^ [warn]: This lambda returns a lambda. Add parenthesis to clarify. +/
 			pragma(msg, typeof({ return a; }));
 			pragma(msg, typeof(a => () { return a; }));
-		}`c;
-	assertAnalyzerWarnings(code, sac);
+		}
+	}c, sac);
+
+
+	assertAutoFix(q{
+		void main()
+		{
+			int[] b;
+			auto a = b.map!(a => { return a * a + 2; }).array(); // fix:0
+			auto a = b.map!(a => { return a * a + 2; }).array(); // fix:1
+			pragma(msg, typeof(a => { return a; })); // fix:0
+			pragma(msg, typeof(a => { return a; })); // fix:1
+			pragma(msg, typeof((a) => { return a; })); // fix:0
+			pragma(msg, typeof((a) => { return a; })); // fix:1
+		}
+	}c, q{
+		void main()
+		{
+			int[] b;
+			auto a = b.map!((a) { return a * a + 2; }).array(); // fix:0
+			auto a = b.map!(a => ({ return a * a + 2; })).array(); // fix:1
+			pragma(msg, typeof((a) { return a; })); // fix:0
+			pragma(msg, typeof(a => ({ return a; }))); // fix:1
+			pragma(msg, typeof((a) { return a; })); // fix:0
+			pragma(msg, typeof((a) => ({ return a; }))); // fix:1
+		}
+	}c, sac);
+
 	stderr.writeln("Unittest for LambdaReturnCheck passed.");
 }

src/dscanner/analysis/length_subtraction.d

@@ -65,5 +65,19 @@ unittest
 				writeln("something");
 		}
 	}c, sac);
+
+	assertAutoFix(q{
+		void testSizeT()
+		{
+			if (i < a.length - 1) // fix
+				writeln("something");
+		}
+	}c, q{
+		void testSizeT()
+		{
+			if (i < cast(ptrdiff_t) a.length - 1) // fix
+				writeln("something");
+		}
+	}c, sac);
 	stderr.writeln("Unittest for IfElseSameCheck passed.");
 }

src/dscanner/analysis/run.d

@@ -510,37 +510,23 @@ unittest
 	assert(test("std.bar.foo", "-barr,+bar"));
 }
 
-MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig analysisConfig,
-		ref ModuleCache moduleCache, const(Token)[] tokens, bool staticAnalyze = true)
+private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
+	const(Token)[] tokens, const Module m,
+	const StaticAnalysisConfig analysisConfig, const Scope* moduleScope)
 {
-	import dsymbol.symbol : DSymbol;
-
-	if (!staticAnalyze)
-		return null;
-
 	version (unittest)
 		enum ut = true;
 	else
 		enum ut = false;
 
+	BaseAnalyzer[] checks;
+
 	string moduleName;
 	if (m !is null && m.moduleDeclaration !is null &&
 		  m.moduleDeclaration.moduleName !is null &&
 		  m.moduleDeclaration.moduleName.identifiers !is null)
 		moduleName = m.moduleDeclaration.moduleName.identifiers.map!(e => e.text).join(".");
 
-	scope first = new FirstPass(m, internString(fileName), &moduleCache, null);
-	first.run();
-
-	secondPass(first.rootSymbol, first.moduleScope, moduleCache);
-	auto moduleScope = first.moduleScope;
-	scope(exit) typeid(DSymbol).destroy(first.rootSymbol.acSymbol);
-	scope(exit) typeid(SemanticSymbol).destroy(first.rootSymbol);
-	scope(exit) typeid(Scope).destroy(first.moduleScope);
-	BaseAnalyzer[] checks;
-
-	GC.disable;
-
 	if (moduleName.shouldRun!AsmStyleCheck(analysisConfig))
 		checks ~= new AsmStyleCheck(fileName, moduleScope,
 		analysisConfig.asm_style_check == Check.skipTests && !ut);
@@ -752,19 +738,73 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
 			checks ~= new IfStatementCheck(fileName, moduleScope,
 			analysisConfig.redundant_if_check == Check.skipTests && !ut);
 
+	return checks;
+}
+
+MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig analysisConfig,
+		ref ModuleCache moduleCache, const(Token)[] tokens, bool staticAnalyze = true)
+{
+	import dsymbol.symbol : DSymbol;
+
+	if (!staticAnalyze)
+		return null;
+
+	scope first = new FirstPass(m, internString(fileName), &moduleCache, null);
+	first.run();
+
+	secondPass(first.rootSymbol, first.moduleScope, moduleCache);
+	auto moduleScope = first.moduleScope;
+	scope(exit) typeid(DSymbol).destroy(first.rootSymbol.acSymbol);
+	scope(exit) typeid(SemanticSymbol).destroy(first.rootSymbol);
+	scope(exit) typeid(Scope).destroy(first.moduleScope);
+
+	GC.disable;
+	scope (exit)
+		GC.enable;
+
 	MessageSet set = new MessageSet;
-	foreach (check; checks)
+	foreach (check; getAnalyzersForModuleAndConfig(fileName, tokens, m, analysisConfig, moduleScope))
 	{
 		check.visit(m);
 		foreach (message; check.messages)
 			set.insert(message);
 	}
 
-	GC.enable;
-
 	return set;
 }
 
+AutoFix.CodeReplacement[] resolveAutoFix(const Message message,
+	const AutoFix.ResolveContext resolve, string fileName,
+	ref ModuleCache moduleCache, const(Token)[] tokens, const Module m,
+	const StaticAnalysisConfig analysisConfig)
+{
+	import dsymbol.symbol : DSymbol;
+
+	scope first = new FirstPass(m, internString(fileName), &moduleCache, null);
+	first.run();
+
+	secondPass(first.rootSymbol, first.moduleScope, moduleCache);
+	auto moduleScope = first.moduleScope;
+	scope(exit) typeid(DSymbol).destroy(first.rootSymbol.acSymbol);
+	scope(exit) typeid(SemanticSymbol).destroy(first.rootSymbol);
+	scope(exit) typeid(Scope).destroy(first.moduleScope);
+
+	GC.disable;
+	scope (exit)
+		GC.enable;
+
+	foreach (BaseAnalyzer check; getAnalyzersForModuleAndConfig(fileName, tokens, m, analysisConfig, moduleScope))
+	{
+		if (check.getName() == message.checkName)
+		{
+			return check.resolveAutoFix(m, tokens, message, resolve);
+		}
+	}
+
+	throw new Exception("Cannot find analyzer " ~ message.checkName
+		~ " to resolve autofix with.");
+}
+
 void improveAutoFixWhitespace(scope const(char)[] code, AutoFix.CodeReplacement[] replacements)
 {
 	import std.ascii : isWhite;

src/dscanner/analysis/static_if_else.d

@@ -65,8 +65,8 @@ final class StaticIfElse : BaseAnalyzer
 
 unittest
 {
-	import dscanner.analysis.helpers : assertAnalyzerWarnings;
-	import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
+	import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
 	import std.stdio : stderr;
 
 	StaticAnalysisConfig sac = disabledConfig();
@@ -92,5 +92,21 @@ unittest
 		}
 	}c, sac);
 
+	assertAutoFix(q{
+		void foo() {
+			static if (false)
+				auto a = 0;
+			else if (true) // fix
+				auto b = 1;
+		}
+	}c, q{
+		void foo() {
+			static if (false)
+				auto a = 0;
+			else static if (true) // fix
+				auto b = 1;
+		}
+	}c, sac);
+
 	stderr.writeln("Unittest for StaticIfElse passed.");
 }