Skip to content

Commit 29d4680

Browse files
BZngrBZngr
committed
Rework and reduce IEncapsulateFieldCandidate
1 parent f3226ef commit 29d4680

File tree

9 files changed

+185
-317
lines changed

9 files changed

+185
-317
lines changed

‎Rubberduck.Refactorings/EncapsulateField/ConflictDetection/EncapsulateFieldConflictFinder.cs

Lines changed: 73 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
using Rubberduck.Parsing.Symbols;
1+
using Rubberduck.Parsing;
2+
using Rubberduck.Parsing.Grammar;
3+
using Rubberduck.Parsing.Symbols;
24
using Rubberduck.Parsing.VBA;
35
using Rubberduck.Refactorings.Common;
46
using Rubberduck.Refactorings.EncapsulateField.Extensions;
@@ -16,6 +18,7 @@ public interface IEncapsulateFieldConflictFinder
1618
void AssignNoConflictIdentifiers(IEncapsulateFieldCandidate candidate);
1719
void AssignNoConflictIdentifiers(IObjectStateUDT stateUDT);
1820
void AssignNoConflictIdentifiers(IEnumerable<IEncapsulateFieldCandidate> candidates);
21+
void AssignNoConflictBackingFieldIdentifier(IEncapsulateFieldCandidate candidate);
1922
}
2023

2124
public class EncapsulateFieldConflictFinder : IEncapsulateFieldConflictFinder
@@ -47,7 +50,8 @@ public EncapsulateFieldConflictFinder(IDeclarationFinderProvider declarationFind
4750
_fieldCandidates = candidates.ToList();
4851

4952
_udtMemberCandidates = new List<IUserDefinedTypeMemberCandidate>();
50-
_fieldCandidates.ForEach(c => LoadUDTMembers(c));
53+
54+
_fieldCandidates.ForEach(c => LoadUDTMemberCandidates(c, _udtMemberCandidates));
5155

5256
_allCandidates = _fieldCandidates.Concat(_udtMemberCandidates).ToList();
5357

@@ -75,13 +79,23 @@ public EncapsulateFieldConflictFinder(IDeclarationFinderProvider declarationFind
7579

7680
var errorMessage = string.Empty;
7781

78-
var hasInvalidIdentifierOrHasConflicts =
79-
VBAIdentifierValidator.TryMatchInvalidIdentifierCriteria(field.PropertyIdentifier, declarationType, out errorMessage, field.Declaration.IsArray)
80-
|| IsConflictingIdentifier(field, field.PropertyIdentifier, out errorMessage)
81-
|| IsConflictingIdentifier(field, field.BackingIdentifier, out errorMessage)
82-
|| field is IEncapsulateFieldAsUDTMemberCandidate && ConflictsWithExistingUDTMembers(SelectedObjectStateUDT(), field.BackingIdentifier, out errorMessage);
82+
if (field.Declaration.IsArray)
83+
{
84+
if (field.Declaration.References.Any(rf => rf.QualifiedModuleName != field.QualifiedModuleName
85+
&& rf.Context.TryGetAncestor<VBAParser.RedimVariableDeclarationContext>(out _)))
86+
{
87+
errorMessage = string.Format(RubberduckUI.EncapsulateField_ArrayHasExternalRedimFormat, field.IdentifierName);
88+
}
89+
return (!string.IsNullOrEmpty(errorMessage), errorMessage);
90+
}
8391

84-
return (string.IsNullOrEmpty(errorMessage), errorMessage);
92+
var hasConflictFreeValidIdentifiers =
93+
!VBAIdentifierValidator.TryMatchInvalidIdentifierCriteria(field.PropertyIdentifier, declarationType, out errorMessage, field.Declaration.IsArray)
94+
&& !IsConflictingIdentifier(field, field.PropertyIdentifier, out errorMessage)
95+
&& !IsConflictingIdentifier(field, field.BackingIdentifier, out errorMessage)
96+
&& !(field is IEncapsulateFieldAsUDTMemberCandidate && ConflictsWithExistingUDTMembers(SelectedObjectStateUDT(), field.BackingIdentifier, out errorMessage));
97+
98+
return (hasConflictFreeValidIdentifiers, errorMessage);
8599
}
86100

87101
public bool IsConflictingIdentifier(IEncapsulateFieldCandidate field, string identifierToCompare, out string errorMessage)
@@ -98,39 +112,39 @@ public void AssignNoConflictIdentifiers(IEnumerable<IEncapsulateFieldCandidate>
98112
{
99113
foreach (var candidate in candidates.Where(c => c.EncapsulateFlag))
100114
{
101-
ResolveFieldConflicts(candidate);
115+
ResolveIdentifierConflicts(candidate);
102116
}
103117
}
104118

105-
private void ResolveFieldConflicts(IEncapsulateFieldCandidate candidate)
119+
private void ResolveIdentifierConflicts(IEncapsulateFieldCandidate candidate)
106120
{
107121
AssignNoConflictIdentifiers(candidate);
108122
if (candidate is IUserDefinedTypeCandidate udtCandidate)
109123
{
110-
ResolveUDTMemberConflicts(udtCandidate.Members);
124+
ResolveUDTMemberIdentifierConflicts(udtCandidate.Members);
111125
}
112126
}
113127

114-
private void ResolveUDTMemberConflicts(IEnumerable<IUserDefinedTypeMemberCandidate> members)
128+
private void ResolveUDTMemberIdentifierConflicts(IEnumerable<IUserDefinedTypeMemberCandidate> members)
115129
{
116130
foreach (var member in members)
117131
{
118132
AssignNoConflictIdentifiers(member);
119133
if (member.WrappedCandidate is IUserDefinedTypeCandidate childUDT
120134
&& childUDT.Declaration.AsTypeDeclaration.HasPrivateAccessibility())
121135
{
122-
ResolveFieldConflicts(childUDT);
136+
ResolveIdentifierConflicts(childUDT);
123137
}
124138
}
125139
}
126140

127141
public void AssignNoConflictIdentifiers(IEncapsulateFieldCandidate candidate)
128142
{
129-
if (candidate is IEncapsulateFieldAsUDTMemberCandidate)
143+
if (candidate is IEncapsulateFieldAsUDTMemberCandidate udtMember)
130144
{
131145
AssignIdentifier(
132-
() => ConflictsWithExistingUDTMembers(SelectedObjectStateUDT(), candidate.PropertyIdentifier, out _),
133-
() => IncrementPropertyIdentifier(candidate));
146+
() => ConflictsWithExistingUDTMembers(SelectedObjectStateUDT(), udtMember.UserDefinedTypeMemberIdentifier, out _),
147+
() => udtMember.UserDefinedTypeMemberIdentifier = udtMember.UserDefinedTypeMemberIdentifier.IncrementEncapsulationIdentifier());
134148
return;
135149
}
136150

@@ -141,15 +155,16 @@ public void AssignNoConflictIdentifiers(IEncapsulateFieldCandidate candidate)
141155
public void AssignNoConflictIdentifiers(IObjectStateUDT stateUDT)
142156
{
143157
AssignIdentifier(
144-
() => HasConflictingFieldIdentifier(stateUDT, stateUDT.FieldIdentifier),
145-
() => stateUDT.FieldIdentifier = stateUDT.FieldIdentifier.IncrementEncapsulationIdentifier());
158+
() => _existingUserUDTsAndEnums.Any(m => m.IdentifierName.IsEquivalentVBAIdentifierTo(stateUDT.TypeIdentifier)),
159+
() => stateUDT.TypeIdentifier = stateUDT.TypeIdentifier.IncrementEncapsulationIdentifier());
146160

147161
AssignIdentifier(
148-
() => _existingUserUDTsAndEnums.Any(m => m.IdentifierName.IsEquivalentVBAIdentifierTo(stateUDT.TypeIdentifier)),
149-
() => stateUDT.TypeIdentifier = stateUDT.TypeIdentifier.IncrementEncapsulationIdentifier());
162+
() => HasConflictingFieldIdentifier(stateUDT, stateUDT.FieldIdentifier),
163+
() => stateUDT.FieldIdentifier = stateUDT.FieldIdentifier.IncrementEncapsulationIdentifier());
150164
}
151165

152-
private IObjectStateUDT SelectedObjectStateUDT() => _objectStateUDTs.SingleOrDefault(os => os.IsSelected);
166+
private IObjectStateUDT SelectedObjectStateUDT()
167+
=> _objectStateUDTs.SingleOrDefault(os => os.IsSelected);
153168

154169
private static bool ConflictsWithExistingUDTMembers(IObjectStateUDT objectStateUDT, string identifier, out string errorMessage)
155170
{
@@ -161,26 +176,20 @@ private static bool ConflictsWithExistingUDTMembers(IObjectStateUDT objectStateU
161176
return !string.IsNullOrEmpty(errorMessage);
162177
}
163178

164-
private void IncrementPropertyIdentifier(IEncapsulateFieldCandidate candidate)
165-
=> candidate.PropertyIdentifier = candidate.PropertyIdentifier.IncrementEncapsulationIdentifier();
166-
167179
private void AssignNoConflictPropertyIdentifier(IEncapsulateFieldCandidate candidate)
168180
{
169181
AssignIdentifier(
170182
() => IsConflictingIdentifier(candidate, candidate.PropertyIdentifier, out _),
171-
() => IncrementPropertyIdentifier(candidate));
183+
() => candidate.PropertyIdentifier = candidate.PropertyIdentifier.IncrementEncapsulationIdentifier());
172184
}
173185

174-
private void AssignNoConflictBackingFieldIdentifier(IEncapsulateFieldCandidate candidate)
186+
public void AssignNoConflictBackingFieldIdentifier(IEncapsulateFieldCandidate candidate)
175187
{
176-
//Private UserDefinedTypes are never used directly as a backing field - so never change their identifier.
177-
//The backing fields for an encapsulated Private UDT are its members.
178-
if (!(candidate is UserDefinedTypeMemberCandidate
179-
|| candidate is IUserDefinedTypeCandidate udtCandidate && udtCandidate.TypeDeclarationIsPrivate))
188+
if (candidate.BackingIdentifierMutator != null)
180189
{
181190
AssignIdentifier(
182191
() => IsConflictingIdentifier(candidate, candidate.BackingIdentifier, out _),
183-
() => candidate.BackingIdentifier = candidate.BackingIdentifier.IncrementEncapsulationIdentifier());
192+
() => candidate.BackingIdentifierMutator(candidate.BackingIdentifier.IncrementEncapsulationIdentifier()));
184193
}
185194
}
186195

@@ -200,13 +209,24 @@ private static void AssignIdentifier(Func<bool> hasConflict, Action incrementIde
200209

201210
private bool HasConflictIdentifiers(IEncapsulateFieldCandidate candidate, string identifierToCompare)
202211
{
203-
if (_allCandidates.Where(c => c.TargetID != candidate.TargetID
212+
return HasInternalPropertyAndBackingFieldConflict(candidate)
213+
|| HasConflictsWithOtherEncapsulationPropertyIdentifiers(candidate, identifierToCompare)
214+
|| HasConflictsWithUnmodifiedPropertyAndFieldIdentifiers(candidate, identifierToCompare)
215+
|| HasConflictWithLocalDeclarationIdentifiers(candidate, identifierToCompare);
216+
}
217+
218+
private bool HasInternalPropertyAndBackingFieldConflict(IEncapsulateFieldCandidate candidate)
219+
=> candidate.BackingIdentifierMutator != null
220+
&& candidate.EncapsulateFlag
221+
&& candidate.PropertyIdentifier.IsEquivalentVBAIdentifierTo(candidate.BackingIdentifier);
222+
223+
private bool HasConflictsWithOtherEncapsulationPropertyIdentifiers(IEncapsulateFieldCandidate candidate, string identifierToCompare)
224+
=> _allCandidates.Where(c => c.TargetID != candidate.TargetID
204225
&& c.EncapsulateFlag
205-
&& c.PropertyIdentifier.IsEquivalentVBAIdentifierTo(identifierToCompare)).Any())
206-
{
207-
return true;
208-
}
226+
&& c.PropertyIdentifier.IsEquivalentVBAIdentifierTo(identifierToCompare)).Any();
209227

228+
private bool HasConflictsWithUnmodifiedPropertyAndFieldIdentifiers(IEncapsulateFieldCandidate candidate, string identifierToCompare)
229+
{
210230
var membersToEvaluate = _members.Where(d => d != candidate.Declaration);
211231

212232
if (candidate is IEncapsulateFieldAsUDTMemberCandidate)
@@ -219,13 +239,22 @@ private bool HasConflictIdentifiers(IEncapsulateFieldCandidate candidate, string
219239
var nameConflictCandidates = membersToEvaluate.Where(member => !(member.IsLocalVariable() || member.IsLocalConstant()
220240
|| _declarationTypesThatNeverConflictWithFieldAndPropertyIdentifiers.Contains(member.DeclarationType)));
221241

222-
if (nameConflictCandidates.Any(m => m.IdentifierName.IsEquivalentVBAIdentifierTo(identifierToCompare)))
242+
return nameConflictCandidates.Any(m => m.IdentifierName.IsEquivalentVBAIdentifierTo(identifierToCompare));
243+
}
244+
245+
private bool HasConflictWithLocalDeclarationIdentifiers(IEncapsulateFieldCandidate candidate, string identifierToCompare)
246+
{
247+
var membersToEvaluate = _members.Where(d => d != candidate.Declaration);
248+
249+
if (candidate is IEncapsulateFieldAsUDTMemberCandidate)
223250
{
224-
return true;
251+
membersToEvaluate = membersToEvaluate.Except(
252+
_fieldCandidates.Where(fc => fc.EncapsulateFlag && fc.Declaration.DeclarationType.HasFlag(DeclarationType.Variable))
253+
.Select(f => f.Declaration));
225254
}
226255

227-
//Only check IdentifierReferences in the declaring module because IdentifierReferences in
228-
//other modules will be module-qualified.
256+
//Only check IdentifierReferences in the declaring module because encapsulated field
257+
//references in other modules will be module-qualified.
229258
var candidateLocalReferences = candidate.Declaration.References.Where(rf => rf.QualifiedModuleName == candidate.QualifiedModuleName);
230259

231260
var localDeclarationConflictCandidates = membersToEvaluate.Where(localDec => candidateLocalReferences
@@ -251,15 +280,15 @@ private bool HasConflictingFieldIdentifier(IObjectStateUDT candidate, string ide
251280
.Where(fc => fc.EncapsulateFlag && fc.Declaration.DeclarationType.HasFlag(DeclarationType.Variable))
252281
.Select(fc => fc.Declaration);
253282

254-
var nameConflictCandidates =
283+
var nameConflictCandidates =
255284
_members.Except(fieldsToRemoveFromConflictCandidates)
256285
.Where(member => !(member.IsLocalVariable() || member.IsLocalConstant()
257286
|| _declarationTypesThatNeverConflictWithFieldAndPropertyIdentifiers.Contains(member.DeclarationType)));
258287

259288
return nameConflictCandidates.Any(m => m.IdentifierName.IsEquivalentVBAIdentifierTo(identifierToCompare));
260289
}
261290

262-
private void LoadUDTMembers(IEncapsulateFieldCandidate candidate)
291+
private void LoadUDTMemberCandidates(IEncapsulateFieldCandidate candidate, List<IUserDefinedTypeMemberCandidate> udtMemberCandidates)
263292
{
264293
if (!(candidate is IUserDefinedTypeCandidate udtCandidate))
265294
{
@@ -268,13 +297,13 @@ private void LoadUDTMembers(IEncapsulateFieldCandidate candidate)
268297

269298
foreach (var member in udtCandidate.Members)
270299
{
271-
_udtMemberCandidates.Add(member);
300+
udtMemberCandidates.Add(member);
272301

273302
if (member.WrappedCandidate is IUserDefinedTypeCandidate childUDT
274303
&& childUDT.Declaration.AsTypeDeclaration.HasPrivateAccessibility())
275304
{
276305
//recursive till a non-UserDefinedType member is found
277-
LoadUDTMembers(childUDT);
306+
LoadUDTMemberCandidates(childUDT, udtMemberCandidates);
278307
}
279308
}
280309
}

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

Lines changed: 2 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,15 @@
1-
using Rubberduck.Parsing;
2-
using Rubberduck.Parsing.Grammar;
1+
using Rubberduck.Parsing.Grammar;
32
using Rubberduck.Parsing.Symbols;
4-
using Rubberduck.Resources;
5-
using System.Linq;
63

74
namespace Rubberduck.Refactorings.EncapsulateField
85
{
9-
public interface IArrayCandidate : IEncapsulateFieldCandidate
6+
public class ArrayCandidate : EncapsulateFieldCandidate
107
{
11-
string UDTMemberDeclaration { get;}
12-
bool HasExternalRedimOperation(out string errorMessage);
13-
}
14-
15-
public class ArrayCandidate : EncapsulateFieldCandidate, IArrayCandidate
16-
{
17-
private string _subscripts;
188
public ArrayCandidate(Declaration declaration)
199
:base(declaration)
2010
{
21-
ImplementLet = false;
22-
ImplementSet = false;
2311
PropertyAsTypeName = Tokens.Variant;
24-
CanBeReadWrite = false;
2512
IsReadOnly = true;
26-
27-
_subscripts = string.Empty;
28-
if (declaration.Context.TryGetChildContext<VBAParser.SubscriptsContext>(out var ctxt))
29-
{
30-
_subscripts = ctxt.GetText();
31-
}
32-
}
33-
34-
public override bool TryValidateEncapsulationAttributes(out string errorMessage)
35-
{
36-
errorMessage = string.Empty;
37-
if (!EncapsulateFlag)
38-
{
39-
return true;
40-
}
41-
42-
if (HasExternalRedimOperation(out errorMessage))
43-
{
44-
return false;
45-
}
46-
47-
(bool IsValid, string ErrorMsg) = ConflictFinder?.ValidateEncapsulationAttributes(this) ?? (true, string.Empty);
48-
errorMessage = ErrorMsg;
49-
return IsValid;
50-
}
51-
52-
public string UDTMemberDeclaration
53-
=> $"{PropertyIdentifier}({_subscripts}) {Tokens.As} {Declaration.AsTypeName}";
54-
55-
public bool HasExternalRedimOperation(out string errorMessage)
56-
{
57-
errorMessage = string.Empty;
58-
if (Declaration.References.Any(rf => rf.QualifiedModuleName != QualifiedModuleName
59-
&& rf.Context.TryGetAncestor<VBAParser.RedimVariableDeclarationContext>(out _)))
60-
{
61-
errorMessage = string.Format(RubberduckUI.EncapsulateField_ArrayHasExternalRedimFormat, IdentifierName);
62-
return true;
63-
}
64-
return false;
6513
}
6614
}
6715
}

0 commit comments

Comments
 (0)