Skip to content

Commit 9d0fcb2

Browse files
BZngrBZngr
committed
Fixed preexisting UDTMembers name change bug
When converting fields to UDTMembers, UDTMember identifiers that pre-existed the refactoring where being updated to non-conflict names...but they should remain constant.
1 parent 917861f commit 9d0fcb2

File tree

9 files changed

+111
-29
lines changed

9 files changed

+111
-29
lines changed

‎Rubberduck.Core/UI/Refactorings/EncapsulateField/EncapsulateFieldViewModel.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ public bool ConvertFieldsToUDTMembers
273273
ReloadListAndPreview();
274274
RefreshValidationResults();
275275
UpdateDetailForSelection();
276+
OnPropertyChanged(nameof(ShowStateUDTSelections));
276277
}
277278
}
278279

‎Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldModel.cs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public EncapsulateFieldModel(Declaration target, IEnumerable<IEncapsulateFieldCa
2424
_newObjectStateUDT = stateUDTField;
2525

2626
EncapsulationCandidates = candidates.ToList();
27-
StateUDTField = stateUDTField;
27+
ConvertFieldsToUDTMembers = false;
2828
}
2929

3030
public List<IEncapsulateFieldCandidate> EncapsulationCandidates { set; get; } = new List<IEncapsulateFieldCandidate>();
@@ -54,15 +54,25 @@ public bool ConvertFieldsToUDTMembers
5454
set
5555
{
5656
_convertFieldsToUDTMembers = value;
57-
foreach (var candidate in EncapsulationCandidates)
58-
{
59-
candidate.ConvertFieldToUDTMember = value;
60-
}
57+
SetFieldsToUDTMemberFlags(value);
6158
}
6259
get => _convertFieldsToUDTMembers;
6360
}
6461

65-
public IObjectStateUDT StateUDTField { set; get; }
62+
private IObjectStateUDT _activeObjectStateUDT;
63+
public IObjectStateUDT StateUDTField
64+
{
65+
set => _activeObjectStateUDT = value;
66+
get => _activeObjectStateUDT ?? _newObjectStateUDT;
67+
}
68+
69+
private void SetFieldsToUDTMemberFlags(bool value)
70+
{
71+
foreach (var candidate in EncapsulationCandidates)
72+
{
73+
candidate.ConvertFieldToUDTMember = value;
74+
}
75+
}
6676

6777
public string PreviewRefactoring() => _previewDelegate(this);
6878

‎Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,25 +98,28 @@ protected override EncapsulateFieldModel InitializeModel(Declaration target)
9898
objectStateUDT.IsSelected = true;
9999
forceUseOfObjectStateUDT = true;
100100
}
101-
else
102-
{
103-
objectStateUDT = _encapsulationCandidateFactory.CreateStateUDTField();
104-
objectStateUDT.IsSelected = true;
105-
}
101+
102+
var defaultStateUDT = _encapsulationCandidateFactory.CreateStateUDTField();
103+
defaultStateUDT.IsSelected = objectStateUDT is null;
106104

107105
Model = new EncapsulateFieldModel(
108106
target,
109107
candidates,
110-
objectStateUDT,
108+
defaultStateUDT,
111109
PreviewRewrite,
112110
validator);
113111

112+
if (forceUseOfObjectStateUDT)
113+
{
114+
Model.ConvertFieldsToUDTMembers = true;
115+
Model.StateUDTField = objectStateUDT;
116+
}
117+
114118
_codeSectionStartIndex = _declarationFinderProvider.DeclarationFinder
115119
.Members(_targetQMN).Where(m => m.IsMember())
116120
.OrderBy(c => c.Selection)
117121
.FirstOrDefault()?.Context.Start.TokenIndex ?? null;
118122

119-
Model.ConvertFieldsToUDTMembers = forceUseOfObjectStateUDT;
120123
return Model;
121124
}
122125

‎Rubberduck.Refactorings/EncapsulateField/FieldCandidates/ObjectStateUDT.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public interface IObjectStateUDT : IEncapsulateFieldDeclaration
2222
Declaration AsTypeDeclaration { get; }
2323
bool IsSelected { set; get; }
2424
bool IsEncapsulateFieldCandidate(IEncapsulateFieldCandidate efc);
25+
IEnumerable<IUserDefinedTypeMemberCandidate> ExistingMembers { get; }
2526
}
2627

2728
//ObjectStateUDT can be an existing UDT (Private only) selected by the user, or a
@@ -66,6 +67,19 @@ private ObjectStateUDT(string typeIdentifier)
6667

6768
public bool IsSelected { set; get; }
6869

70+
public IEnumerable<IUserDefinedTypeMemberCandidate> ExistingMembers
71+
{
72+
get
73+
{
74+
if (IsExistingDeclaration)
75+
{
76+
return _wrappedUDT.Members;
77+
}
78+
return Enumerable.Empty<IUserDefinedTypeMemberCandidate>();
79+
}
80+
}
81+
82+
6983
private QualifiedModuleName _qmn;
7084
public QualifiedModuleName QualifiedModuleName
7185
{

‎Rubberduck.Refactorings/EncapsulateField/FieldCandidates/UserDefinedTypeCandidate.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,10 @@ public override bool EncapsulateFlag
8383
{
8484
if (TypeDeclarationIsPrivate)
8585
{
86-
foreach (var member in Members)
86+
foreach (var member in Members.Where(m => !m.ConvertFieldToUDTMember && m.IsExistingMember))
8787
{
8888
member.EncapsulateFlag = value;
89-
if (!_validator.HasConflictingIdentifier(member, DeclarationType.Property, out _))
89+
if (!member.EncapsulateFlag || !_validator.HasConflictingIdentifier(member, DeclarationType.Property, out _))
9090
{
9191
continue;
9292
}

‎Rubberduck.Refactorings/EncapsulateField/FieldCandidates/UserDefinedTypeMemberCandidate.cs

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ public interface IUserDefinedTypeMemberCandidate : IEncapsulateFieldCandidate
1616
Dictionary<IdentifierReference, (ParserRuleContext, string)> IdentifierReplacements { get; }
1717
IEnumerable<IdentifierReference> ParentContextReferences { get; }
1818
void LoadReferenceExpressions();
19+
bool IsExistingMember { get; }
1920
}
2021

2122
public class UserDefinedTypeMemberCandidate : IUserDefinedTypeMemberCandidate
@@ -167,11 +168,24 @@ public bool IsReadOnly
167168
set => _wrappedCandidate.IsReadOnly = value;
168169
get => _wrappedCandidate.IsReadOnly;
169170
}
171+
172+
private bool _encapsulateFlag;
170173
public bool EncapsulateFlag
171174
{
172-
set => _wrappedCandidate.EncapsulateFlag = value;
173-
get => _wrappedCandidate.EncapsulateFlag;
175+
set
176+
{
177+
_encapsulateFlag = value;
178+
if (!IsExistingMember)
179+
{
180+
_wrappedCandidate.EncapsulateFlag = value;
181+
}
182+
}
183+
184+
get => _encapsulateFlag; //=> _wrappedCandidate.EncapsulateFlag;
174185
}
186+
187+
public bool IsExistingMember => _wrappedCandidate.Declaration.ParentDeclaration.DeclarationType is DeclarationType.UserDefinedType;
188+
175189
public string FieldIdentifier
176190
{
177191
set => _wrappedCandidate.FieldIdentifier = value;
@@ -208,7 +222,12 @@ public string AsTypeName_Property
208222

209223
public bool ImplementSet => _wrappedCandidate.ImplementSet;
210224

211-
public bool ConvertFieldToUDTMember { set; get; }
225+
private bool _convertFieldToUDTMember;
226+
public bool ConvertFieldToUDTMember
227+
{
228+
set => _convertFieldToUDTMember = value;
229+
get => false;
230+
}
212231

213232
public IEnumerable<IPropertyGeneratorAttributes> PropertyAttributeSets => _wrappedCandidate.PropertyAttributeSets;
214233
public string AsUDTMemberDeclaration

‎RubberduckTests/Refactoring/EncapsulateField/EncapsulateFieldValidatorTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ public void UDTReservedMemberArrayIdentifier()
509509
var userInput = new UserInputDataObject()
510510
.UserSelectsField(fieldName);
511511

512-
userInput.EncapsulateAsUDT = true;
512+
userInput.ConvertFieldsToUDTMembers = true;
513513

514514
var presenterAction = Support.SetParameters(userInput);
515515

‎RubberduckTests/Refactoring/EncapsulateField/EncapsulateUsingStateUDTTests.cs

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,10 @@ Public foobar As Byte
144144
.UserSelectsField("foobar");
145145

146146
userInput.EncapsulateUsingUDTField("myBar");
147-
//userInput.ObjectStateUDTTargetID = "myBar";
148147

149148
var presenterAction = Support.SetParameters(userInput);
150149
var actualCode = Support.RefactoredCode(inputCode.ToCodeString(), presenterAction);
151150
StringAssert.DoesNotContain($"Private this As {Support.StateUDTDefaultType}", actualCode);
152-
//StringAssert.Contains($"Private Type {Support.StateUDTDefaultType}", actualCode);
153151
StringAssert.Contains("Foo As Long", actualCode);
154152
StringAssert.DoesNotContain("Public foo As Long", actualCode);
155153
StringAssert.Contains("Bar As String", actualCode);
@@ -162,6 +160,43 @@ Public foobar As Byte
162160
StringAssert.Contains("Second As Long", actualCode);
163161
}
164162

163+
[Test]
164+
[Category("Refactorings")]
165+
[Category("Encapsulate Field")]
166+
public void DoesNotChangeExistingUDTMembers()
167+
{
168+
string inputCode =
169+
$@"
170+
Private Type T{MockVbeBuilder.TestModuleName}
171+
Name As String
172+
End Type
173+
174+
Private this As T{MockVbeBuilder.TestModuleName}
175+
176+
Public foo As Long
177+
Public bar As String
178+
Public foo|bar As Byte
179+
180+
Public Property Let Name(value As String)
181+
this.Name = value
182+
End Property
183+
184+
Public Property Get Name() As String
185+
Name = this.Name
186+
End Property
187+
";
188+
189+
var userInput = new UserInputDataObject()
190+
.UserSelectsField("foobar");
191+
192+
userInput.EncapsulateUsingUDTField($"T{MockVbeBuilder.TestModuleName}");
193+
194+
var presenterAction = Support.SetParameters(userInput);
195+
var actualCode = Support.RefactoredCode(inputCode.ToCodeString(), presenterAction);
196+
StringAssert.DoesNotContain($"Name_1 As String", actualCode);
197+
StringAssert.DoesNotContain($"ThisName As String", actualCode);
198+
}
199+
165200
[Test]
166201
[Category("Refactorings")]
167202
[Category("Encapsulate Field")]

‎RubberduckTests/Refactoring/EncapsulateField/TestSupport.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ public Func<EncapsulateFieldModel, EncapsulateFieldModel> SetParameters(UserInpu
6464
{
6565
return model =>
6666
{
67-
model.ConvertFieldsToUDTMembers = userInput.EncapsulateAsUDT;
68-
if (userInput.EncapsulateAsUDT)
67+
model.ConvertFieldsToUDTMembers = userInput.ConvertFieldsToUDTMembers;
68+
if (userInput.ConvertFieldsToUDTMembers)
6969
{
7070
var stateUDT = model.EncapsulationCandidates.Where(sfc => sfc is IUserDefinedTypeCandidate udt && udt.TargetID == userInput.ObjectStateUDTTargetID)
7171
.Select(sfc => sfc as IUserDefinedTypeCandidate).SingleOrDefault();
@@ -79,7 +79,7 @@ public Func<EncapsulateFieldModel, EncapsulateFieldModel> SetParameters(UserInpu
7979

8080
foreach (var testModifiedAttribute in userInput.EncapsulateFieldAttributes)
8181
{
82-
var attrsInitializedByTheRefactoring = model[testModifiedAttribute.TargetFieldName]; //.EncapsulationAttributes;
82+
var attrsInitializedByTheRefactoring = model[testModifiedAttribute.TargetFieldName];
8383

8484
attrsInitializedByTheRefactoring.PropertyName = testModifiedAttribute.PropertyName;
8585
attrsInitializedByTheRefactoring.EncapsulateFlag = testModifiedAttribute.EncapsulateFlag;
@@ -89,7 +89,7 @@ public Func<EncapsulateFieldModel, EncapsulateFieldModel> SetParameters(UserInpu
8989
};
9090
}
9191

92-
public Func<EncapsulateFieldModel, EncapsulateFieldModel> SetParameters(string originalField, TestEncapsulationAttributes attrs, bool asUDT = false)
92+
public Func<EncapsulateFieldModel, EncapsulateFieldModel> SetParameters(string originalField, TestEncapsulationAttributes attrs, bool convertFieldsToUDTMembers = false)
9393
{
9494
return model =>
9595
{
@@ -98,7 +98,7 @@ public Func<EncapsulateFieldModel, EncapsulateFieldModel> SetParameters(string o
9898
encapsulatedField.IsReadOnly = attrs.IsReadOnly;
9999
encapsulatedField.EncapsulateFlag = attrs.EncapsulateFlag;
100100

101-
model.ConvertFieldsToUDTMembers = asUDT;
101+
model.ConvertFieldsToUDTMembers = convertFieldsToUDTMembers;
102102
return model;
103103
};
104104
}
@@ -139,7 +139,7 @@ public IEncapsulateFieldCandidate RetrieveEncapsulateFieldCandidate(IVBE vbe, st
139139
}
140140
}
141141

142-
public EncapsulateFieldModel RetrieveUserModifiedModelPriorToRefactoring(IVBE vbe, string declarationName, DeclarationType declarationType, Func<EncapsulateFieldModel, EncapsulateFieldModel> presenterAdjustment) //, params string[] fieldIdentifiers)
142+
public EncapsulateFieldModel RetrieveUserModifiedModelPriorToRefactoring(IVBE vbe, string declarationName, DeclarationType declarationType, Func<EncapsulateFieldModel, EncapsulateFieldModel> presenterAdjustment)
143143
{
144144
var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe);
145145
using (state)
@@ -217,12 +217,12 @@ public UserInputDataObject AddUserInputSet(string fieldName, string propertyName
217217
return this;
218218
}
219219

220-
public bool EncapsulateAsUDT { set; get; }
220+
public bool ConvertFieldsToUDTMembers { set; get; }
221221

222222
public void EncapsulateUsingUDTField(string targetID = null)
223223
{
224224
ObjectStateUDTTargetID = targetID;
225-
EncapsulateAsUDT = true;
225+
ConvertFieldsToUDTMembers = true;
226226
}
227227

228228
public string ObjectStateUDTTargetID { set; get; }

0 commit comments

Comments
 (0)