Implement autofix flow for dmd as a library and fix autofix for EnumA…

…rrayVisitor (#143)
Dlang-UPB · Sep 27, 2024 · 8af9081 · 8af9081
1 parent cd503ef
commit 8af9081
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 28 deletions.
diff --git a/src/dscanner/analysis/base.d b/src/dscanner/analysis/base.d
@@ -72,6 +72,16 @@ struct AutoFix
 		return ret;
 	}
 
+	static AutoFix replacement(size_t tokenStart, size_t tokenEnd, string newText, string name)
+	{
+		AutoFix ret;
+		ret.name = name;
+		ret.replacements = [
+			AutoFix.CodeReplacement([tokenStart, tokenEnd], newText)
+		];
+		return ret;
+	}
+
 	static AutoFix replacement(const Token token, string newText, string name = null)
 	{
 		if (!name.length)
@@ -342,6 +352,17 @@ struct Message
 		this.checkName = checkName;
 	}
 
+	this(string fileName, size_t line, size_t column, string key = null, string message = null, string checkName = null, AutoFix[] autofixes = null)
+	{
+		diagnostic.fileName = fileName;
+		diagnostic.startLine = diagnostic.endLine = line;
+		diagnostic.startColumn = diagnostic.endColumn = column;
+		diagnostic.message = message;
+		this.key = key;
+		this.checkName = checkName;
+		this.autofixes = autofixes;
+	}
+
 	this(Diagnostic diagnostic, string key = null, string checkName = null, AutoFix[] autofixes = null)
 	{
 		this.diagnostic = diagnostic;
@@ -366,7 +387,7 @@ enum comparitor = q{ a.startLine < b.startLine || (a.startLine == b.startLine &&
 
 alias MessageSet = RedBlackTree!(Message, comparitor, true);
 
-/** 
+/**
  * Should be present in all visitors to specify the name of the check
  *  done by a patricular visitor
  */
@@ -907,7 +928,7 @@ unittest
 	});
 }
 
-/** 
+/**
  * Visitor that implements the AST traversal logic.
  * Supports collecting error messages
  */
@@ -922,9 +943,9 @@ extern(C++) class BaseAnalyzerDmd : SemanticTimeTransitiveVisitor
 		_messages = new MessageSet;
 	}
 
-	/** 
+	/**
 	 * Ensures that template AnalyzerInfo is instantiated in all classes
-	 *  deriving from this class 
+	 *  deriving from this class
 	 */
 	extern(D) protected string getName()
 	{
@@ -945,17 +966,22 @@ extern(C++) class BaseAnalyzerDmd : SemanticTimeTransitiveVisitor
 
 protected:
 
-	extern(D) void addErrorMessage(size_t line, size_t column, string key, string message)
+	extern (D) void addErrorMessage(size_t line, size_t column, string key, string message)
 	{
 		_messages.insert(Message(fileName, line, column, key, message, getName()));
 	}
 
-	extern(D) bool skipTests;
+	extern (D) void addErrorMessage(size_t line, size_t column, string key, string message, AutoFix[] autofixes)
+	{
+		_messages.insert(Message(fileName, line, column, key, message, getName(), autofixes));
+	}
+
+	extern (D) bool skipTests;
 
 	/**
 	 * The file name
 	 */
-	extern(D) string fileName;
+	extern (D) string fileName;
 
-	extern(D) MessageSet _messages;
+	extern (D) MessageSet _messages;
 }
diff --git a/src/dscanner/analysis/enumarrayliteral.d b/src/dscanner/analysis/enumarrayliteral.d
@@ -7,12 +7,14 @@ module dscanner.analysis.enumarrayliteral;
 
 import dscanner.analysis.base;
 
-extern(C++) class EnumArrayVisitor(AST) : BaseAnalyzerDmd
+extern (C++) class EnumArrayVisitor(AST) : BaseAnalyzerDmd
 {
-	mixin AnalyzerInfo!"enum_array_literal_check";
 	alias visit = BaseAnalyzerDmd.visit;
+	mixin AnalyzerInfo!"enum_array_literal_check";
+
+	private enum KEY = "dscanner.performance.enum_array_literal";
 
-	extern(D) this(string fileName)
+	extern (D) this(string fileName)
 	{
 		super(fileName);
 	}
@@ -22,16 +24,41 @@ extern(C++) class EnumArrayVisitor(AST) : BaseAnalyzerDmd
 		import dmd.astenums : STC, InitKind;
 		import std.string : toStringz;
 
-		string message = "This enum may lead to unnecessary allocation at run-time."
-						~ " Use 'static immutable "
-						~ vd.ident.toString().idup() ~ " = [ ...' instead.";
+		string message = "This enum may lead to unnecessary allocation at run-time. Use 'static immutable "
+							~ vd.ident.toString().idup() ~ " = [ ...' instead.";
 
 		if (!vd.type && vd._init.kind == InitKind.array && vd.storage_class & STC.manifest)
-			addErrorMessage(cast(ulong) vd.loc.linnum,
-				cast(ulong) vd.loc.charnum, KEY,
-				message);
+		{
+			auto fileOffset = vd.loc.fileOffset - 5;
+
+			addErrorMessage(
+				cast(ulong) vd.loc.linnum, cast(ulong) vd.loc.charnum, KEY, message,
+				[AutoFix.replacement(fileOffset, fileOffset + 4, "static immutable", "Replace enum with static immutable")]
+			);
+		}
+
 		super.visit(vd);
 	}
+}
 
-	private enum KEY = "dscanner.performance.enum_array_literal";
-}
+unittest
+{
+	import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
+	import dscanner.analysis.helpers : assertAnalyzerWarningsDMD, assertAutoFix;
+	import std.stdio : stderr;
+
+	StaticAnalysisConfig sac = disabledConfig();
+	sac.enum_array_literal_check = Check.enabled;
+
+	assertAnalyzerWarningsDMD(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, true);
+
+	stderr.writeln("Unittest for EnumArrayLiteralCheck passed.");
+}
diff --git a/src/dscanner/analysis/helpers.d b/src/dscanner/analysis/helpers.d
@@ -233,7 +233,7 @@ private static immutable fileEol = q{
  * comment. Alternatively you can also just write `// fix` to apply the only
  * available suggestion.
  */
-void assertAutoFix(string before, string after, const StaticAnalysisConfig config,
+void assertAutoFix(string before, string after, const StaticAnalysisConfig config, bool useDmd = false,
 		const AutoFixFormatting formattingConfig = AutoFixFormatting(AutoFixFormatting.BraceStyle.otbs, "\t", 4, fileEol),
 		string file = __FILE__, size_t line = __LINE__)
 {
@@ -244,18 +244,43 @@ void assertAutoFix(string before, string after, const StaticAnalysisConfig confi
 	import std.conv : to;
 	import std.sumtype : match;
 	import std.typecons : tuple, Tuple;
+	import std.file : exists, remove;
+	import std.path : dirName;
+	import std.stdio : File;
+	import dscanner.analysis.rundmd : analyzeDmd, parseDmdModule;
+	import dscanner.utils : getModuleName;
 
-	StringCache cache = StringCache(StringCache.defaultBucketCount);
-	RollbackAllocator r;
-	const(Token)[] tokens;
-	const(Module) m = parseModule(file, cast(ubyte[]) before, &r, defaultErrorFormat, cache, false, tokens);
+	MessageSet rawWarnings;
 
-	ModuleCache moduleCache;
+	if (useDmd)
+	{
+		auto testFileName = "test.d";
+		File f = File(testFileName, "w");
+		scope(exit)
+		{
+			assert(exists(testFileName));
+			remove(testFileName);
+		}
 
-	// Run the code and get any warnings
-	MessageSet rawWarnings = analyze("test", m, config, moduleCache, tokens, true, true, formattingConfig);
-	string[] codeLines = before.splitLines();
+		f.write(before);
+		f.close();
 
+		auto dmdModule = parseDmdModule(file, before);
+		rawWarnings = analyzeDmd(testFileName, dmdModule, getModuleName(dmdModule.md), config);
+	}
+	else
+	{
+		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;
+
+		rawWarnings = analyze("test", m, config, moduleCache, tokens, true, true, formattingConfig);
+	}
+
+	string[] codeLines = before.splitLines();
 	Tuple!(Message, int)[] toApply;
 	int[] applyLines;