Skip to content

Commit ae60ea4

Browse files
committed
add end line/column to warnings
1 parent 5a53c53 commit ae60ea4

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

61 files changed

+1240
-631
lines changed

dub.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"built_with_dub"
1212
],
1313
"dependencies": {
14-
"libdparse": ">=0.23.0 <0.24.0",
14+
"libdparse": ">=0.23.1 <0.24.0",
1515
"dcd:dsymbol": ">=0.16.0-beta.2 <0.17.0",
1616
"inifiled": "~>1.3.1",
1717
"emsi_containers": "~>0.9.0",

dub.selections.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"emsi_containers": "0.9.0",
77
"inifiled": "1.3.3",
88
"libddoc": "0.8.0",
9-
"libdparse": "0.23.0",
9+
"libdparse": "0.23.1",
1010
"stdx-allocator": "2.77.5"
1111
}
1212
}

libdparse

src/dscanner/analysis/alias_syntax_check.d

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ final class AliasSyntaxCheck : BaseAnalyzer
2929
return;
3030
assert(ad.declaratorIdentifierList.identifiers.length > 0,
3131
"Identifier list length is zero, libdparse has a bug");
32-
addErrorMessage(ad.declaratorIdentifierList.identifiers[0].line,
33-
ad.declaratorIdentifierList.identifiers[0].column, KEY,
32+
addErrorMessage(ad, KEY,
3433
"Prefer the new \"'alias' identifier '=' type ';'\" syntax"
3534
~ " to the old \"'alias' type identifier ';'\" syntax.");
3635
}
@@ -48,7 +47,8 @@ unittest
4847
StaticAnalysisConfig sac = disabledConfig();
4948
sac.alias_syntax_check = Check.enabled;
5049
assertAnalyzerWarnings(q{
51-
alias int abcde; // [warn]: Prefer the new "'alias' identifier '=' type ';'" syntax to the old "'alias' type identifier ';'" syntax.
50+
alias int abcde; /+
51+
^^^^^^^^^^^^^^^^ [warn]: Prefer the new "'alias' identifier '=' type ';'" syntax to the old "'alias' type identifier ';'" syntax.+/
5252
alias abcde = int;
5353
}c, sac);
5454

src/dscanner/analysis/allman.d

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ final class AllManCheck : BaseAnalyzer
4747
continue;
4848
// ignore inline { } braces
4949
if (curLine != tokens[i + 1].line)
50-
addErrorMessage(tokens[i].line, tokens[i].column, KEY, MESSAGE);
50+
addErrorMessage(tokens[i], KEY, MESSAGE);
5151
}
5252
if (tokens[i].type == tok!"}" && curLine == prevTokenLine)
5353
{
@@ -56,7 +56,7 @@ final class AllManCheck : BaseAnalyzer
5656
continue;
5757
// ignore inline { } braces
5858
if (!tokens[0 .. i].retro.until!(t => t.line != curLine).canFind!(t => t.type == tok!"{"))
59-
addErrorMessage(tokens[i].line, tokens[i].column, KEY, MESSAGE);
59+
addErrorMessage(tokens[i], KEY, MESSAGE);
6060
}
6161
}
6262
}
@@ -79,7 +79,8 @@ unittest
7979
assertAnalyzerWarnings(q{
8080
void testAllman()
8181
{
82-
while (true) { // [warn]: %s
82+
while (true) { /+
83+
^ [warn]: %s +/
8384
auto f = 1;
8485
}
8586

src/dscanner/analysis/asm_style.d

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ final class AsmStyleCheck : BaseAnalyzer
3232
if (brExp.asmBrExp !is null && brExp.asmBrExp.asmUnaExp !is null
3333
&& brExp.asmBrExp.asmUnaExp.asmPrimaryExp !is null)
3434
{
35-
addErrorMessage(brExp.line, brExp.column, "dscanner.confusing.brexp",
35+
addErrorMessage(brExp, "dscanner.confusing.brexp",
3636
"This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.");
3737
}
3838
brExp.accept(this);
@@ -50,7 +50,8 @@ unittest
5050
{
5151
asm
5252
{
53-
mov a, someArray[1]; // [warn]: This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.
53+
mov a, someArray[1]; /+
54+
^^^^^^^^^^^^ [warn]: This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify. +/
5455
add near ptr [EAX], 3;
5556
}
5657
}

src/dscanner/analysis/assert_without_msg.d

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ final class AssertWithoutMessageCheck : BaseAnalyzer
4141
&& expr.assertArguments.message !is null;
4242

4343
if (!hasMessage)
44-
addErrorMessage(expr.line, expr.column, KEY, MESSAGE);
44+
addErrorMessage(expr, KEY, MESSAGE);
4545
}
4646

4747
override void visit(const FunctionCallExpression expr)
@@ -55,7 +55,7 @@ final class AssertWithoutMessageCheck : BaseAnalyzer
5555
auto ident = iot.identifier;
5656
if (ident.text == "enforce" && expr.arguments !is null && expr.arguments.namedArgumentList !is null &&
5757
expr.arguments.namedArgumentList.items.length < 2)
58-
addErrorMessage(ident.line, ident.column, KEY, MESSAGE);
58+
addErrorMessage(expr, KEY, MESSAGE);
5959
}
6060
}
6161

@@ -112,7 +112,8 @@ unittest
112112
assertAnalyzerWarnings(q{
113113
unittest {
114114
assert(0, "foo bar");
115-
assert(0); // [warn]: %s
115+
assert(0); /+
116+
^^^^^^^^^ [warn]: %s +/
116117
}
117118
}c.format(
118119
AssertWithoutMessageCheck.MESSAGE,
@@ -121,7 +122,8 @@ unittest
121122
assertAnalyzerWarnings(q{
122123
unittest {
123124
static assert(0, "foo bar");
124-
static assert(0); // [warn]: %s
125+
static assert(0); /+
126+
^^^^^^^^^ [warn]: %s +/
125127
}
126128
}c.format(
127129
AssertWithoutMessageCheck.MESSAGE,
@@ -133,7 +135,8 @@ unittest
133135
enforce(0); // std.exception not imported yet - could be a user-defined symbol
134136
import std.exception;
135137
enforce(0, "foo bar");
136-
enforce(0); // [warn]: %s
138+
enforce(0); /+
139+
^^^^^^^^^^ [warn]: %s +/
137140
}
138141
}c.format(
139142
AssertWithoutMessageCheck.MESSAGE,

src/dscanner/analysis/auto_function.d

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import dparse.ast;
1111
import dparse.lexer;
1212

1313
import std.stdio;
14-
import std.algorithm.searching : any;
14+
import std.algorithm : map, filter;
1515

1616
/**
1717
* Checks for auto functions without return statement.
@@ -26,7 +26,8 @@ final class AutoFunctionChecker : BaseAnalyzer
2626
private:
2727

2828
enum string KEY = "dscanner.suspicious.missing_return";
29-
enum string MESSAGE = "Auto function without return statement, prefer an explicit void";
29+
enum string MESSAGE = "Auto function without return statement, prefer replacing auto with void";
30+
enum string MESSAGE_INSERT = "Auto function without return statement, prefer inserting void to be explicit";
3031

3132
bool[] _returns;
3233
size_t _mixinDepth;
@@ -44,19 +45,41 @@ public:
4445
super(fileName, null, skipTests);
4546
}
4647

48+
package static const(Token)[] findAutoReturnType(const(FunctionDeclaration) decl)
49+
{
50+
auto autoFunTokens = decl.storageClasses
51+
.map!(a => a.token.type == tok!"auto"
52+
? [a.token]
53+
: a.atAttribute
54+
? a.atAttribute.tokens
55+
: null)
56+
.filter!(a => a.length > 0);
57+
return autoFunTokens.empty ? null : autoFunTokens.front;
58+
}
59+
4760
override void visit(const(FunctionDeclaration) decl)
4861
{
4962
_returns.length += 1;
5063
scope(exit) _returns.length -= 1;
5164
_returns[$-1] = false;
5265

53-
const bool autoFun = decl.storageClasses
54-
.any!(a => a.token.type == tok!"auto" || a.atAttribute !is null);
66+
auto autoTokens = findAutoReturnType(decl);
67+
bool isAtAttribute = autoTokens.length > 1;
5568

5669
decl.accept(this);
5770

58-
if (decl.functionBody.specifiedFunctionBody && autoFun && !_returns[$-1])
59-
addErrorMessage(decl.name.line, decl.name.column, KEY, MESSAGE);
71+
if (decl.functionBody.specifiedFunctionBody && autoTokens.length && !_returns[$-1])
72+
{
73+
if (isAtAttribute)
74+
{
75+
// highlight on the whitespace between attribute and function name
76+
auto tok = autoTokens[$ - 1];
77+
auto whitespace = tok.column + (tok.text.length ? tok.text.length : str(tok.type).length);
78+
addErrorMessage(tok.line, whitespace, whitespace + 1, KEY, MESSAGE_INSERT);
79+
}
80+
else
81+
addErrorMessage(autoTokens, KEY, MESSAGE);
82+
}
6083
}
6184

6285
override void visit(const(ReturnStatement) rst)
@@ -165,9 +188,12 @@ unittest
165188
StaticAnalysisConfig sac = disabledConfig();
166189
sac.auto_function_check = Check.enabled;
167190
assertAnalyzerWarnings(q{
168-
auto ref doStuff(){} // [warn]: %s
169-
auto doStuff(){} // [warn]: %s
170-
int doStuff(){auto doStuff(){}} // [warn]: %s
191+
auto ref doStuff(){} /+
192+
^^^^ [warn]: %s +/
193+
auto doStuff(){} /+
194+
^^^^ [warn]: %s +/
195+
int doStuff(){auto doStuff(){}} /+
196+
^^^^ [warn]: %s +/
171197
auto doStuff(){return 0;}
172198
int doStuff(){/*error but not the aim*/}
173199
}c.format(
@@ -177,55 +203,63 @@ unittest
177203
), sac);
178204

179205
assertAnalyzerWarnings(q{
180-
auto doStuff(){assert(true);} // [warn]: %s
206+
auto doStuff(){assert(true);} /+
207+
^^^^ [warn]: %s +/
181208
auto doStuff(){assert(false);}
182209
}c.format(
183210
AutoFunctionChecker.MESSAGE,
184211
), sac);
185212

186213
assertAnalyzerWarnings(q{
187-
auto doStuff(){assert(1);} // [warn]: %s
214+
auto doStuff(){assert(1);} /+
215+
^^^^ [warn]: %s +/
188216
auto doStuff(){assert(0);}
189217
}c.format(
190218
AutoFunctionChecker.MESSAGE,
191219
), sac);
192220

193221
assertAnalyzerWarnings(q{
194-
auto doStuff(){mixin("0+0");} // [warn]: %s
222+
auto doStuff(){mixin("0+0");} /+
223+
^^^^ [warn]: %s +/
195224
auto doStuff(){mixin("return 0;");}
196225
}c.format(
197226
AutoFunctionChecker.MESSAGE,
198227
), sac);
199228

200229
assertAnalyzerWarnings(q{
201-
auto doStuff(){mixin("0+0");} // [warn]: %s
230+
auto doStuff(){mixin("0+0");} /+
231+
^^^^ [warn]: %s +/
202232
auto doStuff(){mixin("static if (true)" ~ " return " ~ 0.stringof ~ ";");}
203233
}c.format(
204234
AutoFunctionChecker.MESSAGE,
205235
), sac);
206236

207237
assertAnalyzerWarnings(q{
208-
auto doStuff(){} // [warn]: %s
238+
auto doStuff(){} /+
239+
^^^^ [warn]: %s +/
209240
extern(C) auto doStuff();
210241
}c.format(
211242
AutoFunctionChecker.MESSAGE,
212243
), sac);
213244

214245
assertAnalyzerWarnings(q{
215-
auto doStuff(){} // [warn]: %s
246+
auto doStuff(){} /+
247+
^^^^ [warn]: %s +/
216248
@disable auto doStuff();
217249
}c.format(
218250
AutoFunctionChecker.MESSAGE,
219251
), sac);
220252

221253
assertAnalyzerWarnings(q{
222-
@property doStuff(){} // [warn]: %s
223-
@safe doStuff(){} // [warn]: %s
254+
@property doStuff(){} /+
255+
^ [warn]: %s +/
256+
@safe doStuff(){} /+
257+
^ [warn]: %s +/
224258
@disable doStuff();
225259
@safe void doStuff();
226260
}c.format(
227-
AutoFunctionChecker.MESSAGE,
228-
AutoFunctionChecker.MESSAGE,
261+
AutoFunctionChecker.MESSAGE_INSERT,
262+
AutoFunctionChecker.MESSAGE_INSERT,
229263
), sac);
230264

231265
assertAnalyzerWarnings(q{

src/dscanner/analysis/auto_ref_assignment.d

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,29 +54,29 @@ final class AutoRefAssignmentCheck : BaseAnalyzer
5454
{
5555
if (assign.operator == tok!"" || scopes.length == 0)
5656
return;
57-
interest++;
57+
interest ~= assign;
5858
assign.ternaryExpression.accept(this);
59-
interest--;
59+
interest.length--;
6060
}
6161

6262
override void visit(const IdentifierOrTemplateInstance ioti)
6363
{
6464
import std.algorithm.searching : canFind;
6565

66-
if (ioti.identifier == tok!"" || interest <= 0)
66+
if (ioti.identifier == tok!"" || !interest.length)
6767
return;
6868
if (scopes[$ - 1].canFind(ioti.identifier.text))
69-
addErrorMessage(ioti.identifier.line, ioti.identifier.column, KEY, MESSAGE);
69+
addErrorMessage(interest[$ - 1], KEY, MESSAGE);
7070
}
7171

7272
override void visit(const IdentifierChain ic)
7373
{
7474
import std.algorithm.searching : canFind;
7575

76-
if (ic.identifiers.length == 0 || interest <= 0)
76+
if (ic.identifiers.length == 0 || !interest.length)
7777
return;
7878
if (scopes[$ - 1].canFind(ic.identifiers[0].text))
79-
addErrorMessage(ic.identifiers[0].line, ic.identifiers[0].column, KEY, MESSAGE);
79+
addErrorMessage(interest[$ - 1], KEY, MESSAGE);
8080
}
8181

8282
alias visit = BaseAnalyzer.visit;
@@ -86,12 +86,7 @@ private:
8686
enum string MESSAGE = "Assignment to auto-ref function parameter.";
8787
enum string KEY = "dscanner.suspicious.auto_ref_assignment";
8888

89-
invariant
90-
{
91-
assert(interest >= 0);
92-
}
93-
94-
int interest;
89+
const(AssignExpression)[] interest;
9590

9691
void addSymbol(string symbolName)
9792
{
@@ -123,7 +118,8 @@ unittest
123118
assertAnalyzerWarnings(q{
124119
int doStuff(T)(auto ref int a)
125120
{
126-
a = 10; // [warn]: %s
121+
a = 10; /+
122+
^^^^^^ [warn]: %s +/
127123
}
128124

129125
int doStuff(T)(ref int a)

0 commit comments

Comments
 (0)