Skip to content

Commit b44c020

Browse files
Hosch250Hosch250
committed
Add inspection
1 parent 72a53d3 commit b44c020

File tree

8 files changed

+403
-0
lines changed

8 files changed

+403
-0
lines changed

‎Rubberduck.Inspections/Concrete/BooleanAssignedInIfElseInspection.cs‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using Antlr4.Runtime;
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.Inspections.Resources;
11+
using Rubberduck.Parsing.Symbols;
12+
using Rubberduck.Parsing.VBA;
13+
using Rubberduck.VBEditor;
14+
15+
namespace Rubberduck.Inspections.Concrete
16+
{
17+
public sealed class BooleanAssignedInIfElseInspection : ParseTreeInspectionBase
18+
{
19+
public BooleanAssignedInIfElseInspection(RubberduckParserState state)
20+
: base(state) { }
21+
22+
public override Type Type => typeof(BooleanAssignedInIfElseInspection);
23+
24+
public override CodeInspectionType InspectionType => CodeInspectionType.LanguageOpportunities;
25+
26+
public override IInspectionListener Listener { get; } =
27+
new BooleanAssignedInIfElseListener();
28+
29+
public override IEnumerable<IInspectionResult> GetInspectionResults()
30+
{
31+
return Listener.Contexts
32+
.Where(result => !IsIgnoringInspectionResultFor(result.ModuleName, result.Context.Start.Line))
33+
.Select(result => new QualifiedContextInspectionResult(this,
34+
string.Format(InspectionsUI.BooleanAssignedInIfElseInspectionResultFormat,
35+
ParserRuleContextHelper.GetDescendent<VBAParser.LetStmtContext>(((VBAParser.IfStmtContext)result.Context).block()).lExpression().GetText().Trim()),
36+
result));
37+
}
38+
39+
public class BooleanAssignedInIfElseListener : VBAParserBaseListener, IInspectionListener
40+
{
41+
private readonly List<QualifiedContext<ParserRuleContext>> _contexts = new List<QualifiedContext<ParserRuleContext>>();
42+
public IReadOnlyList<QualifiedContext<ParserRuleContext>> Contexts => _contexts;
43+
44+
public QualifiedModuleName CurrentModuleName { get; set; }
45+
46+
public void ClearContexts()
47+
{
48+
_contexts.Clear();
49+
}
50+
51+
public override void ExitIfStmt(VBAParser.IfStmtContext context)
52+
{
53+
if (context.elseIfBlock().Any())
54+
{
55+
return;
56+
}
57+
58+
if (!IsSingleBooleanAssignment(context.block()) ||
59+
!IsSingleBooleanAssignment(context.elseBlock().block()))
60+
{
61+
return;
62+
}
63+
64+
// make sure the assignments are the opposite
65+
if (!(ParserRuleContextHelper.GetDescendent<VBAParser.BooleanLiteralIdentifierContext>(context.block()).GetText() == Tokens.True ^
66+
ParserRuleContextHelper.GetDescendent<VBAParser.BooleanLiteralIdentifierContext>(context.elseBlock().block()).GetText() == Tokens.True))
67+
{
68+
return;
69+
}
70+
71+
if (ParserRuleContextHelper.GetDescendent<VBAParser.LetStmtContext>(context.block()).lExpression().GetText().ToLowerInvariant() !=
72+
ParserRuleContextHelper.GetDescendent<VBAParser.LetStmtContext>(context.elseBlock().block()).lExpression().GetText().ToLowerInvariant())
73+
{
74+
return;
75+
}
76+
77+
_contexts.Add(new QualifiedContext<ParserRuleContext>(CurrentModuleName, context));
78+
}
79+
80+
private bool IsSingleBooleanAssignment(VBAParser.BlockContext block)
81+
{
82+
if (block.ChildCount != 2)
83+
{
84+
return false;
85+
}
86+
87+
var mainBlockStmtContext =
88+
ParserRuleContextHelper.GetDescendent<VBAParser.MainBlockStmtContext>(block);
89+
90+
return mainBlockStmtContext.children.FirstOrDefault() is VBAParser.LetStmtContext letStmt &&
91+
letStmt.expression() is VBAParser.LiteralExprContext literal &&
92+
ParserRuleContextHelper.GetDescendent<VBAParser.BooleanLiteralIdentifierContext>(literal) != null;
93+
}
94+
}
95+
}
96+
}

‎Rubberduck.Inspections/Rubberduck.Inspections.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
<Compile Include="Concrete\EmptyDoWhileBlockInspection.cs" />
6868
<Compile Include="Concrete\EmptyForEachBlockInspection.cs" />
6969
<Compile Include="Concrete\EmptyForLoopBlockInspection.cs" />
70+
<Compile Include="Concrete\BooleanAssignedInIfElseInspection.cs" />
7071
<Compile Include="Concrete\EmptyWhileWendBlockInspection.cs" />
7172
<Compile Include="Concrete\StopKeywordInspection.cs" />
7273
<Compile Include="Concrete\LineLabelNotUsedInspection.cs" />

‎Rubberduck.Parsing/Inspections/Resources/InspectionsUI.resx‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -857,4 +857,13 @@ If the parameter can be null, ignore this inspection result; passing a null valu
857857
<data name="ShadowedDeclarationInspectionResultFormat" xml:space="preserve">
858858
<value>{0} '{1}' hides {2} '{3}'</value>
859859
</data>
860+
<data name="BooleanAssignedInIfElseInspectionMeta" xml:space="preserve">
861+
<value>A member is assigned True/False in different branches of an if statement with no other statements in the conditional. Use the condition directly to the member instead.</value>
862+
</data>
863+
<data name="BooleanAssignedInIfElseInspectionName" xml:space="preserve">
864+
<value>Boolean literal assignment in conditional</value>
865+
</data>
866+
<data name="BooleanAssignedInIfElseInspectionResultFormat" xml:space="preserve">
867+
<value>Boolean literal '{0}' assigned in conditional</value>
868+
</data>
860869
</root>

‎Rubberduck.Parsing/Inspections/Resources/InspectionsUI1.Designer.cs‎

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rubberduck.Parsing/Symbols/ParserRuleContextHelper.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System;
44
using System.Collections.Generic;
55
using System.Linq;
6+
using Antlr4.Runtime.Tree;
67

78
namespace Rubberduck.Parsing.Symbols
89
{
@@ -104,5 +105,24 @@ public static T GetDescendent<T>(RuleContext context)
104105

105106
return default(T);
106107
}
108+
109+
public static IEnumerable<IParseTree> GetDescendents(IParseTree context)
110+
{
111+
if (context == null)
112+
{
113+
yield break;
114+
}
115+
116+
for (var index = 0; index < context.ChildCount; index++)
117+
{
118+
var child = context.GetChild(index);
119+
yield return child;
120+
121+
foreach (var node in GetDescendents(child))
122+
{
123+
yield return node;
124+
}
125+
}
126+
}
107127
}
108128
}

‎RubberduckTests/Inspections/BooleanAssignedInIfElseInspectionTests.cs‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
using System.Linq;
2+
using System.Threading;
3+
using Microsoft.VisualStudio.TestTools.UnitTesting;
4+
using Rubberduck.Inspections.Concrete;
5+
using RubberduckTests.Mocks;
6+
7+
namespace RubberduckTests.Inspections
8+
{
9+
[TestClass]
10+
public class BooleanAssignedInIfElseInspectionTests
11+
{
12+
[TestMethod]
13+
[TestCategory("Inspections")]
14+
public void Simple()
15+
{
16+
const string inputcode =
17+
@"Sub Foo()
18+
Dim d As Boolean
19+
If True Then
20+
d = True
21+
Else
22+
d = False
23+
EndIf
24+
End Sub";
25+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputcode, out _);
26+
var state = MockParser.CreateAndParse(vbe.Object);
27+
28+
var inspection = new BooleanAssignedInIfElseInspection(state);
29+
var inspector = InspectionsHelper.GetInspector(inspection);
30+
var results = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
31+
32+
Assert.AreEqual(1, results.Count());
33+
}
34+
35+
[TestMethod]
36+
[TestCategory("Inspections")]
37+
public void MultipleResults()
38+
{
39+
const string inputcode =
40+
@"Sub Foo()
41+
Dim d As Boolean
42+
If True Then
43+
d = True
44+
Else
45+
d = False
46+
EndIf
47+
48+
If True Then
49+
d = False
50+
Else
51+
d = True
52+
EndIf
53+
End Sub";
54+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputcode, out _);
55+
var state = MockParser.CreateAndParse(vbe.Object);
56+
57+
var inspection = new BooleanAssignedInIfElseInspection(state);
58+
var inspector = InspectionsHelper.GetInspector(inspection);
59+
var results = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
60+
61+
Assert.AreEqual(2, results.Count());
62+
}
63+
64+
[TestMethod]
65+
[TestCategory("Inspections")]
66+
public void AssignsInteger()
67+
{
68+
const string inputcode =
69+
@"Sub Foo()
70+
Dim d
71+
If True Then
72+
d = 0
73+
Else
74+
d = 1
75+
EndIf
76+
End Sub";
77+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputcode, out _);
78+
var state = MockParser.CreateAndParse(vbe.Object);
79+
80+
var inspection = new BooleanAssignedInIfElseInspection(state);
81+
var inspector = InspectionsHelper.GetInspector(inspection);
82+
var results = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
83+
84+
Assert.IsFalse(results.Any());
85+
}
86+
87+
[TestMethod]
88+
[TestCategory("Inspections")]
89+
public void AssignsTheSameValue() // worthy of an inspection in its own right
90+
{
91+
const string inputcode =
92+
@"Sub Foo()
93+
Dim d
94+
If True Then
95+
d = True
96+
Else
97+
d = True
98+
EndIf
99+
End Sub";
100+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputcode, out _);
101+
var state = MockParser.CreateAndParse(vbe.Object);
102+
103+
var inspection = new BooleanAssignedInIfElseInspection(state);
104+
var inspector = InspectionsHelper.GetInspector(inspection);
105+
var results = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
106+
107+
Assert.IsFalse(results.Any());
108+
}
109+
110+
[TestMethod]
111+
[TestCategory("Inspections")]
112+
public void AssignsToDifferentVariables()
113+
{
114+
const string inputcode =
115+
@"Sub Foo()
116+
Dim d1, d2
117+
If True Then
118+
d1 = True
119+
Else
120+
d2 = False
121+
EndIf
122+
End Sub";
123+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputcode, out _);
124+
var state = MockParser.CreateAndParse(vbe.Object);
125+
126+
var inspection = new BooleanAssignedInIfElseInspection(state);
127+
var inspector = InspectionsHelper.GetInspector(inspection);
128+
var results = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
129+
130+
Assert.IsFalse(results.Any());
131+
}
132+
133+
[TestMethod]
134+
[TestCategory("Inspections")]
135+
public void ConditionalContainsElseIfBlock()
136+
{
137+
const string inputcode =
138+
@"Sub Foo()
139+
Dim d
140+
If True Then
141+
d = True
142+
ElseIf False Then
143+
d = True
144+
Else
145+
d = False
146+
EndIf
147+
End Sub";
148+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputcode, out _);
149+
var state = MockParser.CreateAndParse(vbe.Object);
150+
151+
var inspection = new BooleanAssignedInIfElseInspection(state);
152+
var inspector = InspectionsHelper.GetInspector(inspection);
153+
var results = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
154+
155+
Assert.IsFalse(results.Any());
156+
}
157+
158+
[TestMethod]
159+
[TestCategory("Inspections")]
160+
public void ConditionalDoesNotContainElseBlock()
161+
{
162+
const string inputcode =
163+
@"Sub Foo()
164+
Dim d
165+
If True Then
166+
d = True
167+
EndIf
168+
End Sub";
169+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputcode, out _);
170+
var state = MockParser.CreateAndParse(vbe.Object);
171+
172+
var inspection = new BooleanAssignedInIfElseInspection(state);
173+
var inspector = InspectionsHelper.GetInspector(inspection);
174+
var results = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
175+
176+
Assert.IsFalse(results.Any());
177+
}
178+
179+
[TestMethod]
180+
[TestCategory("Inspections")]
181+
public void IsIgnored()
182+
{
183+
const string inputcode =
184+
@"Sub Foo()
185+
Dim d
186+
'@Ignore BooleanAssignedInIfElse
187+
If True Then
188+
d = True
189+
Else
190+
d = False
191+
EndIf
192+
End Sub";
193+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputcode, out _);
194+
var state = MockParser.CreateAndParse(vbe.Object);
195+
196+
var inspection = new BooleanAssignedInIfElseInspection(state);
197+
var inspector = InspectionsHelper.GetInspector(inspection);
198+
var results = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
199+
200+
Assert.IsFalse(results.Any());
201+
}
202+
}
203+
}

0 commit comments

Comments
 (0)