Skip to content

Commit a6ea37c

Browse files
VladiwostokVladiwostok
committed
Fix Autofix for FinalAttributeChecker (dlang-community#170)
1 parent 764b746 commit a6ea37c

File tree

1 file changed

+140
-97
lines changed

1 file changed

+140
-97
lines changed

‎src/dscanner/analysis/final_attribute.d

Lines changed: 140 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,23 @@
66
module dscanner.analysis.final_attribute;
77

88
import dscanner.analysis.base;
9-
import dscanner.analysis.helpers;
10-
import std.string : format;
11-
import std.stdio;
12-
import dmd.dsymbol;
139
import dmd.astcodegen;
10+
import dmd.dsymbol;
11+
import dmd.tokens : Token, TOK;
12+
import std.algorithm;
13+
import std.array;
14+
import std.range;
15+
import std.string : format;
1416

1517
/**
1618
* Checks for useless usage of the final attribute.
1719
*
1820
* There are several cases where the compiler allows them even if it's a noop.
1921
*/
20-
extern(C++) class FinalAttributeChecker(AST) : BaseAnalyzerDmd
22+
extern (C++) class FinalAttributeChecker(AST) : BaseAnalyzerDmd
2123
{
22-
23-
mixin AnalyzerInfo!"final_attribute_check";
24-
// alias visit = BaseAnalyzerDmd!AST.visit;
2524
alias visit = BaseAnalyzerDmd.visit;
25+
mixin AnalyzerInfo!"final_attribute_check";
2626

2727
enum Parent
2828
{
@@ -41,6 +41,8 @@ extern(C++) class FinalAttributeChecker(AST) : BaseAnalyzerDmd
4141
bool _blockFinal;
4242
Parent _parent = Parent.module_;
4343

44+
Token[] tokens;
45+
4446
enum pushPopPrivate = q{
4547
const bool wasPrivate = _private;
4648
_private = false;
@@ -50,6 +52,24 @@ extern(C++) class FinalAttributeChecker(AST) : BaseAnalyzerDmd
5052
extern(D) this(string fileName)
5153
{
5254
super(fileName);
55+
lexFile();
56+
}
57+
58+
private void lexFile()
59+
{
60+
import dscanner.utils : readFile;
61+
import dmd.errorsink : ErrorSinkNull;
62+
import dmd.globals : global;
63+
import dmd.lexer : Lexer;
64+
65+
auto bytes = readFile(fileName) ~ '\0';
66+
__gshared ErrorSinkNull errorSinkNull;
67+
if (!errorSinkNull)
68+
errorSinkNull = new ErrorSinkNull;
69+
70+
scope lexer = new Lexer(null, cast(char*) bytes, 0, bytes.length, 0, 0, errorSinkNull, &global.compileEnv);
71+
while (lexer.nextToken() != TOK.endOfFile)
72+
tokens ~= lexer.token;
5373
}
5474

5575
override void visit(AST.StorageClassDeclaration scd)
@@ -74,16 +94,22 @@ extern(C++) class FinalAttributeChecker(AST) : BaseAnalyzerDmd
7494
auto sd = member.isStructDeclaration();
7595
auto ud = member.isUnionDeclaration();
7696

77-
if (!ud && sd && scd.stc & STC.final_)
97+
if ((scd.stc & STC.final_) != 0)
7898
{
79-
addErrorMessage(cast(ulong) sd.loc.linnum, cast(ulong) sd.loc.charnum, KEY,
80-
MSGB.format(FinalAttributeChecker.MESSAGE.struct_i));
81-
}
99+
auto finalTokenOffset = tokens.filter!(t => t.loc.linnum == member.loc.linnum)
100+
.find!(t => t.value == TOK.final_)
101+
.front.loc.fileOffset;
82102

83-
if (ud && scd.stc & STC.final_)
84-
{
85-
addErrorMessage(cast(ulong) ud.loc.linnum, cast(ulong) ud.loc.charnum, KEY,
86-
MSGB.format(FinalAttributeChecker.MESSAGE.union_i));
103+
ulong lineNum = cast(ulong) member.loc.linnum;
104+
ulong charNum = cast(ulong) member.loc.charnum;
105+
106+
AutoFix fix = AutoFix.replacement(finalTokenOffset, finalTokenOffset + 6, "", "Remove final attribute");
107+
108+
if (!ud && sd)
109+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.struct_i), [fix]);
110+
111+
if (ud)
112+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.union_i), [fix]);
87113
}
88114

89115
member.accept(this);
@@ -101,15 +127,22 @@ extern(C++) class FinalAttributeChecker(AST) : BaseAnalyzerDmd
101127
{
102128
auto fd = member.isFuncDeclaration();
103129

104-
if (fd)
130+
if (fd && (fd.storage_class & STC.final_))
105131
{
106-
if (_parent == Parent.class_ && fd.storage_class & STC.final_)
107-
addErrorMessage(cast(ulong) fd.loc.linnum, cast(ulong) fd.loc.charnum, KEY,
108-
MSGB.format(FinalAttributeChecker.MESSAGE.class_t));
132+
auto finalTokenOffset = tokens.filter!(t => t.loc.linnum == fd.loc.linnum)
133+
.find!(t => t.value == TOK.final_)
134+
.front.loc.fileOffset;
135+
136+
ulong lineNum = cast(ulong) fd.loc.linnum;
137+
ulong charNum = cast(ulong) fd.loc.charnum;
138+
139+
AutoFix fix = AutoFix.replacement(finalTokenOffset, finalTokenOffset + 6, "", "Remove final attribute");
140+
141+
if (_parent == Parent.class_)
142+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.class_t), [fix]);
109143

110-
if (_parent == Parent.interface_ && fd.storage_class & STC.final_)
111-
addErrorMessage(cast(ulong) fd.loc.linnum, cast(ulong) fd.loc.charnum, KEY,
112-
MSGB.format(FinalAttributeChecker.MESSAGE.interface_t));
144+
if (_parent == Parent.interface_)
145+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.interface_t), [fix]);
113146
}
114147
}
115148

@@ -135,33 +168,38 @@ extern(C++) class FinalAttributeChecker(AST) : BaseAnalyzerDmd
135168
{
136169
import dmd.astenums : STC;
137170

138-
if (_parent == Parent.class_ && _private && fd.storage_class & STC.final_)
139-
addErrorMessage(cast(ulong) fd.loc.linnum, cast(ulong) fd.loc.charnum, KEY,
140-
MSGB.format(FinalAttributeChecker.MESSAGE.class_p));
171+
if ((fd.storage_class & STC.final_) != 0)
172+
{
173+
auto finalTokenOffset = tokens.filter!(t => t.loc.linnum == fd.loc.linnum)
174+
.array()
175+
.retro()
176+
.find!(t => t.value == TOK.final_)
177+
.front.loc.fileOffset;
178+
179+
ulong lineNum = cast(ulong) fd.loc.linnum;
180+
ulong charNum = cast(ulong) fd.loc.charnum;
141181

142-
else if (fd.storage_class & STC.final_ && (fd.storage_class & STC.static_ || _blockStatic))
143-
addErrorMessage(cast(ulong) fd.loc.linnum, cast(ulong) fd.loc.charnum, KEY,
144-
MSGB.format(FinalAttributeChecker.MESSAGE.class_s));
182+
AutoFix fix = AutoFix.replacement(finalTokenOffset, finalTokenOffset + 6, "", "Remove final attribute");
145183

146-
else if (_parent == Parent.class_ && _inFinalClass && fd.storage_class & STC.final_)
147-
addErrorMessage(cast(ulong) fd.loc.linnum, cast(ulong) fd.loc.charnum, KEY,
148-
MSGB.format(FinalAttributeChecker.MESSAGE.class_f));
184+
if (_parent == Parent.class_ && _private)
185+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.class_p), [fix]);
186+
else if (fd.storage_class & STC.static_ || _blockStatic)
187+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.class_s), [fix]);
188+
else if (_parent == Parent.class_ && _inFinalClass)
189+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.class_f), [fix]);
149190

150-
if (_parent == Parent.struct_ && fd.storage_class & STC.final_)
151-
addErrorMessage(cast(ulong) fd.loc.linnum, cast(ulong) fd.loc.charnum, KEY,
152-
MSGB.format(FinalAttributeChecker.MESSAGE.struct_f));
191+
if (_parent == Parent.struct_)
192+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.struct_f), [fix]);
153193

154-
if (_parent == Parent.union_ && fd.storage_class & STC.final_)
155-
addErrorMessage(cast(ulong) fd.loc.linnum, cast(ulong) fd.loc.charnum, KEY,
156-
MSGB.format(FinalAttributeChecker.MESSAGE.union_f));
194+
if (_parent == Parent.union_)
195+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.union_f), [fix]);
157196

158-
if (_parent == Parent.module_ && fd.storage_class & STC.final_)
159-
addErrorMessage(cast(ulong) fd.loc.linnum, cast(ulong) fd.loc.charnum, KEY,
160-
MSGB.format(FinalAttributeChecker.MESSAGE.func_g));
197+
if (_parent == Parent.module_)
198+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.func_g), [fix]);
161199

162-
if (_parent == Parent.function_ && fd.storage_class & STC.final_)
163-
addErrorMessage(cast(ulong) fd.loc.linnum, cast(ulong) fd.loc.charnum, KEY,
164-
MSGB.format(FinalAttributeChecker.MESSAGE.func_n));
200+
if (_parent == Parent.function_)
201+
addErrorMessage(lineNum, charNum, KEY, MSGB.format(FinalAttributeChecker.MESSAGE.func_n), [fix]);
202+
}
165203

166204
_blockStatic = false;
167205
mixin (pushPopPrivate);
@@ -232,7 +270,9 @@ extern(C++) class FinalAttributeChecker(AST) : BaseAnalyzerDmd
232270

233271
@system unittest
234272
{
235-
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
273+
import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
274+
import dscanner.analysis.helpers : assertAnalyzerWarningsDMD, assertAutoFix;
275+
import std.stdio : stderr;
236276

237277
StaticAnalysisConfig sac = disabledConfig();
238278
sac.final_attribute_check = Check.enabled;
@@ -393,58 +433,61 @@ extern(C++) class FinalAttributeChecker(AST) : BaseAnalyzerDmd
393433
}
394434
}, sac);
395435

396-
// TODO: Check if it works and fix otherwise
397-
//assertAutoFix(q{
398-
//int foo() @property { return 0; }
399-
400-
//class ClassName {
401-
//const int confusingConst() { return 0; } // fix:0
402-
//const int confusingConst() { return 0; } // fix:1
403-
404-
//int bar() @property { return 0; } // fix:0
405-
//int bar() @property { return 0; } // fix:1
406-
//int bar() @property { return 0; } // fix:2
407-
//}
408-
409-
//struct StructName {
410-
//int bar() @property { return 0; } // fix:0
411-
//}
412-
413-
//union UnionName {
414-
//int bar() @property { return 0; } // fix:0
415-
//}
416-
417-
//interface InterfaceName {
418-
//int bar() @property; // fix:0
419-
420-
//abstract int method(); // fix
421-
//}
422-
//}c, q{
423-
//int foo() @property { return 0; }
424-
425-
//class ClassName {
426-
//int confusingConst() const { return 0; } // fix:0
427-
//const(int) confusingConst() { return 0; } // fix:1
428-
429-
//int bar() const @property { return 0; } // fix:0
430-
//int bar() inout @property { return 0; } // fix:1
431-
//int bar() immutable @property { return 0; } // fix:2
432-
//}
433-
434-
//struct StructName {
435-
//int bar() const @property { return 0; } // fix:0
436-
//}
437-
438-
//union UnionName {
439-
//int bar() const @property { return 0; } // fix:0
440-
//}
441-
442-
//interface InterfaceName {
443-
//int bar() const @property; // fix:0
444-
445-
//int method(); // fix
446-
//}
447-
//}c, sac);
436+
assertAutoFix(q{
437+
final void foo(){} // fix
438+
void foo(){final void foo(){}} // fix
439+
void foo()
440+
{
441+
static if (true)
442+
final class A{ private: final protected void foo(){}} // fix
443+
}
444+
final struct Foo{} // fix
445+
final union Foo{} // fix
446+
class Foo{private final void foo(){}} // fix
447+
class Foo{private: final void foo(){}} // fix
448+
interface Foo{final void foo(T)(){}} // fix
449+
final class Foo{final void foo(){}} // fix
450+
private: final class Foo {public: private final void foo(){}} // fix
451+
class Foo {final static void foo(){}} // fix
452+
class Foo
453+
{
454+
void foo(){}
455+
static: final void foo(){} // fix
456+
}
457+
class Foo
458+
{
459+
void foo(){}
460+
static{final void foo(){}} // fix
461+
void foo(){}
462+
}
463+
}, q{
464+
void foo(){} // fix
465+
void foo(){void foo(){}} // fix
466+
void foo()
467+
{
468+
static if (true)
469+
final class A{ private: protected void foo(){}} // fix
470+
}
471+
struct Foo{} // fix
472+
union Foo{} // fix
473+
class Foo{private void foo(){}} // fix
474+
class Foo{private: void foo(){}} // fix
475+
interface Foo{void foo(T)(){}} // fix
476+
final class Foo{void foo(){}} // fix
477+
private: final class Foo {public: private void foo(){}} // fix
478+
class Foo {static void foo(){}} // fix
479+
class Foo
480+
{
481+
void foo(){}
482+
static: void foo(){} // fix
483+
}
484+
class Foo
485+
{
486+
void foo(){}
487+
static{void foo(){}} // fix
488+
void foo(){}
489+
}
490+
}, sac, true);
448491

449492
stderr.writeln("Unittest for FinalAttributeChecker passed.");
450493
}

0 commit comments

Comments
 (0)