Skip to content

Commit 99f383c

Browse files
committed
Add 2 more ThunderCode inspections, closes #3185
1 parent c22ae5e commit 99f383c

File tree

7 files changed

+247
-2
lines changed

7 files changed

+247
-2
lines changed
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
3+
using Antlr4.Runtime;
4+
using Antlr4.Runtime.Tree;
5+
using Rubberduck.Inspections.Abstract;
6+
using Rubberduck.Inspections.Results;
7+
using Rubberduck.Parsing;
8+
using Rubberduck.Parsing.Grammar;
9+
using Rubberduck.Parsing.Inspections.Abstract;
10+
using Rubberduck.Parsing.VBA;
11+
using Rubberduck.Resources.Inspections;
12+
using Rubberduck.VBEditor;
13+
14+
namespace Rubberduck.Inspections.Inspections.Concrete.ThunderCode
15+
{
16+
public class NegativeLineNumberInspection : ParseTreeInspectionBase
17+
{
18+
public NegativeLineNumberInspection(RubberduckParserState state) : base(state)
19+
{
20+
Listener = new NegativeLineNumberKeywordsListener();
21+
}
22+
23+
protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
24+
{
25+
return Listener.Contexts.Select(c => new QualifiedContextInspectionResult(
26+
this,
27+
28+
InspectionResults.NegativeLineNumberInspection.
29+
ThunderCodeFormat(),
30+
c));
31+
}
32+
33+
public override IInspectionListener Listener { get; }
34+
35+
public class NegativeLineNumberKeywordsListener : VBAParserBaseListener, IInspectionListener
36+
{
37+
private readonly List<QualifiedContext<ParserRuleContext>> _contexts = new List<QualifiedContext<ParserRuleContext>>();
38+
39+
public IReadOnlyList<QualifiedContext<ParserRuleContext>> Contexts => _contexts;
40+
41+
public void ClearContexts()
42+
{
43+
_contexts.Clear();
44+
}
45+
46+
public QualifiedModuleName CurrentModuleName { get; set; }
47+
48+
public override void EnterOnErrorStmt(VBAParser.OnErrorStmtContext context)
49+
{
50+
CheckContext(context, context.expression());
51+
base.EnterOnErrorStmt(context);
52+
}
53+
54+
public override void EnterGoToStmt(VBAParser.GoToStmtContext context)
55+
{
56+
CheckContext(context, context.expression());
57+
base.EnterGoToStmt(context);
58+
}
59+
60+
public override void EnterLineNumberLabel(VBAParser.LineNumberLabelContext context)
61+
{
62+
CheckContext(context, context);
63+
base.EnterLineNumberLabel(context);
64+
}
65+
66+
private void CheckContext(ParserRuleContext context, IParseTree expression)
67+
{
68+
var target = expression?.GetText().Trim() ?? string.Empty;
69+
if (target.StartsWith("-") && int.TryParse(target.Substring(1), out _))
70+
{
71+
_contexts.Add(new QualifiedContext<ParserRuleContext>(CurrentModuleName, context));
72+
}
73+
}
74+
}
75+
}
76+
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
3+
using Antlr4.Runtime;
4+
using Antlr4.Runtime.Tree;
5+
using Rubberduck.Inspections.Abstract;
6+
using Rubberduck.Inspections.Results;
7+
using Rubberduck.Parsing;
8+
using Rubberduck.Parsing.Grammar;
9+
using Rubberduck.Parsing.Inspections.Abstract;
10+
using Rubberduck.Parsing.VBA;
11+
using Rubberduck.Resources.Inspections;
12+
using Rubberduck.VBEditor;
13+
14+
namespace Rubberduck.Inspections.Inspections.Concrete.ThunderCode
15+
{
16+
public class OnErrorGoToMinusOneInspection : ParseTreeInspectionBase
17+
{
18+
public OnErrorGoToMinusOneInspection(RubberduckParserState state) : base(state)
19+
{
20+
Listener = new OnErrorGoToMinusOneListener();
21+
}
22+
23+
protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
24+
{
25+
return Listener.Contexts.Select(c => new QualifiedContextInspectionResult(
26+
this,
27+
28+
InspectionResults.OnErrorGoToMinusOneInspection.
29+
ThunderCodeFormat(),
30+
c));
31+
}
32+
33+
public override IInspectionListener Listener { get; }
34+
35+
public class OnErrorGoToMinusOneListener : VBAParserBaseListener, IInspectionListener
36+
{
37+
private readonly List<QualifiedContext<ParserRuleContext>> _contexts = new List<QualifiedContext<ParserRuleContext>>();
38+
39+
public IReadOnlyList<QualifiedContext<ParserRuleContext>> Contexts => _contexts;
40+
41+
public void ClearContexts()
42+
{
43+
_contexts.Clear();
44+
}
45+
46+
public QualifiedModuleName CurrentModuleName { get; set; }
47+
48+
public override void EnterOnErrorStmt(VBAParser.OnErrorStmtContext context)
49+
{
50+
CheckContext(context, context.expression());
51+
base.EnterOnErrorStmt(context);
52+
}
53+
54+
private void CheckContext(ParserRuleContext context, IParseTree expression)
55+
{
56+
var target = expression?.GetText().Trim() ?? string.Empty;
57+
if (target.StartsWith("-") && int.TryParse(target.Substring(1), out var result) && result == 1)
58+
{
59+
_contexts.Add(new QualifiedContext<ParserRuleContext>(CurrentModuleName, context));
60+
}
61+
}
62+
}
63+
}
64+
}

Rubberduck.Parsing/Grammar/VBAParser.g4

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,10 @@ standaloneLineNumberLabel :
622622
lineNumberLabel whiteSpace? COLON
623623
| lineNumberLabel;
624624
combinedLabels : lineNumberLabel whiteSpace identifierStatementLabel;
625-
lineNumberLabel : numberLiteral;
625+
// Technically, the negative numbers are illegal but VBE can prettify a
626+
// &HFFFFFFFF into a -1 which becomes a legal line number. Editing the same
627+
// line subsequently then breaks it.
628+
lineNumberLabel : MINUS? numberLiteral;
626629

627630
numberLiteral : HEXLITERAL | OCTLITERAL | FLOATLITERAL | INTEGERLITERAL;
628631

Rubberduck.Resources/Inspections/InspectionInfo.resx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,4 +373,10 @@ If the parameter can be null, ignore this inspection result; passing a null valu
373373
<data name="NonBreakingSpaceIdentifierInspection" xml:space="preserve">
374374
<value>The identiifer contains a non-breaking space which looks very much like just an ordinary space, which obfsucates the code and makes for a confusing experience. Consider using visible characters for the identifiers.</value>
375375
</data>
376+
<data name="NegativeLineNumberInspection" xml:space="preserve">
377+
<value>Negative line numbers are actually input as hex literal and then prettified by VBE. Editing the line again will cause it to turn red since negative line numbers are in fact illegal.</value>
378+
</data>
379+
<data name="OnErrorGoToMinusOneInspection" xml:space="preserve">
380+
<value>While this is legal, this is poorly documented "feature" that means something different -- the error state is also cleared in addition to disabling any error handling. However, this can be ambiguous as a negative line label of -1 may end up as a target and excessively complex error handling usually indicates a need of refactoring the procedure.</value>
381+
</data>
376382
</root>

Rubberduck.Resources/Inspections/InspectionNames.resx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,4 +372,10 @@
372372
<data name="NonBreakingSpaceIdentifierInspection" xml:space="preserve">
373373
<value>ThunderCode!</value>
374374
</data>
375+
<data name="NegativeLineNumberInspection" xml:space="preserve">
376+
<value>ThunderCode!</value>
377+
</data>
378+
<data name="OnErrorGoToMinusOneInspection" xml:space="preserve">
379+
<value>ThunderCode!</value>
380+
</data>
375381
</root>

Rubberduck.Resources/Inspections/InspectionResults.resx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,4 +415,10 @@ You're seeing this inspection result because there's no way that's real code and
415415
In memoriam, 1972-2018</value>
416416
<comment>{0} ThunderCode inspection description</comment>
417417
</data>
418+
<data name="NegativeLineNumberInspection" xml:space="preserve">
419+
<value>Negative line number encountered</value>
420+
</data>
421+
<data name="OnErrorGoToMinusOneInspection" xml:space="preserve">
422+
<value>On Error GoTo -1 encountered</value>
423+
</data>
418424
</root>

RubberduckTests/Inspections/ThunderCode/ThunderCodeInspectionTests.cs

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,91 @@ public void EvilLineContinuation_ReturnResults(int expectedCount, string inputCo
150150
ThunderCatsGo(func, inputCode, expectedCount);
151151
}
152152

153-
private void ThunderCatsGo(Func<RubberduckParserState, IInspection> inspectionFunction, string inputCode, int expectedCount)
153+
[Test]
154+
[TestCase(0,@"Public Sub Gogo()
155+
GoTo 1
156+
1:
157+
End Sub")]
158+
[TestCase(0, @"Public Sub Gogo()
159+
GoTo 1
160+
1
161+
End Sub")]
162+
[TestCase(1, @"Public Sub Gogo()
163+
-1:
164+
End Sub")]
165+
[TestCase(1, @"Public Sub Gogo()
166+
-1
167+
End Sub")]
168+
[TestCase(1, @"Public Sub Gogo()
169+
GoTo 1
170+
1
171+
-1:
172+
End Sub")]
173+
[TestCase(2, @"Public Sub Gogo()
174+
GoTo -1
175+
1
176+
-1:
177+
End Sub")]
178+
[TestCase(2, @"Public Sub Gogo()
179+
GoTo -1
180+
1:
181+
-1
182+
End Sub")]
183+
[TestCase(2, @"Public Sub Gogo()
184+
GoTo -5
185+
1
186+
-5:
187+
End Sub")]
188+
public void NegativeLineNumberLabel_ReturnResults(int expectedCount, string inputCode)
189+
{
190+
var func = new Func<RubberduckParserState, IInspection>(state =>
191+
new NegativeLineNumberInspection(state));
192+
ThunderCatsGo(func, inputCode, expectedCount);
193+
}
194+
195+
[Test]
196+
[TestCase(0, @"Public Sub Derp()
197+
On Error GoTo 1
198+
Exit Sub
199+
1
200+
Debug.Print ""derp""
201+
End Sub")]
202+
[TestCase(0, @"Public Sub Derp()
203+
On Error GoTo 1
204+
Exit Sub
205+
1:
206+
Debug.Print ""derp""
207+
End Sub")]
208+
[TestCase(0, @"Public Sub Derp()
209+
On Error GoTo 0
210+
Exit Sub
211+
Debug.Print ""derp""
212+
End Sub")]
213+
[TestCase(1, @"Public Sub Derp()
214+
On Error GoTo -1
215+
Exit Sub
216+
Debug.Print ""derp""
217+
End Sub")]
218+
[TestCase(0, @"Public Sub Derp()
219+
On Error GoTo -2
220+
Exit Sub
221+
-2
222+
Debug.Print ""derp""
223+
End Sub")]
224+
[TestCase(0, @"Public Sub Derp()
225+
On Error GoTo -2
226+
Exit Sub
227+
-2:
228+
Debug.Print ""derp""
229+
End Sub")]
230+
public void OnErrorGoToMinusOne_ReturnResults(int expectedCount, string inputCode)
231+
{
232+
var func = new Func<RubberduckParserState, IInspection>(state =>
233+
new OnErrorGoToMinusOneInspection(state));
234+
ThunderCatsGo(func, inputCode, expectedCount);
235+
}
236+
237+
private static void ThunderCatsGo(Func<RubberduckParserState, IInspection> inspectionFunction, string inputCode, int expectedCount)
154238
{
155239
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out var component);
156240
using (var state = MockParser.CreateAndParse(vbe.Object))

0 commit comments

Comments
 (0)