Skip to content

Commit d374061

Browse files
Hosch250Hosch250
committed
Merge pull request #899 from Hosch250/ExtractInterface
Fix broken RemoveExtraComma method. I should probably make it into a…
2 parents 7385390 + f86a8b7 commit d374061

File tree

6 files changed

+211
-114
lines changed

6 files changed

+211
-114
lines changed

‎RetailCoder.VBE/Common/DeclarationExtensions.cs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,56 @@ public static bool HasMultipleDeclarationsInStatement(this Declaration target)
7979

8080
var statement = target.Context.Parent as VBAParser.VariableListStmtContext;
8181

82-
return statement != null && statement.children.Count(i => i is VBAParser.VariableSubStmtContext) > 1;
82+
return statement != null && statement.children.OfType<VBAParser.VariableSubStmtContext>().Any();
83+
}
84+
85+
/// <summary>
86+
/// Returns the number of variable declarations in a single statement.
87+
/// </summary>
88+
/// <exception cref="ArgumentException">Throws when target's DeclarationType is not Variable.</exception>
89+
/// <param name="target"></param>
90+
/// <returns></returns>
91+
public static int CountOfDeclarationsInStatement(this Declaration target)
92+
{
93+
if (target.DeclarationType != DeclarationType.Variable)
94+
{
95+
throw new ArgumentException("Target DeclarationType is not Variable.", "target");
96+
}
97+
98+
var statement = target.Context.Parent as VBAParser.VariableListStmtContext;
99+
100+
if (statement != null)
101+
{
102+
return statement.children.OfType<VBAParser.VariableSubStmtContext>().Count();
103+
}
104+
105+
throw new ArgumentException("'target.Context.Parent' is not type VBAParser.VariabelListStmtContext", "target");
106+
}
107+
108+
/// <summary>
109+
/// Returns the number of variable declarations in a single statement. Adjusted to be 1-indexed rather than 0-indexed.
110+
/// </summary>
111+
/// <exception cref="ArgumentException">Throws when target's DeclarationType is not Variable.</exception>
112+
/// <param name="target"></param>
113+
/// <returns></returns>
114+
public static int IndexOfVariableDeclarationInStatement(this Declaration target)
115+
{
116+
if (target.DeclarationType != DeclarationType.Variable)
117+
{
118+
throw new ArgumentException("Target DeclarationType is not Variable.", "target");
119+
}
120+
121+
var statement = target.Context.Parent as VBAParser.VariableListStmtContext;
122+
123+
if (statement != null)
124+
{
125+
return statement.children.OfType<VBAParser.VariableSubStmtContext>()
126+
.ToList()
127+
.IndexOf((VBAParser.VariableSubStmtContext)target.Context) + 1;
128+
}
129+
130+
// ReSharper disable once LocalizableElement
131+
throw new ArgumentException("'target.Context.Parent' is not type VBAParser.VariabelListStmtContext", "target");
83132
}
84133

85134
public static readonly DeclarationType[] ProcedureTypes =

‎RetailCoder.VBE/Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
using System;
2-
using System.Linq;
32
using Microsoft.Vbe.Interop;
43
using Rubberduck.Common;
54
using Rubberduck.Parsing.Symbols;
5+
using Rubberduck.Parsing.VBA;
66
using Rubberduck.VBEditor;
77

88
namespace Rubberduck.Refactorings.EncapsulateField
@@ -119,7 +119,8 @@ private void RemoveField(Declaration target)
119119
if (multipleDeclarations)
120120
{
121121
selection = target.GetVariableStmtContextSelection();
122-
newLines = RemoveExtraComma(_editor.GetLines(selection).Replace(oldLines, newLines));
122+
newLines = RemoveExtraComma(_editor.GetLines(selection).Replace(oldLines, newLines),
123+
target.CountOfDeclarationsInStatement(), target.IndexOfVariableDeclarationInStatement());
123124
}
124125

125126
_editor.DeleteLines(selection);
@@ -130,34 +131,40 @@ private void RemoveField(Declaration target)
130131
}
131132
}
132133

133-
private string RemoveExtraComma(string str)
134+
private string RemoveExtraComma(string str, int numParams, int indexRemoved)
134135
{
135-
if (str.Count(c => c == ',') == 1)
136-
{
137-
return str.Remove(str.IndexOf(','), 1);
138-
}
139-
140-
var significantCharacterAfterComma = false;
141-
142-
for (var index = str.IndexOf("Dim", StringComparison.Ordinal) + 3; index < str.Length; index++)
143-
{
144-
if (!significantCharacterAfterComma && str[index] == ',')
145-
{
146-
return str.Remove(index, 1);
147-
}
148-
149-
if (!char.IsWhiteSpace(str[index]) && str[index] != '_' && str[index] != ',')
150-
{
151-
significantCharacterAfterComma = true;
152-
}
153-
154-
if (str[index] == ',')
155-
{
156-
significantCharacterAfterComma = false;
157-
}
158-
}
159-
160-
return str.Remove(str.LastIndexOf(','), 1);
136+
// Example use cases for this method (fields and variables):
137+
// Dim fizz as Boolean, dizz as Double
138+
// Private fizz as Boolean, dizz as Double
139+
// Public fizz as Boolean, _
140+
// dizz as Double
141+
// Private fizz as Boolean _
142+
// , dizz as Double _
143+
// , iizz as Integer
144+
145+
// Before this method is called, the parameter to be removed has
146+
// already been removed. This means 'str' will look like:
147+
// Dim fizz as Boolean,
148+
// Private , dizz as Double
149+
// Public fizz as Boolean, _
150+
//
151+
// Private _
152+
// , dizz as Double _
153+
// , iizz as Integer
154+
155+
// This method is responsible for removing the redundant comma
156+
// and returning a string similar to:
157+
// Dim fizz as Boolean
158+
// Private dizz as Double
159+
// Public fizz as Boolean _
160+
//
161+
// Private _
162+
// dizz as Double _
163+
// , iizz as Integer
164+
165+
var commaToRemove = numParams == indexRemoved ? indexRemoved - 1 : indexRemoved;
166+
167+
return str.Remove(str.NthIndexOf(',', commaToRemove), 1);
161168
}
162169

163170
private string GetPropertyText()

‎RetailCoder.VBE/Refactorings/IntroduceField/IntroduceFieldRefactoring.cs

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -94,41 +94,48 @@ private void RemoveVariable(Declaration target)
9494
if (multipleDeclarations)
9595
{
9696
selection = target.GetVariableStmtContextSelection();
97-
newLines = RemoveExtraComma(_editor.GetLines(selection).Replace(oldLines, newLines));
97+
newLines = RemoveExtraComma(_editor.GetLines(selection).Replace(oldLines, newLines),
98+
target.CountOfDeclarationsInStatement(), target.IndexOfVariableDeclarationInStatement());
9899
}
99100

100101
_editor.DeleteLines(selection);
101102
_editor.InsertLines(selection.StartLine, newLines);
102103
}
103104

104-
private string RemoveExtraComma(string str)
105+
private string RemoveExtraComma(string str, int numParams, int indexRemoved)
105106
{
106-
if (str.Count(c => c == ',') == 1)
107-
{
108-
return str.Remove(str.IndexOf(','), 1);
109-
}
110-
111-
var significantCharacterAfterComma = false;
112-
113-
for (var index = str.IndexOf("Dim", StringComparison.Ordinal) + 3; index < str.Length; index++)
114-
{
115-
if (!significantCharacterAfterComma && str[index] == ',')
116-
{
117-
return str.Remove(index, 1);
118-
}
119-
120-
if (!char.IsWhiteSpace(str[index]) && str[index] != '_' && str[index] != ',')
121-
{
122-
significantCharacterAfterComma = true;
123-
}
124-
125-
if (str[index] == ',')
126-
{
127-
significantCharacterAfterComma = false;
128-
}
129-
}
130-
131-
return str.Remove(str.LastIndexOf(','), 1);
107+
// Example use cases for this method (fields and variables):
108+
// Dim fizz as Boolean, dizz as Double
109+
// Private fizz as Boolean, dizz as Double
110+
// Public fizz as Boolean, _
111+
// dizz as Double
112+
// Private fizz as Boolean _
113+
// , dizz as Double _
114+
// , iizz as Integer
115+
116+
// Before this method is called, the parameter to be removed has
117+
// already been removed. This means 'str' will look like:
118+
// Dim fizz as Boolean,
119+
// Private , dizz as Double
120+
// Public fizz as Boolean, _
121+
//
122+
// Private _
123+
// , dizz as Double _
124+
// , iizz as Integer
125+
126+
// This method is responsible for removing the redundant comma
127+
// and returning a string similar to:
128+
// Dim fizz as Boolean
129+
// Private dizz as Double
130+
// Public fizz as Boolean _
131+
//
132+
// Private _
133+
// dizz as Double _
134+
// , iizz as Integer
135+
136+
var commaToRemove = numParams == indexRemoved ? indexRemoved - 1 : indexRemoved;
137+
138+
return str.Remove(str.NthIndexOf(',', commaToRemove), 1);
132139
}
133140

134141
private string GetFieldDefinition(Declaration target)

‎RetailCoder.VBE/Refactorings/IntroduceParameter/IntroduceParameterRefactoring.cs

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,8 @@ private void RemoveVariable(Declaration target)
229229
if (multipleDeclarations)
230230
{
231231
selection = target.GetVariableStmtContextSelection();
232-
newLines = RemoveExtraComma(_editor.GetLines(selection).Replace(oldLines, newLines));
232+
newLines = RemoveExtraComma(_editor.GetLines(selection).Replace(oldLines, newLines),
233+
target.CountOfDeclarationsInStatement(), target.IndexOfVariableDeclarationInStatement());
233234
}
234235

235236
_editor.DeleteLines(selection);
@@ -312,34 +313,40 @@ private Declaration GetInterfaceImplementation(Declaration target)
312313
return interfaceMember;
313314
}
314315

315-
private string RemoveExtraComma(string str)
316+
private string RemoveExtraComma(string str, int numParams, int indexRemoved)
316317
{
317-
if (str.Count(c => c == ',') == 1)
318-
{
319-
return str.Remove(str.IndexOf(','), 1);
320-
}
321-
322-
var significantCharacterAfterComma = false;
323-
324-
for (var index = str.IndexOf("Dim", StringComparison.Ordinal) + 3; index < str.Length; index++)
325-
{
326-
if (!significantCharacterAfterComma && str[index] == ',')
327-
{
328-
return str.Remove(index, 1);
329-
}
330-
331-
if (!char.IsWhiteSpace(str[index]) && str[index] != '_' && str[index] != ',')
332-
{
333-
significantCharacterAfterComma = true;
334-
}
335-
336-
if (str[index] == ',')
337-
{
338-
significantCharacterAfterComma = false;
339-
}
340-
}
341-
342-
return str.Remove(str.LastIndexOf(','), 1);
318+
// Example use cases for this method (fields and variables):
319+
// Dim fizz as Boolean, dizz as Double
320+
// Private fizz as Boolean, dizz as Double
321+
// Public fizz as Boolean, _
322+
// dizz as Double
323+
// Private fizz as Boolean _
324+
// , dizz as Double _
325+
// , iizz as Integer
326+
327+
// Before this method is called, the parameter to be removed has
328+
// already been removed. This means 'str' will look like:
329+
// Dim fizz as Boolean,
330+
// Private , dizz as Double
331+
// Public fizz as Boolean, _
332+
//
333+
// Private _
334+
// , dizz as Double _
335+
// , iizz as Integer
336+
337+
// This method is responsible for removing the redundant comma
338+
// and returning a string similar to:
339+
// Dim fizz as Boolean
340+
// Private dizz as Double
341+
// Public fizz as Boolean _
342+
//
343+
// Private _
344+
// dizz as Double _
345+
// , iizz as Integer
346+
347+
var commaToRemove = numParams == indexRemoved ? indexRemoved - 1 : indexRemoved;
348+
349+
return str.Remove(str.NthIndexOf(',', commaToRemove), 1);
343350
}
344351

345352
private string GetParameterDefinition(Declaration target)

‎RetailCoder.VBE/Refactorings/MoveCloserToUsage/MoveCloserToUsageRefactoring.cs

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ private void RemoveVariable(Declaration target)
126126
if (multipleDeclarations)
127127
{
128128
selection = target.GetVariableStmtContextSelection();
129-
newLines = RemoveExtraComma(_editor.GetLines(selection).Replace(oldLines, newLines));
129+
newLines = RemoveExtraComma(_editor.GetLines(selection).Replace(oldLines, newLines),
130+
target.CountOfDeclarationsInStatement(), target.IndexOfVariableDeclarationInStatement());
130131
}
131132

132133
_editor.DeleteLines(selection);
@@ -137,34 +138,40 @@ private void RemoveVariable(Declaration target)
137138
}
138139
}
139140

140-
private string RemoveExtraComma(string str)
141+
private string RemoveExtraComma(string str, int numParams, int indexRemoved)
141142
{
142-
if (str.Count(c => c == ',') == 1)
143-
{
144-
return str.Remove(str.IndexOf(','), 1);
145-
}
146-
147-
var significantCharacterAfterComma = false;
148-
149-
for (var index = str.IndexOf("Dim", StringComparison.Ordinal) + 3; index < str.Length; index++)
150-
{
151-
if (!significantCharacterAfterComma && str[index] == ',')
152-
{
153-
return str.Remove(index, 1);
154-
}
155-
156-
if (!char.IsWhiteSpace(str[index]) && str[index] != '_' && str[index] != ',')
157-
{
158-
significantCharacterAfterComma = true;
159-
}
160-
161-
if (str[index] == ',')
162-
{
163-
significantCharacterAfterComma = false;
164-
}
165-
}
166-
167-
return str.Remove(str.LastIndexOf(','), 1);
143+
// Example use cases for this method (fields and variables):
144+
// Dim fizz as Boolean, dizz as Double
145+
// Private fizz as Boolean, dizz as Double
146+
// Public fizz as Boolean, _
147+
// dizz as Double
148+
// Private fizz as Boolean _
149+
// , dizz as Double _
150+
// , iizz as Integer
151+
152+
// Before this method is called, the parameter to be removed has
153+
// already been removed. This means 'str' will look like:
154+
// Dim fizz as Boolean,
155+
// Private , dizz as Double
156+
// Public fizz as Boolean, _
157+
//
158+
// Private _
159+
// , dizz as Double _
160+
// , iizz as Integer
161+
162+
// This method is responsible for removing the redundant comma
163+
// and returning a string similar to:
164+
// Dim fizz as Boolean
165+
// Private dizz as Double
166+
// Public fizz as Boolean _
167+
//
168+
// Private _
169+
// dizz as Double _
170+
// , iizz as Integer
171+
172+
var commaToRemove = numParams == indexRemoved ? indexRemoved - 1 : indexRemoved;
173+
174+
return str.Remove(str.NthIndexOf(',', commaToRemove), 1);
168175
}
169176
}
170177
}

0 commit comments

Comments
 (0)