Skip to content

Commit 4bfc3a1

Browse files
retailcoderretailcoder
committed
Merge pull request #1728 from Hosch250/InspectionBugs
Fix Use Meaningful Name selection and Remove Identifier Not Used--Var…
2 parents 7587f7c + e089115 commit 4bfc3a1

File tree

3 files changed

+181
-15
lines changed

3 files changed

+181
-15
lines changed

‎RetailCoder.VBE/Common/DeclarationExtensions.cs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,25 @@ public static Selection GetVariableStmtContextSelection(this Declaration target)
4848
statement.Stop.Line, statement.Stop.Column);
4949
}
5050

51+
/// <summary>
52+
/// Returns the Selection of a ConstStmtContext.
53+
/// </summary>
54+
/// <exception cref="ArgumentException">Throws when target's DeclarationType is not Constant.</exception>
55+
/// <param name="target"></param>
56+
/// <returns></returns>
57+
public static Selection GetConstStmtContextSelection(this Declaration target)
58+
{
59+
if (target.DeclarationType != DeclarationType.Constant)
60+
{
61+
throw new ArgumentException("Target DeclarationType is not Constant.", "target");
62+
}
63+
64+
var statement = GetConstStmtContext(target);
65+
66+
return new Selection(statement.Start.Line, statement.Start.Column,
67+
statement.Stop.Line, statement.Stop.Column);
68+
}
69+
5170
/// <summary>
5271
/// Returns a VariableStmtContext.
5372
/// </summary>
@@ -70,6 +89,28 @@ public static VBAParser.VariableStmtContext GetVariableStmtContext(this Declarat
7089
return statement;
7190
}
7291

92+
/// <summary>
93+
/// Returns a ConstStmtContext.
94+
/// </summary>
95+
/// <exception cref="ArgumentException">Throws when target's DeclarationType is not Constant.</exception>
96+
/// <param name="target"></param>
97+
/// <returns></returns>
98+
public static VBAParser.ConstStmtContext GetConstStmtContext(this Declaration target)
99+
{
100+
if (target.DeclarationType != DeclarationType.Constant)
101+
{
102+
throw new ArgumentException("Target DeclarationType is not Constant.", "target");
103+
}
104+
105+
var statement = target.Context.Parent as VBAParser.ConstStmtContext;
106+
if (statement == null)
107+
{
108+
throw new MissingMemberException("Statement not found");
109+
}
110+
111+
return statement;
112+
}
113+
73114
/// <summary>
74115
/// Returns whether a variable declaration statement contains multiple declarations in a single statement.
75116
/// </summary>

‎RetailCoder.VBE/Inspections/IdentifierNotUsedInspectionResult.cs

Lines changed: 135 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1+
using System;
12
using System.Collections.Generic;
3+
using System.Linq;
24
using Antlr4.Runtime;
35
using Rubberduck.Common;
46
using Rubberduck.Parsing.Symbols;
7+
using Rubberduck.Parsing.VBA;
58
using Rubberduck.UI;
69
using Rubberduck.VBEditor;
10+
using Rubberduck.VBEditor.Extensions;
711

812
namespace Rubberduck.Inspections
913
{
@@ -17,7 +21,7 @@ public IdentifierNotUsedInspectionResult(IInspection inspection, Declaration tar
1721
{
1822
_quickFixes = new CodeInspectionQuickFix[]
1923
{
20-
new RemoveUnusedDeclarationQuickFix(context, QualifiedSelection),
24+
new RemoveUnusedDeclarationQuickFix(context, QualifiedSelection, Target),
2125
new IgnoreOnceQuickFix(context, QualifiedSelection, Inspection.AnnotationName),
2226
};
2327
}
@@ -42,32 +46,148 @@ public override NavigateCodeEventArgs GetNavigationArgs()
4246
/// </summary>
4347
public class RemoveUnusedDeclarationQuickFix : CodeInspectionQuickFix
4448
{
45-
public RemoveUnusedDeclarationQuickFix(ParserRuleContext context, QualifiedSelection selection)
49+
private readonly Declaration _target;
50+
51+
public RemoveUnusedDeclarationQuickFix(ParserRuleContext context, QualifiedSelection selection, Declaration target)
4652
: base(context, selection, InspectionsUI.RemoveUnusedDeclarationQuickFix)
4753
{
54+
_target = target;
4855
}
4956

5057
public override void Fix()
5158
{
52-
var module = Selection.QualifiedName.Component.CodeModule;
53-
var selection = Selection.Selection;
59+
if (_target.DeclarationType == DeclarationType.Variable || _target.DeclarationType == DeclarationType.Constant)
60+
{
61+
RemoveVariable(_target);
62+
}
63+
else
64+
{
65+
var module = Selection.QualifiedName.Component.CodeModule;
66+
var selection = Selection.Selection;
67+
68+
var originalCodeLines = module.Lines[selection.StartLine, selection.LineCount];
69+
70+
var originalInstruction = Context.GetText();
71+
module.DeleteLines(selection.StartLine, selection.LineCount);
72+
73+
var newCodeLines = originalCodeLines.Replace(originalInstruction, string.Empty);
74+
if (!string.IsNullOrEmpty(newCodeLines))
75+
{
76+
module.InsertLines(selection.StartLine, newCodeLines);
77+
}
78+
}
79+
}
80+
81+
private void RemoveVariable(Declaration target)
82+
{
83+
Selection selection;
84+
var declarationText = target.Context.GetText().Replace(" _" + Environment.NewLine, string.Empty);
85+
var multipleDeclarations = target.DeclarationType == DeclarationType.Variable && target.HasMultipleDeclarationsInStatement();
86+
87+
if (!multipleDeclarations)
88+
{
89+
declarationText = GetStmtContext(target).GetText().Replace(" _" + Environment.NewLine, string.Empty);
90+
selection = GetStmtContextSelection(target);
91+
}
92+
else
93+
{
94+
selection = new Selection(target.Context.Start.Line, target.Context.Start.Column,
95+
target.Context.Stop.Line, target.Context.Stop.Column);
96+
}
5497

55-
var originalCodeLines = module.get_Lines(selection.StartLine, selection.LineCount)
56-
.Replace("\r\n", " ")
57-
.Replace("_", string.Empty);
98+
var codeModule = target.QualifiedName.QualifiedModuleName.Component.CodeModule;
99+
var oldLines = codeModule.GetLines(selection);
58100

59-
var originalInstruction = Context.GetText();
60-
module.DeleteLines(selection.StartLine, selection.LineCount);
101+
var newLines = oldLines.Replace(" _" + Environment.NewLine, string.Empty)
102+
.Remove(selection.StartColumn, declarationText.Length);
61103

62-
var newInstruction = string.Empty;
63-
var newCodeLines = string.IsNullOrEmpty(newInstruction)
64-
? string.Empty
65-
: originalCodeLines.Replace(originalInstruction, newInstruction);
104+
if (multipleDeclarations)
105+
{
106+
selection = GetStmtContextSelection(target);
107+
newLines = RemoveExtraComma(codeModule.GetLines(selection).Replace(oldLines, newLines),
108+
target.CountOfDeclarationsInStatement(), target.IndexOfVariableDeclarationInStatement());
109+
}
66110

67-
if (!string.IsNullOrEmpty(newCodeLines))
111+
var newLinesWithoutExcessSpaces = newLines.Split(new[] { Environment.NewLine }, StringSplitOptions.None);
112+
for (var i = 0; i < newLinesWithoutExcessSpaces.Length; i++)
68113
{
69-
module.InsertLines(selection.StartLine, newCodeLines);
114+
newLinesWithoutExcessSpaces[i] = newLinesWithoutExcessSpaces[i].RemoveExtraSpacesLeavingIndentation();
70115
}
116+
117+
for (var i = newLinesWithoutExcessSpaces.Length - 1; i >= 0; i--)
118+
{
119+
if (newLinesWithoutExcessSpaces[i].Trim() == string.Empty)
120+
{
121+
continue;
122+
}
123+
124+
if (newLinesWithoutExcessSpaces[i].EndsWith(" _"))
125+
{
126+
newLinesWithoutExcessSpaces[i] =
127+
newLinesWithoutExcessSpaces[i].Remove(newLinesWithoutExcessSpaces[i].Length - 2);
128+
}
129+
break;
130+
}
131+
132+
// remove all lines with only whitespace
133+
newLinesWithoutExcessSpaces = newLinesWithoutExcessSpaces.Where(str => str.Any(c => !char.IsWhiteSpace(c))).ToArray();
134+
135+
codeModule.DeleteLines(selection);
136+
if (newLinesWithoutExcessSpaces.Any())
137+
{
138+
codeModule.InsertLines(selection.StartLine,
139+
string.Join(Environment.NewLine, newLinesWithoutExcessSpaces));
140+
}
141+
}
142+
143+
private Selection GetStmtContextSelection(Declaration target)
144+
{
145+
return target.DeclarationType == DeclarationType.Variable
146+
? target.GetVariableStmtContextSelection()
147+
: target.GetConstStmtContextSelection();
148+
}
149+
150+
private ParserRuleContext GetStmtContext(Declaration target)
151+
{
152+
return target.DeclarationType == DeclarationType.Variable
153+
? (ParserRuleContext)target.GetVariableStmtContext()
154+
: (ParserRuleContext)target.GetConstStmtContext();
155+
}
156+
157+
private string RemoveExtraComma(string str, int numParams, int indexRemoved)
158+
{
159+
// Example use cases for this method (fields and variables):
160+
// Dim fizz as Boolean, dizz as Double
161+
// Private fizz as Boolean, dizz as Double
162+
// Public fizz as Boolean, _
163+
// dizz as Double
164+
// Private fizz as Boolean _
165+
// , dizz as Double _
166+
// , iizz as Integer
167+
168+
// Before this method is called, the parameter to be removed has
169+
// already been removed. This means 'str' will look like:
170+
// Dim fizz as Boolean,
171+
// Private , dizz as Double
172+
// Public fizz as Boolean, _
173+
//
174+
// Private _
175+
// , dizz as Double _
176+
// , iizz as Integer
177+
178+
// This method is responsible for removing the redundant comma
179+
// and returning a string similar to:
180+
// Dim fizz as Boolean
181+
// Private dizz as Double
182+
// Public fizz as Boolean _
183+
//
184+
// Private _
185+
// dizz as Double _
186+
// , iizz as Integer
187+
188+
var commaToRemove = numParams == indexRemoved ? indexRemoved - 1 : indexRemoved;
189+
190+
return str.Remove(str.NthIndexOf(',', commaToRemove), 1);
71191
}
72192
}
73193
}

‎RetailCoder.VBE/Inspections/UseMeaningfulNameInspectionResult.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ public override string Description
3030
{
3131
get { return string.Format(InspectionsUI.UseMeaningfulNameInspectionResultFormat, RubberduckUI.ResourceManager.GetString("DeclarationType_" + Target.DeclarationType), Target.IdentifierName); }
3232
}
33+
34+
public override NavigateCodeEventArgs GetNavigationArgs()
35+
{
36+
return new NavigateCodeEventArgs(Target);
37+
}
3338
}
3439

3540
/// <summary>

0 commit comments

Comments
 (0)