Skip to content

Commit 8b265a8

Browse files
committed
Separate name validation and conflict detection
1 parent d5d8e7d commit 8b265a8

File tree

4 files changed

+67
-106
lines changed

4 files changed

+67
-106
lines changed

Rubberduck.Refactorings/EncapsulateField/Validations/ConvertFieldsToUDTMembersStrategyConflictFinder.cs

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,27 @@
99

1010
namespace Rubberduck.Refactorings.EncapsulateField
1111
{
12-
public class ConvertFieldsToUDTMembersStrategyConflictFinder : EncapsulateFieldConflictFinderBase
12+
public interface IConvertFieldsToUDTMembersStrategyConflictFinder : IEncapsulateFieldConflictFinder
1313
{
14+
IObjectStateUDT AssignNoConflictIdentifiers(IObjectStateUDT stateUDT, IDeclarationFinderProvider declarationFinderProvider);
15+
}
16+
17+
public class ConvertFieldsToUDTMembersStrategyConflictFinder : EncapsulateFieldConflictFinderBase, IConvertFieldsToUDTMembersStrategyConflictFinder
18+
{
19+
private static DeclarationType[] _udtTypeIdentifierNonConflictTypes = new DeclarationType[]
20+
{
21+
DeclarationType.Project,
22+
DeclarationType.Module,
23+
DeclarationType.Property,
24+
DeclarationType.Function,
25+
DeclarationType.Procedure,
26+
DeclarationType.Variable,
27+
DeclarationType.Constant,
28+
DeclarationType.UserDefinedTypeMember,
29+
DeclarationType.EnumerationMember,
30+
DeclarationType.Parameter
31+
};
32+
1433
private IEnumerable<IObjectStateUDT> _objectStateUDTs;
1534
public ConvertFieldsToUDTMembersStrategyConflictFinder(IDeclarationFinderProvider declarationFinderProvider, IEnumerable<IEncapsulateFieldCandidate> candidates, IEnumerable<IUserDefinedTypeMemberCandidate> udtCandidates, IEnumerable<IObjectStateUDT> objectStateUDTs)
1635
: base(declarationFinderProvider, candidates, udtCandidates)
@@ -33,6 +52,26 @@ public override bool TryValidateEncapsulationAttributes(IEncapsulateFieldCandida
3352
return !ConflictsWithExistingUDTMembers(objectStateUDT, field.BackingIdentifier);
3453
}
3554

55+
public IObjectStateUDT AssignNoConflictIdentifiers(IObjectStateUDT stateUDT, IDeclarationFinderProvider declarationFinderProvider)
56+
{
57+
var members = declarationFinderProvider.DeclarationFinder.Members(stateUDT.QualifiedModuleName);
58+
var guard = 0;
59+
while (guard++ < 10 && members.Any(m => m.IdentifierName.IsEquivalentVBAIdentifierTo(stateUDT.FieldIdentifier)))
60+
{
61+
stateUDT.FieldIdentifier = stateUDT.FieldIdentifier.IncrementEncapsulationIdentifier();
62+
}
63+
64+
members = declarationFinderProvider.DeclarationFinder.Members(stateUDT.QualifiedModuleName)
65+
.Where(m => !_udtTypeIdentifierNonConflictTypes.Any(nct => m.DeclarationType.HasFlag(nct)));
66+
67+
guard = 0;
68+
while (guard++ < 10 && members.Any(m => m.IdentifierName.IsEquivalentVBAIdentifierTo(stateUDT.TypeIdentifier)))
69+
{
70+
stateUDT.TypeIdentifier = stateUDT.TypeIdentifier.IncrementEncapsulationIdentifier();
71+
}
72+
return stateUDT;
73+
}
74+
3675
public override IEncapsulateFieldCandidate AssignNoConflictIdentifiers(IEncapsulateFieldCandidate candidate)
3776
{
3877
candidate = base.AssignNoConflictIdentifier(candidate, DeclarationType.Property);

Rubberduck.Refactorings/EncapsulateField/Validations/EncapsulateFieldConflictFinderBase.cs

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,9 @@
44
using Rubberduck.Refactorings.EncapsulateField.Extensions;
55
using Rubberduck.Resources;
66
using Rubberduck.VBEditor;
7-
using System;
87
using System.Collections.Generic;
98
using System.Diagnostics;
109
using System.Linq;
11-
using System.Text;
12-
using System.Threading.Tasks;
1310

1411
namespace Rubberduck.Refactorings.EncapsulateField
1512
{
@@ -21,23 +18,26 @@ public interface IEncapsulateFieldConflictFinder
2118
bool TryValidateEncapsulationAttributes(IEncapsulateFieldCandidate field, out string errorMessage);
2219
}
2320

24-
public abstract class EncapsulateFieldConflictFinderBase : IEncapsulateFieldConflictFinder
21+
public abstract class EncapsulateFieldConflictFinderBase
2522
{
2623
protected readonly IDeclarationFinderProvider _declarationFinderProvider;
2724
protected List<IEncapsulateFieldCandidate> _fieldCandidates { set; get; } = new List<IEncapsulateFieldCandidate>();
2825
protected List<IUserDefinedTypeMemberCandidate> _udtMemberCandidates { set; get; } = new List<IUserDefinedTypeMemberCandidate>();
2926

30-
public EncapsulateFieldConflictFinderBase(IDeclarationFinderProvider declarationFinderProvider, IEnumerable<IEncapsulateFieldCandidate> candidates, IEnumerable<IUserDefinedTypeMemberCandidate> udtCandidates)
27+
public EncapsulateFieldConflictFinderBase(IDeclarationFinderProvider declarationFinderProvider, IEnumerable<IEncapsulateFieldCandidate> candidates, IEnumerable<IUserDefinedTypeMemberCandidate> udtMemberCandidates)
3128
{
3229
_declarationFinderProvider = declarationFinderProvider;
3330
_fieldCandidates.AddRange(candidates);
34-
_udtMemberCandidates.AddRange(udtCandidates);
31+
_udtMemberCandidates.AddRange(udtMemberCandidates);
3532
}
3633

3734
public virtual bool TryValidateEncapsulationAttributes(IEncapsulateFieldCandidate field, out string errorMessage)
3835
{
3936
errorMessage = string.Empty;
40-
if (!field.EncapsulateFlag) { return true; }
37+
if (!field.EncapsulateFlag)
38+
{
39+
return true;
40+
}
4141

4242
if (!field.NameValidator.IsValidVBAIdentifier(field.PropertyIdentifier, out errorMessage))
4343
{
@@ -101,18 +101,22 @@ protected virtual bool InternalHasConflictingIdentifier(IEncapsulateFieldCandida
101101
errorMessage = string.Empty;
102102

103103
var potentialDeclarationIdentifierConflicts = new List<string>();
104-
potentialDeclarationIdentifierConflicts.AddRange(PotentialConflictIdentifiers(field, declarationType));
104+
var membersAndLocalReferencesMatches = PotentialConflictIdentifiers(field, declarationType);
105+
potentialDeclarationIdentifierConflicts.AddRange(membersAndLocalReferencesMatches);
105106

107+
var propertiesOfOtherFieldCandidates = _fieldCandidates.Where(fc => fc.TargetID != field.TargetID).Select(fc => fc.PropertyIdentifier);
106108
if (ignoreEncapsulationFlags)
107109
{
108-
potentialDeclarationIdentifierConflicts.AddRange(_fieldCandidates.Where(fc => fc.TargetID != field.TargetID).Select(fc => fc.PropertyIdentifier));
110+
potentialDeclarationIdentifierConflicts.AddRange(propertiesOfOtherFieldCandidates);
109111
}
110112
else
111113
{
112114
potentialDeclarationIdentifierConflicts.AddRange(FlaggedCandidates.Where(fc => fc.TargetID != field.TargetID).Select(fc => fc.PropertyIdentifier));
113115
}
114116

115-
potentialDeclarationIdentifierConflicts.AddRange(_udtMemberCandidates.Where(udtm => udtm.TargetID != field.TargetID && udtm.EncapsulateFlag).Select(udtm => udtm.PropertyIdentifier));
117+
var udtMemberNameConflictCandidates = _udtMemberCandidates.Where(udtm => udtm.TargetID != field.TargetID && udtm.EncapsulateFlag).Select(udtm => udtm.PropertyIdentifier);
118+
119+
potentialDeclarationIdentifierConflicts.AddRange(udtMemberNameConflictCandidates);
116120

117121
var identifierToCompare = IdentifierToCompare(field, declarationType);
118122

@@ -134,10 +138,12 @@ protected string IdentifierToCompare(IEncapsulateFieldCandidate field, Declarati
134138
protected bool HasConflictingIdentifierIgnoreEncapsulationFlag(IEncapsulateFieldCandidate field, DeclarationType declarationType, out string errorMessage)
135139
=> InternalHasConflictingIdentifier(field, declarationType, true, out errorMessage);
136140

137-
//The refactoring only inserts new code elements with the following Accessibilities:
138-
//Variables => Private
139-
//Properties => Public
140-
//UDTs => Private
141+
/// <summary>
142+
///The refactoring only inserts new code elements with the following Accessibilities:
143+
///Variables => Private
144+
///Properties => Public
145+
///UDTs => Private
146+
/// </summary>
141147
private bool IsAlwaysIgnoreNameConflictType(Declaration d, DeclarationType toEnapsulateDeclarationType)
142148
{
143149
//5.3.1.6 Each<subroutine-declaration> and<function-declaration> must have a procedure
@@ -179,8 +185,8 @@ private bool IsAlwaysIgnoreNameConflictType(Declaration d, DeclarationType toEna
179185
NeverCauseNameConflictTypes.Remove(DeclarationType.EnumerationMember);
180186
}
181187
return d.IsLocalVariable()
182-
|| d.IsLocalConstant()
183-
|| NeverCauseNameConflictTypes.Contains(d.DeclarationType);
188+
|| d.IsLocalConstant()
189+
|| NeverCauseNameConflictTypes.Contains(d.DeclarationType);
184190
}
185191

186192
private List<string> PotentialConflictIdentifiers(IEncapsulateFieldCandidate candidate, DeclarationType declarationType)
Lines changed: 4 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,5 @@
11
using Rubberduck.Parsing.Symbols;
2-
using Rubberduck.Parsing.VBA;
3-
using Rubberduck.Refactorings.Common;
4-
using Rubberduck.Refactorings.EncapsulateField.Extensions;
5-
using Rubberduck.Resources;
6-
using Rubberduck.VBEditor;
7-
using System;
82
using System.Collections.Generic;
9-
using System.Linq;
10-
using System.Text;
11-
using System.Threading.Tasks;
123

134
namespace Rubberduck.Refactorings.EncapsulateField
145
{
@@ -20,95 +11,20 @@ public enum NameValidators
2011
UserDefinedTypeMemberArray
2112
}
2213

23-
public interface IEncapsulateFieldValidationsProvider
14+
public class EncapsulateFieldValidationsProvider
2415
{
25-
IEncapsulateFieldConflictFinder ConflictDetector(EncapsulateFieldStrategy strategy, IDeclarationFinderProvider declarationFinderProvider);
26-
}
27-
28-
public class EncapsulateFieldValidationsProvider : IEncapsulateFieldValidationsProvider
29-
{
30-
private static Dictionary<NameValidators, IValidateVBAIdentifiers> _nameOnlyValidators = new Dictionary<NameValidators, IValidateVBAIdentifiers>()
16+
private static readonly Dictionary<NameValidators, IValidateVBAIdentifiers> _nameOnlyValidators = new Dictionary<NameValidators, IValidateVBAIdentifiers>()
3117
{
3218
[NameValidators.Default] = new IdentifierOnlyValidator(DeclarationType.Variable, false),
3319
[NameValidators.UserDefinedType] = new IdentifierOnlyValidator(DeclarationType.UserDefinedType, false),
3420
[NameValidators.UserDefinedTypeMember] = new IdentifierOnlyValidator(DeclarationType.UserDefinedTypeMember, false),
3521
[NameValidators.UserDefinedTypeMemberArray] = new IdentifierOnlyValidator(DeclarationType.UserDefinedTypeMember, true),
3622
};
3723

38-
private static DeclarationType[] _udtTypeIdentifierNonConflictTypes = new DeclarationType[]
39-
{
40-
DeclarationType.Project,
41-
DeclarationType.Module,
42-
DeclarationType.Property,
43-
DeclarationType.Function,
44-
DeclarationType.Procedure,
45-
DeclarationType.Variable,
46-
DeclarationType.Constant,
47-
DeclarationType.UserDefinedTypeMember,
48-
DeclarationType.EnumerationMember,
49-
DeclarationType.Parameter
50-
};
51-
52-
53-
private List<IEncapsulateFieldCandidate> _candidates;
54-
private List<IUserDefinedTypeMemberCandidate> _udtMemberCandidates;
55-
private List<IObjectStateUDT> _objectStateUDTs;
56-
57-
public EncapsulateFieldValidationsProvider(IEnumerable<IEncapsulateFieldCandidate> candidates, IEnumerable<IObjectStateUDT> objectStateUDTCandidates)
58-
{
59-
_udtMemberCandidates = new List<IUserDefinedTypeMemberCandidate>();
60-
_objectStateUDTs = objectStateUDTCandidates.ToList();
61-
_candidates = candidates.ToList();
62-
var udtCandidates = candidates.Where(c => c is IUserDefinedTypeCandidate).Cast<IUserDefinedTypeCandidate>();
63-
64-
foreach (var udtCandidate in candidates.Where(c => c is IUserDefinedTypeCandidate).Cast<IUserDefinedTypeCandidate>())
65-
{
66-
LoadUDTMemberCandidates(udtCandidate);
67-
}
68-
}
69-
70-
private void LoadUDTMemberCandidates(IUserDefinedTypeCandidate udtCandidate)
71-
{
72-
foreach (var member in udtCandidate.Members)
73-
{
74-
if (member.WrappedCandidate is IUserDefinedTypeCandidate udt)
75-
{
76-
LoadUDTMemberCandidates(udt);
77-
}
78-
_udtMemberCandidates.Add(member);
79-
}
80-
}
24+
public EncapsulateFieldValidationsProvider()
25+
{}
8126

8227
public static IValidateVBAIdentifiers NameOnlyValidator(NameValidators validatorType)
8328
=> _nameOnlyValidators[validatorType];
84-
85-
public static IObjectStateUDT AssignNoConflictIdentifiers(IObjectStateUDT stateUDT, IDeclarationFinderProvider declarationFinderProvider)
86-
{
87-
var members = declarationFinderProvider.DeclarationFinder.Members(stateUDT.QualifiedModuleName);
88-
var guard = 0;
89-
while (guard++ < 10 && members.Any(m => m.IdentifierName.IsEquivalentVBAIdentifierTo(stateUDT.FieldIdentifier)))
90-
{
91-
stateUDT.FieldIdentifier = stateUDT.FieldIdentifier.IncrementEncapsulationIdentifier();
92-
}
93-
94-
members = declarationFinderProvider.DeclarationFinder.Members(stateUDT.QualifiedModuleName)
95-
.Where(m => !_udtTypeIdentifierNonConflictTypes.Any(nct => m.DeclarationType.HasFlag(nct)));
96-
97-
guard = 0;
98-
while (guard++ < 10 && members.Any(m => m.IdentifierName.IsEquivalentVBAIdentifierTo(stateUDT.TypeIdentifier)))
99-
{
100-
stateUDT.TypeIdentifier = stateUDT.TypeIdentifier.IncrementEncapsulationIdentifier();
101-
}
102-
return stateUDT;
103-
}
104-
105-
public IEncapsulateFieldConflictFinder ConflictDetector(EncapsulateFieldStrategy strategy, IDeclarationFinderProvider declarationFinderProvider)
106-
{
107-
if (strategy == EncapsulateFieldStrategy.UseBackingFields)
108-
{
109-
return new UseBackingFieldsStrategyConflictFinder(declarationFinderProvider, _candidates, _udtMemberCandidates);
110-
}
111-
return new ConvertFieldsToUDTMembersStrategyConflictFinder(declarationFinderProvider, _candidates, _udtMemberCandidates, _objectStateUDTs);
112-
}
11329
}
11430
}

Rubberduck.Refactorings/EncapsulateField/Validations/UseBackingFieldsStrategyConflictFinder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
namespace Rubberduck.Refactorings.EncapsulateField
1010
{
11-
public class UseBackingFieldsStrategyConflictFinder : EncapsulateFieldConflictFinderBase
11+
public class UseBackingFieldsStrategyConflictFinder : EncapsulateFieldConflictFinderBase, IEncapsulateFieldConflictFinder
1212
{
1313
public UseBackingFieldsStrategyConflictFinder(IDeclarationFinderProvider declarationFinderProvider, IEnumerable<IEncapsulateFieldCandidate> candidates, IEnumerable<IUserDefinedTypeMemberCandidate> udtCandidates)
1414
: base(declarationFinderProvider, candidates, udtCandidates) { }

0 commit comments

Comments
 (0)