Skip to content

Commit 669a8a2

Browse files
retailcoderretailcoder
authored
Merge pull request #4068 from MDoerner/FixEvilForLoop
Handle evil For/ForEach loops from Hell
2 parents 7a2eaaf + 5734e9c commit 669a8a2

18 files changed

+162
-116
lines changed

‎Rubberduck.CodeAnalysis/Inspections/Concrete/EmptyBlockInspectionListenerBase.cs‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Rubberduck.VBEditor;
66
using System.Collections.Generic;
77
using System.Diagnostics;
8+
using Antlr4.Runtime.Tree;
89

910
namespace Rubberduck.Inspections.Concrete
1011
{
@@ -35,12 +36,12 @@ public void AddResult(QualifiedContext<ParserRuleContext> qualifiedContext)
3536

3637
private bool BlockContainsExecutableStatements(VBAParser.BlockContext block)
3738
{
38-
return block?.children != null && ContainsExecutableStatements(block);
39+
return block?.children != null && ContainsExecutableStatements(block.children);
3940
}
4041

41-
private bool ContainsExecutableStatements(VBAParser.BlockContext block)
42+
private bool ContainsExecutableStatements(IList<IParseTree> blockChildren)
4243
{
43-
foreach (var child in block.children)
44+
foreach (var child in blockChildren)
4445
{
4546
if (child is VBAParser.BlockStmtContext blockStmt)
4647
{
@@ -76,5 +77,18 @@ child is VBAParser.CommentOrAnnotationContext ||
7677

7778
return false;
7879
}
80+
81+
public void InspectBlockForExecutableStatements<T>(VBAParser.UnterminatedBlockContext block, T context) where T : ParserRuleContext
82+
{
83+
if (!BlockContainsExecutableStatements(block))
84+
{
85+
AddResult(new QualifiedContext<ParserRuleContext>(CurrentModuleName, context));
86+
}
87+
}
88+
89+
private bool BlockContainsExecutableStatements(VBAParser.UnterminatedBlockContext block)
90+
{
91+
return block?.children != null && ContainsExecutableStatements(block.children);
92+
}
7993
}
8094
}

‎Rubberduck.CodeAnalysis/Inspections/Concrete/EmptyForEachBlockInspection.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public class EmptyForEachBlockListener : EmptyBlockInspectionListenerBase
3333
{
3434
public override void EnterForEachStmt([NotNull] VBAParser.ForEachStmtContext context)
3535
{
36-
InspectBlockForExecutableStatements(context.block(), context);
36+
InspectBlockForExecutableStatements(context.unterminatedBlock(), context);
3737
}
3838
}
3939
}

‎Rubberduck.CodeAnalysis/Inspections/Concrete/EmptyForLoopBlockInspection.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public class EmptyForloopBlockListener : EmptyBlockInspectionListenerBase
3333
{
3434
public override void EnterForNextStmt([NotNull] VBAParser.ForNextStmtContext context)
3535
{
36-
InspectBlockForExecutableStatements(context.block(), context);
36+
InspectBlockForExecutableStatements(context.unterminatedBlock(), context);
3737
}
3838
}
3939
}

‎Rubberduck.Parsing/Grammar/VBAParser.g4‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ moduleBodyElement :
108108

109109
block : (blockStmt endOfStatement)*;
110110

111+
unterminatedBlock : blockStmt (endOfStatement blockStmt)*;
112+
111113
blockStmt :
112114
statementLabelDefinition whiteSpace? mainBlockStmt?
113115
| mainBlockStmt
@@ -351,16 +353,18 @@ eventStmt : (visibility whiteSpace)? EVENT whiteSpace identifier whiteSpace? arg
351353
exitStmt : EXIT_DO | EXIT_FOR | EXIT_FUNCTION | EXIT_PROPERTY | EXIT_SUB;
352354

353355
forEachStmt :
354-
FOR whiteSpace EACH whiteSpace expression whiteSpace IN whiteSpace expression endOfStatement
355-
block
356-
statementLabelDefinition? whiteSpace? NEXT (whiteSpace expression)?
356+
FOR whiteSpace EACH whiteSpace expression whiteSpace IN whiteSpace expression
357+
(endOfStatement unterminatedBlock)?
358+
(endOfStatement statementLabelDefinition? whiteSpace? NEXT (whiteSpace expression)?
359+
| whiteSpace? COMMA whiteSpace? expression)
357360
;
358361

359362
// expression EQ expression refactored to expression to allow SLL
360363
forNextStmt :
361-
FOR whiteSpace expression whiteSpace TO whiteSpace expression stepStmt? whiteSpace* endOfStatement
362-
block
363-
statementLabelDefinition? whiteSpace? NEXT (whiteSpace expression)?
364+
FOR whiteSpace expression whiteSpace TO whiteSpace expression stepStmt? whiteSpace*
365+
(endOfStatement unterminatedBlock)?
366+
(endOfStatement statementLabelDefinition? whiteSpace? NEXT (whiteSpace expression)?
367+
| whiteSpace? COMMA whiteSpace? expression)
364368
;
365369

366370
stepStmt : whiteSpace STEP whiteSpace expression;

‎RubberduckTests/Common/DeploymentItemAttribute.cs‎

Lines changed: 0 additions & 57 deletions
This file was deleted.

‎RubberduckTests/Grammar/VBAParserTests.cs‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,6 +1484,126 @@ Next n%
14841484
AssertTree(parseResult.Item1, parseResult.Item2, "//forNextStmt");
14851485
}
14861486

1487+
[Category("Parser")]
1488+
[Test]
1489+
public void TestCombinedForNextStatement()
1490+
{
1491+
string code = @"
1492+
Sub Test()
1493+
For n = 1 To 10
1494+
For m = 1 To 20
1495+
a = m + n
1496+
Next m _
1497+
, n%
1498+
End Sub";
1499+
var parseResult = Parse(code);
1500+
AssertTree(parseResult.Item1, parseResult.Item2, "//forNextStmt", matches => matches.Count == 2);
1501+
}
1502+
1503+
[Category("Parser")]
1504+
[Test]
1505+
public void TestCombinedForNextStatementWhithItermediateCode()
1506+
{
1507+
string code = @"
1508+
Sub Test()
1509+
For n = 1 To 10
1510+
b = n
1511+
For m = 1 To 20
1512+
a = m + n
1513+
Next m,n%
1514+
End Sub";
1515+
var parseResult = Parse(code);
1516+
AssertTree(parseResult.Item1, parseResult.Item2, "//forNextStmt", matches => matches.Count == 2);
1517+
}
1518+
1519+
[Category("Parser")]
1520+
[Test]
1521+
public void TestCombinedForEachStatement()
1522+
{
1523+
string code = @"
1524+
Sub Test()
1525+
Dim foo As Collection
1526+
Dim bar As Collection
1527+
For Each n In foo
1528+
For Each m In bar
1529+
a = m + n
1530+
Next m _
1531+
, _
1532+
n%
1533+
End Sub";
1534+
var parseResult = Parse(code);
1535+
AssertTree(parseResult.Item1, parseResult.Item2, "//forEachStmt", matches => matches.Count == 2);
1536+
}
1537+
1538+
[Category("Parser")]
1539+
[Test]
1540+
public void TestCombinedForEachStatementWhithItermediateCode()
1541+
{
1542+
string code = @"
1543+
Sub Test()
1544+
Dim foo As Collection
1545+
Dim bar As Collection
1546+
For Each n In foo
1547+
b = n
1548+
For Each m In bar
1549+
a = m + n
1550+
Next m,n%
1551+
End Sub";
1552+
var parseResult = Parse(code);
1553+
AssertTree(parseResult.Item1, parseResult.Item2, "//forEachStmt", matches => matches.Count == 2);
1554+
}
1555+
1556+
[Category("Parser")]
1557+
[Test]
1558+
public void TestMixedCombinedForEachAndForNextStatement()
1559+
{
1560+
string code = @"
1561+
Sub Test()
1562+
Dim foo As Collection
1563+
Dim bar As Collection
1564+
For n = 1 To 10
1565+
b = n
1566+
For Each c In foo
1567+
For m = 1 To 20
1568+
For Each d In bar
1569+
a = m + n + c + d
1570+
For k = 0 To 100
1571+
t = a + k
1572+
Next k, d, m,_
1573+
c, _
1574+
n
1575+
End Sub";
1576+
var parseResult = Parse(code);
1577+
AssertTree(parseResult.Item1, parseResult.Item2, "//forEachStmt", matches => matches.Count == 2);
1578+
AssertTree(parseResult.Item1, parseResult.Item2, "//forNextStmt", matches => matches.Count == 3);
1579+
}
1580+
1581+
[Category("Parser")]
1582+
[Test]
1583+
public void TestMixedRegularAndCombinedForEachAndForNextStatement()
1584+
{
1585+
string code = @"
1586+
Sub Test()
1587+
Dim foo As Collection
1588+
Dim bar As Collection
1589+
For n = 1 To 10
1590+
For Each c In foo
1591+
Next c
1592+
For m = 1 To 20
1593+
For k = 0 To 100
1594+
t = a + k
1595+
Next
1596+
For Each d In bar
1597+
For l = 15 To 23
1598+
a = m + n + d + l
1599+
Next l, d
1600+
Next m, n
1601+
End Sub";
1602+
var parseResult = Parse(code);
1603+
AssertTree(parseResult.Item1, parseResult.Item2, "//forEachStmt", matches => matches.Count == 2);
1604+
AssertTree(parseResult.Item1, parseResult.Item2, "//forNextStmt", matches => matches.Count == 4);
1605+
}
1606+
14871607
[Category("Parser")]
14881608
[Test]
14891609
public void TestLineLabelStatement()

‎RubberduckTests/Grammar/VBAParserValidityTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,10 @@
55
using System.Collections.Generic;
66
using System.IO;
77
using System.Linq;
8+
using System.Reflection;
89
using System.Threading;
910
using Rubberduck.VBEditor.SafeComWrappers.Abstract;
1011
using Rubberduck.VBEditor;
11-
using Rubberduck.Parsing.Symbols;
12-
using RubberduckTests.Common;
1312

1413
namespace RubberduckTests.Grammar
1514
{
@@ -20,7 +19,6 @@ public class VBAParserValidityTests
2019
[Category("LongRunning")]
2120
[Category("Grammar")]
2221
[Category("Parser")]
23-
[DeploymentItem(@"Testfiles\")]
2422
public void TestParser()
2523
{
2624
foreach (var testfile in GetTestFiles())
@@ -38,7 +36,9 @@ private void AssertParseResult(string filename, string originalCode, string mate
3836

3937
private IEnumerable<Tuple<string, string>> GetTestFiles()
4038
{
41-
return Directory.EnumerateFiles("Testfiles//Grammar").Select(file => Tuple.Create(file, File.ReadAllText(file))).ToList();
39+
var basePath = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
40+
basePath = Directory.GetParent(basePath).Parent.FullName;
41+
return Directory.EnumerateFiles(Path.Combine(basePath, "Testfiles//Grammar")).Select(file => Tuple.Create(file, File.ReadAllText(file))).ToList();
4242
}
4343

4444
private static string Parse(string code, string filename)

‎RubberduckTests/Inspections/ApplicationWorksheetFunctionInspectionTests.cs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using Rubberduck.Inspections.Concrete;
55
using Rubberduck.Parsing.VBA;
66
using Rubberduck.VBEditor.SafeComWrappers;
7-
using RubberduckTests.Common;
87
using RubberduckTests.Mocks;
98

109
namespace RubberduckTests.Inspections
@@ -36,7 +35,6 @@ private static RubberduckParserState ArrangeParserAndParse(string inputCode)
3635
}
3736

3837
[Test]
39-
[DeploymentItem(@"TestFiles\")]
4038
[Category("Inspections")]
4139
public void ApplicationWorksheetFunction_ReturnsResult_GlobalApplication()
4240
{
@@ -58,7 +56,6 @@ End Sub
5856
}
5957

6058
[Test]
61-
[DeploymentItem(@"TestFiles\")]
6259
[Category("Inspections")]
6360
public void ApplicationWorksheetFunction_ReturnsResult_WithGlobalApplication()
6461
{
@@ -82,7 +79,6 @@ End Sub
8279
}
8380

8481
[Test]
85-
[DeploymentItem(@"TestFiles\")]
8682
[Category("Inspections")]
8783
public void ApplicationWorksheetFunction_ReturnsResult_ApplicationVariable()
8884
{
@@ -106,7 +102,6 @@ End Sub
106102
}
107103

108104
[Test]
109-
[DeploymentItem(@"TestFiles\")]
110105
[Category("Inspections")]
111106
public void ApplicationWorksheetFunction_ReturnsResult_WithApplicationVariable()
112107
{
@@ -132,7 +127,6 @@ End Sub
132127
}
133128

134129
[Test]
135-
[DeploymentItem(@"TestFiles\")]
136130
[Category("Inspections")]
137131
public void ApplicationWorksheetFunction_DoesNotReturnResult_ExplicitUseGlobalApplication()
138132
{
@@ -154,7 +148,6 @@ End Sub
154148
}
155149

156150
[Test]
157-
[DeploymentItem(@"TestFiles\")]
158151
[Category("Inspections")]
159152
public void ApplicationWorksheetFunction_DoesNotReturnResult_ExplicitUseApplicationVariable()
160153
{
@@ -209,7 +202,6 @@ End Sub
209202
}
210203

211204
[Test]
212-
[DeploymentItem(@"Testfiles\")]
213205
[Category("Inspections")]
214206
public void ApplicationWorksheetFunction_Ignored_DoesNotReturnResult()
215207
{

‎RubberduckTests/Inspections/HostSpecificExpressionInspectionTests.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using Rubberduck.Parsing.VBA;
77
using Rubberduck.VBEditor.SafeComWrappers;
88
using Rubberduck.VBEditor.SafeComWrappers.Abstract;
9-
using RubberduckTests.Common;
109
using RubberduckTests.Mocks;
1110

1211
namespace RubberduckTests.Inspections
@@ -16,7 +15,6 @@ public class HostSpecificExpressionInspectionTests
1615
{
1716
[Test]
1817
[Category("Inspections")]
19-
[DeploymentItem(@"TestFiles\")]
2018
public void ReturnsResultForExpressionOnLeftHandSide()
2119
{
2220
const string code = @"

0 commit comments

Comments
 (0)