Skip to content

Commit 6759c49

Browse files
retailcoderretailcoder
authored
Merge pull request #3456 from Hosch250/booleanAssignment
Introduces inspection and quickfix to identify and simplify overly verbose Boolean assignments.
2 parents 4d72489 + f2e9e7f commit 6759c49

10 files changed

+731
-41
lines changed

‎Rubberduck.Inspections/Concrete/BooleanAssignedInIfElseInspection.cs

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

‎Rubberduck.Inspections/QuickFixes/ReplaceIfElseWithConditionalStatementQuickFix.cs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
using Rubberduck.Inspections.Abstract;
2+
using Rubberduck.Inspections.Concrete;
3+
using Rubberduck.Parsing.Grammar;
4+
using Rubberduck.Parsing.Inspections.Abstract;
5+
using Rubberduck.Parsing.Inspections.Resources;
6+
using Rubberduck.Parsing.Symbols;
7+
using Rubberduck.Parsing.VBA;
8+
9+
namespace Rubberduck.Inspections.QuickFixes
10+
{
11+
public sealed class ReplaceIfElseWithConditionalStatementQuickFix : QuickFixBase
12+
{
13+
private readonly RubberduckParserState _state;
14+
15+
public ReplaceIfElseWithConditionalStatementQuickFix(RubberduckParserState state)
16+
: base(typeof(BooleanAssignedInIfElseInspection))
17+
{
18+
_state = state;
19+
}
20+
21+
public override void Fix(IInspectionResult result)
22+
{
23+
var ifContext = (VBAParser.IfStmtContext) result.Context;
24+
var letStmt = ParserRuleContextHelper.GetDescendent<VBAParser.LetStmtContext>(ifContext.block());
25+
26+
var conditional = ifContext.booleanExpression().GetText();
27+
28+
if (letStmt.expression().GetText() == Tokens.False)
29+
{
30+
conditional = $"Not ({conditional})";
31+
}
32+
33+
var rewriter = _state.GetRewriter(result.QualifiedSelection.QualifiedName);
34+
rewriter.Replace(result.Context, $"{letStmt.lExpression().GetText()} = {conditional}");
35+
}
36+
37+
public override string Description(IInspectionResult result)
38+
{
39+
return InspectionsUI.ReplaceIfElseWithConditionalStatementQuickFix;
40+
}
41+
42+
public override bool CanFixInProcedure { get; } = true;
43+
public override bool CanFixInModule { get; } = true;
44+
public override bool CanFixInProject { get; } = true;
45+
}
46+
}

‎Rubberduck.Inspections/Rubberduck.Inspections.csproj

Lines changed: 2 additions & 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\ObsoleteErrorSyntaxInspection.cs" />
7273
<Compile Include="Concrete\StopKeywordInspection.cs" />
@@ -116,6 +117,7 @@
116117
<Compile Include="Concrete\ProcedureCanBeWrittenAsFunctionInspection.cs" />
117118
<Compile Include="Concrete\ProcedureNotUsedInspection.cs" />
118119
<Compile Include="Properties\AssemblyInfo.cs" />
120+
<Compile Include="QuickFixes\ReplaceIfElseWithConditionalStatementQuickFix.cs" />
119121
<Compile Include="QuickFixes\AddIdentifierToWhiteListQuickFix.cs" />
120122
<Compile Include="QuickFixes\ApplicationWorksheetFunctionQuickFix.cs" />
121123
<Compile Include="QuickFixes\AssignedByValParameterMakeLocalCopyQuickFix.cs" />

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<?xml version="1.0" encoding="UTF-8"?>
1+
<?xml version="1.0" encoding="utf-8"?>
22
<root>
33
<!--
44
Microsoft ResX Schema
@@ -59,7 +59,7 @@
5959
: using a System.ComponentModel.TypeConverter
6060
: and then encoded with base64 encoding.
6161
-->
62-
<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:msdata="urn:schemas-microsoft-com:xml-msdata" id="root">
62+
<xsd:schema id="root" xmlns="" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:msdata="urn:schemas-microsoft-com:xml-msdata">
6363
<xsd:import namespace="http://www.w3.org/XML/1998/namespace" />
6464
<xsd:element name="root" msdata:IsDataSet="true">
6565
<xsd:complexType>
@@ -866,4 +866,16 @@ If the parameter can be null, ignore this inspection result; passing a null valu
866866
<data name="ReplaceObsoleteErrorStatementQuickFix" xml:space="preserve">
867867
<value>Replace 'Error' with 'Err.Raise'</value>
868868
</data>
869-
</root>
869+
<data name="BooleanAssignedInIfElseInspectionMeta" xml:space="preserve">
870+
<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>
871+
</data>
872+
<data name="BooleanAssignedInIfElseInspectionName" xml:space="preserve">
873+
<value>Boolean literal assignment in conditional</value>
874+
</data>
875+
<data name="BooleanAssignedInIfElseInspectionResultFormat" xml:space="preserve">
876+
<value>Boolean literal '{0}' assigned in conditional</value>
877+
</data>
878+
<data name="ReplaceIfElseWithConditionalStatementQuickFix" xml:space="preserve">
879+
<value>Replace If/Else with single assignment</value>
880+
</data>
881+
</root>

0 commit comments

Comments
 (0)