mirror of
https://github.com/dlang-community/D-Scanner.git
synced 2025-04-26 13:20:07 +03:00
Implement autofix flow for dmd as a library and fix autofix for EnumArrayVisitor (#143)
This commit is contained in:
parent
c0c881ed39
commit
c90a8f03e9
3 changed files with 106 additions and 28 deletions
|
@ -72,6 +72,16 @@ struct AutoFix
|
||||||
return ret;
|
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)
|
static AutoFix replacement(const Token token, string newText, string name = null)
|
||||||
{
|
{
|
||||||
if (!name.length)
|
if (!name.length)
|
||||||
|
@ -342,6 +352,17 @@ struct Message
|
||||||
this.checkName = checkName;
|
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, string key = null, string checkName = null, AutoFix[] autofixes = null)
|
||||||
{
|
{
|
||||||
this.diagnostic = diagnostic;
|
this.diagnostic = diagnostic;
|
||||||
|
@ -366,7 +387,7 @@ enum comparitor = q{ a.startLine < b.startLine || (a.startLine == b.startLine &&
|
||||||
|
|
||||||
alias MessageSet = RedBlackTree!(Message, comparitor, true);
|
alias MessageSet = RedBlackTree!(Message, comparitor, true);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Should be present in all visitors to specify the name of the check
|
* Should be present in all visitors to specify the name of the check
|
||||||
* done by a patricular visitor
|
* done by a patricular visitor
|
||||||
*/
|
*/
|
||||||
|
@ -907,7 +928,7 @@ unittest
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Visitor that implements the AST traversal logic.
|
* Visitor that implements the AST traversal logic.
|
||||||
* Supports collecting error messages
|
* Supports collecting error messages
|
||||||
*/
|
*/
|
||||||
|
@ -922,9 +943,9 @@ extern(C++) class BaseAnalyzerDmd : SemanticTimeTransitiveVisitor
|
||||||
_messages = new MessageSet;
|
_messages = new MessageSet;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Ensures that template AnalyzerInfo is instantiated in all classes
|
* Ensures that template AnalyzerInfo is instantiated in all classes
|
||||||
* deriving from this class
|
* deriving from this class
|
||||||
*/
|
*/
|
||||||
extern(D) protected string getName()
|
extern(D) protected string getName()
|
||||||
{
|
{
|
||||||
|
@ -945,17 +966,22 @@ extern(C++) class BaseAnalyzerDmd : SemanticTimeTransitiveVisitor
|
||||||
|
|
||||||
protected:
|
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()));
|
_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
|
* The file name
|
||||||
*/
|
*/
|
||||||
extern(D) string fileName;
|
extern (D) string fileName;
|
||||||
|
|
||||||
extern(D) MessageSet _messages;
|
extern (D) MessageSet _messages;
|
||||||
}
|
}
|
||||||
|
|
|
@ -7,12 +7,14 @@ module dscanner.analysis.enumarrayliteral;
|
||||||
|
|
||||||
import dscanner.analysis.base;
|
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;
|
alias visit = BaseAnalyzerDmd.visit;
|
||||||
|
mixin AnalyzerInfo!"enum_array_literal_check";
|
||||||
|
|
||||||
extern(D) this(string fileName)
|
private enum KEY = "dscanner.performance.enum_array_literal";
|
||||||
|
|
||||||
|
extern (D) this(string fileName)
|
||||||
{
|
{
|
||||||
super(fileName);
|
super(fileName);
|
||||||
}
|
}
|
||||||
|
@ -22,16 +24,41 @@ extern(C++) class EnumArrayVisitor(AST) : BaseAnalyzerDmd
|
||||||
import dmd.astenums : STC, InitKind;
|
import dmd.astenums : STC, InitKind;
|
||||||
import std.string : toStringz;
|
import std.string : toStringz;
|
||||||
|
|
||||||
string message = "This enum may lead to unnecessary allocation at run-time."
|
string message = "This enum may lead to unnecessary allocation at run-time. Use 'static immutable "
|
||||||
~ " Use 'static immutable "
|
~ vd.ident.toString().idup() ~ " = [ ...' instead.";
|
||||||
~ vd.ident.toString().idup() ~ " = [ ...' instead.";
|
|
||||||
|
|
||||||
if (!vd.type && vd._init.kind == InitKind.array && vd.storage_class & STC.manifest)
|
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,
|
auto fileOffset = vd.loc.fileOffset - 5;
|
||||||
message);
|
|
||||||
|
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);
|
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.");
|
||||||
|
}
|
||||||
|
|
|
@ -233,7 +233,7 @@ private static immutable fileEol = q{
|
||||||
* comment. Alternatively you can also just write `// fix` to apply the only
|
* comment. Alternatively you can also just write `// fix` to apply the only
|
||||||
* available suggestion.
|
* 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),
|
const AutoFixFormatting formattingConfig = AutoFixFormatting(AutoFixFormatting.BraceStyle.otbs, "\t", 4, fileEol),
|
||||||
string file = __FILE__, size_t line = __LINE__)
|
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.conv : to;
|
||||||
import std.sumtype : match;
|
import std.sumtype : match;
|
||||||
import std.typecons : tuple, Tuple;
|
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);
|
MessageSet rawWarnings;
|
||||||
RollbackAllocator r;
|
|
||||||
const(Token)[] tokens;
|
|
||||||
const(Module) m = parseModule(file, cast(ubyte[]) before, &r, defaultErrorFormat, cache, false, tokens);
|
|
||||||
|
|
||||||
ModuleCache moduleCache;
|
if (useDmd)
|
||||||
|
{
|
||||||
|
auto testFileName = "test.d";
|
||||||
|
File f = File(testFileName, "w");
|
||||||
|
scope(exit)
|
||||||
|
{
|
||||||
|
assert(exists(testFileName));
|
||||||
|
remove(testFileName);
|
||||||
|
}
|
||||||
|
|
||||||
|
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);
|
||||||
|
}
|
||||||
|
|
||||||
// Run the code and get any warnings
|
|
||||||
MessageSet rawWarnings = analyze("test", m, config, moduleCache, tokens, true, true, formattingConfig);
|
|
||||||
string[] codeLines = before.splitLines();
|
string[] codeLines = before.splitLines();
|
||||||
|
|
||||||
Tuple!(Message, int)[] toApply;
|
Tuple!(Message, int)[] toApply;
|
||||||
int[] applyLines;
|
int[] applyLines;
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue