diff --git a/src/dscanner/analysis/base.d b/src/dscanner/analysis/base.d index 5e40c9c..9413024 100644 --- 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 index fce6ab7..a4fc5bc 100644 --- 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"; -} \ No newline at end of file +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 index c53ec7a..e5780da 100644 --- 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;