Skip to content

Commit 42a7bab

Browse files
BZngrBZngr
committed
Improved UDTMember validations
1 parent c8c477c commit 42a7bab

19 files changed

+271
-164
lines changed

‎Rubberduck.Core/UI/Refactorings/EncapsulateField/EncapsulateFieldView.xaml

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -58,26 +58,26 @@
5858
<Grid Margin="5,10,0,2"
5959
VerticalAlignment="Center">
6060
<StackPanel Orientation="Vertical">
61-
<CheckBox Name="WrapAsUDTCheckBox"
62-
Grid.Row="1"
63-
Grid.Column="0"
64-
Content="Wrap Fields in Private Type"
65-
IsChecked="{Binding EncapsulateAsUDT, Mode=TwoWay}"
66-
FontWeight="Bold"
67-
VerticalAlignment="Top"
68-
IsTabStop="False"
69-
Width="200"
70-
HorizontalAlignment="Left"
71-
Margin="20,5,0,5" />
72-
<ComboBox ItemsSource="{Binding UDTFields}"
73-
Margin="20,5,0,5"
74-
Width="300"
75-
HorizontalAlignment="Left"
76-
DisplayMemberPath="FieldDeclarationBlock"
77-
SelectedItem="{Binding SelectedObjectStateUDT, Mode=TwoWay}"
78-
Visibility="{Binding ShowStateUDTSelections, Converter={StaticResource BoolToVisibleVisibility}}">
79-
</ComboBox>
80-
</StackPanel>
61+
<CheckBox Name="WrapAsUDTCheckBox"
62+
Grid.Row="1"
63+
Grid.Column="0"
64+
Content="Wrap Fields in Private Type"
65+
IsChecked="{Binding ConvertFieldsToUDTMembers, Mode=TwoWay}"
66+
FontWeight="Bold"
67+
VerticalAlignment="Top"
68+
IsTabStop="False"
69+
Width="200"
70+
HorizontalAlignment="Left"
71+
Margin="20,5,0,5" />
72+
<ComboBox ItemsSource="{Binding UDTFields}"
73+
Margin="20,5,0,5"
74+
Width="300"
75+
HorizontalAlignment="Left"
76+
DisplayMemberPath="FieldDeclarationBlock"
77+
SelectedItem="{Binding SelectedObjectStateUDT, Mode=TwoWay}"
78+
Visibility="{Binding ShowStateUDTSelections, Converter={StaticResource BoolToVisibleVisibility}}">
79+
</ComboBox>
80+
</StackPanel>
8181
</Grid>
8282
<Grid Margin="5,0,0,10">
8383
<Grid MaxHeight="150">

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public bool ShowStateUDTSelections
132132
get
133133
{
134134
return Model.ObjectStateUDTCandidates.Count() > 1
135-
&& EncapsulateAsUDT;
135+
&& ConvertFieldsToUDTMembers;
136136
}
137137
}
138138

@@ -264,15 +264,15 @@ public bool IsReadOnly
264264
get => _masterDetail.DetailField?.IsReadOnly ?? SelectedField?.IsReadOnly ?? false;
265265
}
266266

267-
public bool EncapsulateAsUDT
267+
public bool ConvertFieldsToUDTMembers
268268
{
269-
get => Model.EncapsulateWithUDT;
269+
get => Model.ConvertFieldsToUDTMembers;
270270
set
271271
{
272-
Model.EncapsulateWithUDT = value;
273-
OnPropertyChanged(nameof(EncapsulationFields));
274-
OnPropertyChanged(nameof(ShowStateUDTSelections));
275-
OnPropertyChanged(nameof(PropertiesPreview));
272+
Model.ConvertFieldsToUDTMembers = value;
273+
ReloadListAndPreview();
274+
RefreshValidationResults();
275+
UpdateDetailForSelection();
276276
}
277277
}
278278

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,4 @@
1-
using Rubberduck.Parsing.Symbols;
2-
using Rubberduck.Refactorings.EncapsulateField;
3-
using System;
4-
using System.Collections.Generic;
5-
using System.Linq;
6-
using System.Text;
7-
using System.Threading.Tasks;
8-
using System.Windows;
1+
using Rubberduck.Refactorings.EncapsulateField;
92

103
namespace Rubberduck.UI.Refactorings.EncapsulateField
114
{

‎Rubberduck.Refactorings/Common/VBAIdentifierValidator.cs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,14 @@ bool IsTooShort()
6868
/// <summary>
6969
/// Predicate function determining if an identifier string conforms to MS-VBAL naming requirements
7070
/// </summary>
71-
public static bool IsValidIdentifier(string name, DeclarationType declarationType)
72-
=> !TryMatchInvalidIdentifierCriteria(name, declarationType, out _);
71+
public static bool IsValidIdentifier(string name, DeclarationType declarationType, bool isArrayDeclaration = false)
72+
=> !TryMatchInvalidIdentifierCriteria(name, declarationType, out _, isArrayDeclaration);
7373

7474
/// <summary>
7575
/// Evaluates an identifier string's conformance with MS-VBAL naming requirements.
7676
/// </summary>
7777
/// <returns>Message for first matching invalid identifier criteria. Or, an empty string if the identifier is valid</returns>
78-
public static bool TryMatchInvalidIdentifierCriteria(string name, DeclarationType declarationType, out string criteriaMatchMessage)
78+
public static bool TryMatchInvalidIdentifierCriteria(string name, DeclarationType declarationType, out string criteriaMatchMessage, bool isArrayDeclaration = false)
7979
{
8080
criteriaMatchMessage = string.Empty;
8181

@@ -103,11 +103,28 @@ public static bool TryMatchInvalidIdentifierCriteria(string name, DeclarationTyp
103103
}
104104

105105
//Is a reserved identifier
106-
if (!declarationType.HasFlag(DeclarationType.UserDefinedTypeMember)
107-
&& ReservedIdentifiers.Contains(name, StringComparer.InvariantCultureIgnoreCase))
106+
if (!declarationType.HasFlag(DeclarationType.UserDefinedTypeMember))
108107
{
109-
criteriaMatchMessage = string.Format(RubberduckUI.InvalidNameCriteria_IsReservedKeywordFormat, name);
110-
return true;
108+
if (ReservedIdentifiers.Contains(name, StringComparer.InvariantCultureIgnoreCase))
109+
{
110+
criteriaMatchMessage = string.Format(RubberduckUI.InvalidNameCriteria_IsReservedKeywordFormat, name);
111+
return true;
112+
}
113+
}
114+
else if (isArrayDeclaration) //is a DeclarationType.UserDefinedTypeMember
115+
{
116+
//DeclarationType.UserDefinedTypeMember can have reserved identifier keywords
117+
//...unless the declaration is an array. Adding the parentheses causes errors.
118+
119+
//Name is not a reserved identifier, but when used as a UDTMember array declaration
120+
//it collides with the 'Name' Statement (Renames a disk file, directory, or folder)
121+
var invalidUDTArrayIdentifiers = ReservedIdentifiers.Concat(new List<string>() { "Name" });
122+
123+
if (invalidUDTArrayIdentifiers.Contains(name, StringComparer.InvariantCultureIgnoreCase))
124+
{
125+
criteriaMatchMessage = string.Format(RubberduckUI.InvalidNameCriteria_IsReservedKeywordFormat, name);
126+
return true;
127+
}
111128
}
112129

113130
//"VBA" identifier not allowed for projects

‎Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldElementFactory.cs

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,68 +13,58 @@ namespace Rubberduck.Refactorings.EncapsulateField
1313
public class EncapsulateFieldElementFactory
1414
{
1515
private readonly IDeclarationFinderProvider _declarationFinderProvider;
16-
private readonly IEncapsulateFieldNamesValidator _validator;
16+
private readonly IEncapsulateFieldValidator _validator;
17+
private readonly IValidateEncapsulateFieldNames _namesValidator;
1718
private QualifiedModuleName _targetQMN;
1819

19-
public EncapsulateFieldElementFactory(IDeclarationFinderProvider declarationFinderProvider, QualifiedModuleName targetQMN, IEncapsulateFieldNamesValidator validator)
20+
public EncapsulateFieldElementFactory(IDeclarationFinderProvider declarationFinderProvider, QualifiedModuleName targetQMN, IEncapsulateFieldValidator validator )
2021
{
2122
_declarationFinderProvider = declarationFinderProvider;
2223
_validator = validator;
24+
_namesValidator = validator as IValidateEncapsulateFieldNames;
2325
_targetQMN = targetQMN;
2426
}
2527

2628
public IObjectStateUDT CreateStateUDTField()
2729
{
2830
var stateUDT = new ObjectStateUDT(_targetQMN) as IObjectStateUDT;
2931

30-
stateUDT = SetNonConflictIdentifier(stateUDT, c => { return _validator.IsConflictingStateUDTFieldIdentifier(stateUDT); }, (s) => { stateUDT.FieldIdentifier = s; }, () => stateUDT.FieldIdentifier, _validator);
32+
stateUDT = SetNonConflictIdentifier(stateUDT, c => { return _validator.IsConflictingStateUDTFieldIdentifier(stateUDT); }, (s) => { stateUDT.FieldIdentifier = s; }, () => stateUDT.FieldIdentifier, _namesValidator);
3133

32-
stateUDT = SetNonConflictIdentifier(stateUDT, c => { return _validator.IsConflictingStateUDTTypeIdentifier(stateUDT); }, (s) => { stateUDT.TypeIdentifier = s; }, () => stateUDT.TypeIdentifier, _validator);
34+
stateUDT = SetNonConflictIdentifier(stateUDT, c => { return _validator.IsConflictingStateUDTTypeIdentifier(stateUDT); }, (s) => { stateUDT.TypeIdentifier = s; }, () => stateUDT.TypeIdentifier, _namesValidator);
3335

3436
return stateUDT;
3537
}
3638

37-
public IEncapsulateFieldCandidate CreateEncapsulationCandidate(Declaration target)
39+
private IEncapsulateFieldCandidate CreateCandidate(Declaration target)
3840
{
39-
Debug.Assert(!target.DeclarationType.Equals(DeclarationType.UserDefinedTypeMember));
40-
41-
IEncapsulateFieldCandidate candidate = CreateCandidate(target);
42-
43-
if (candidate is IUserDefinedTypeCandidate udtVariable)
41+
if (target.IsUserDefinedTypeField())
4442
{
45-
(Declaration udtDeclaration, IEnumerable<Declaration> udtMembers) = GetUDTAndMembersForField(udtVariable);
43+
var udtField = new UserDefinedTypeCandidate(target, _namesValidator) as IUserDefinedTypeCandidate;
44+
45+
(Declaration udtDeclaration, IEnumerable<Declaration> udtMembers) = GetUDTAndMembersForField(udtField);
4646

47-
udtVariable.TypeDeclarationIsPrivate = udtDeclaration.HasPrivateAccessibility();
47+
udtField.TypeDeclarationIsPrivate = udtDeclaration.HasPrivateAccessibility();
4848

4949
foreach (var udtMemberDeclaration in udtMembers)
5050
{
51-
var candidateUDTMember = new UserDefinedTypeMemberCandidate(CreateCandidate(udtMemberDeclaration), udtVariable, _validator) as IUserDefinedTypeMemberCandidate;
51+
var candidateUDTMember = new UserDefinedTypeMemberCandidate(CreateCandidate(udtMemberDeclaration), udtField, _namesValidator) as IUserDefinedTypeMemberCandidate;
5252

53-
udtVariable.AddMember(candidateUDTMember);
53+
udtField.AddMember(candidateUDTMember);
5454
}
5555

5656
var udtVariablesOfSameType = _declarationFinderProvider.DeclarationFinder.UserDeclarations(DeclarationType.Variable)
5757
.Where(v => v.AsTypeDeclaration == udtDeclaration);
5858

59-
udtVariable.CanBeObjectStateUDT = udtVariable.TypeDeclarationIsPrivate && udtVariablesOfSameType.Count() == 1;
60-
}
61-
62-
_validator.RegisterFieldCandidate(candidate);
59+
udtField.CanBeObjectStateUDT = udtField.TypeDeclarationIsPrivate && udtVariablesOfSameType.Count() == 1;
6360

64-
return candidate;
65-
}
66-
67-
private IEncapsulateFieldCandidate CreateCandidate(Declaration target)
68-
{
69-
if (target.IsUserDefinedTypeField())
70-
{
71-
return new UserDefinedTypeCandidate(target, _validator);
61+
return udtField;
7262
}
7363
else if (target.IsArray)
7464
{
75-
return new ArrayCandidate(target, _validator);
65+
return new ArrayCandidate(target, _namesValidator);
7666
}
77-
return new EncapsulateFieldCandidate(target, _validator);
67+
return new EncapsulateFieldCandidate(target, _namesValidator);
7868
}
7969

8070
public IEnumerable<IEncapsulateFieldCandidate> CreateEncapsulationCandidates()
@@ -84,17 +74,22 @@ public IEnumerable<IEncapsulateFieldCandidate> CreateEncapsulationCandidates()
8474
.Where(v => v.IsMemberVariable() && !v.IsWithEvents);
8575

8676
var candidates = new List<IEncapsulateFieldCandidate>();
87-
foreach (var field in fieldDeclarations)
77+
foreach (var fieldDeclaration in fieldDeclarations)
8878
{
89-
var fieldEncapsulationCandidate = CreateEncapsulationCandidate(field);
79+
Debug.Assert(!fieldDeclaration.DeclarationType.Equals(DeclarationType.UserDefinedTypeMember));
80+
81+
var fieldEncapsulationCandidate = CreateCandidate(fieldDeclaration);
82+
83+
_validator.RegisterFieldCandidate(fieldEncapsulationCandidate);
84+
9085

9186
candidates.Add(fieldEncapsulationCandidate);
9287
}
9388

9489
return candidates;
9590
}
9691

97-
private IObjectStateUDT SetNonConflictIdentifier(IObjectStateUDT candidate, Predicate<IObjectStateUDT> conflictDetector, Action<string> setValue, Func<string> getIdentifier, IEncapsulateFieldNamesValidator validator)
92+
private IObjectStateUDT SetNonConflictIdentifier(IObjectStateUDT candidate, Predicate<IObjectStateUDT> conflictDetector, Action<string> setValue, Func<string> getIdentifier, IValidateEncapsulateFieldNames validator)
9893
{
9994
var isConflictingIdentifier = conflictDetector(candidate);
10095
for (var count = 1; count < 10 && isConflictingIdentifier; count++)

‎Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldModel.cs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ public class EncapsulateFieldModel : IRefactoringModel
1111
{
1212
private readonly Func<EncapsulateFieldModel, string> _previewDelegate;
1313
private QualifiedModuleName _targetQMN;
14-
private IEncapsulateFieldNamesValidator _validator;
14+
private IValidateEncapsulateFieldNames _validator;
1515
private IObjectStateUDT _newObjectStateUDT;
1616

1717
private IDictionary<Declaration, (Declaration, IEnumerable<Declaration>)> _udtFieldToUdtDeclarationMap = new Dictionary<Declaration, (Declaration, IEnumerable<Declaration>)>();
1818

19-
public EncapsulateFieldModel(Declaration target, IEnumerable<IEncapsulateFieldCandidate> candidates, IObjectStateUDT stateUDTField, Func<EncapsulateFieldModel, string> previewDelegate, IEncapsulateFieldNamesValidator validator)
19+
public EncapsulateFieldModel(Declaration target, IEnumerable<IEncapsulateFieldCandidate> candidates, IObjectStateUDT stateUDTField, Func<EncapsulateFieldModel, string> previewDelegate, IValidateEncapsulateFieldNames validator)
2020
{
2121
_previewDelegate = previewDelegate;
2222
_targetQMN = target.QualifiedModuleName;
@@ -48,7 +48,19 @@ public IEncapsulateFieldCandidate this[string encapsulatedFieldTargetID]
4848
public IEncapsulateFieldCandidate this[Declaration fieldDeclaration]
4949
=> EncapsulationCandidates.Where(c => c.Declaration == fieldDeclaration).Single();
5050

51-
public bool EncapsulateWithUDT { set; get; }
51+
private bool _convertFieldsToUDTMembers;
52+
public bool ConvertFieldsToUDTMembers
53+
{
54+
set
55+
{
56+
_convertFieldsToUDTMembers = value;
57+
foreach (var candidate in EncapsulationCandidates)
58+
{
59+
candidate.ConvertFieldToUDTMember = value;
60+
}
61+
}
62+
get => _convertFieldsToUDTMembers;
63+
}
5264

5365
public IObjectStateUDT StateUDTField { set; get; }
5466

‎Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,13 @@ protected override EncapsulateFieldModel InitializeModel(Declaration target)
8585

8686
_targetQMN = target.QualifiedModuleName;
8787

88-
var validator = new EncapsulateFieldNamesValidator(_declarationFinderProvider);
88+
var validator = new EncapsulateFieldValidator(_declarationFinderProvider) as IEncapsulateFieldValidator;
8989
_encapsulationCandidateFactory = new EncapsulateFieldElementFactory(_declarationFinderProvider, _targetQMN, validator);
9090

9191
var candidates = _encapsulationCandidateFactory.CreateEncapsulationCandidates();
9292
var selected = candidates.Single(c => c.Declaration == target);
9393
selected.EncapsulateFlag = true;
9494

95-
//var newObjectStateUDT = _encapsulationCandidateFactory.CreateStateUDTField();
96-
9795
if (!TryRetrieveExistingObjectStateUDT(target, candidates, out var objectStateUDT))
9896
{
9997
objectStateUDT = _encapsulationCandidateFactory.CreateStateUDTField();
@@ -193,7 +191,7 @@ private void ModifyReferences(EncapsulateFieldModel model, IEncapsulateFieldRewr
193191
{
194192
foreach (var field in model.SelectedFieldCandidates)
195193
{
196-
field.ReferenceQualifier = model.EncapsulateWithUDT
194+
field.ReferenceQualifier = model.ConvertFieldsToUDTMembers
197195
? model.StateUDTField.FieldIdentifier
198196
: null;
199197

@@ -210,7 +208,7 @@ private void ModifyReferences(EncapsulateFieldModel model, IEncapsulateFieldRewr
210208

211209
private void ModifyFields(EncapsulateFieldModel model, IEncapsulateFieldRewriteSession refactorRewriteSession)
212210
{
213-
if (model.EncapsulateWithUDT)
211+
if (model.ConvertFieldsToUDTMembers)
214212
{
215213
IModuleRewriter rewriter;
216214

@@ -300,7 +298,7 @@ private void InsertNewContent(EncapsulateFieldModel model, IEncapsulateFieldRewr
300298

301299
private void LoadNewDeclarationBlocks(EncapsulateFieldModel model)
302300
{
303-
if (model.EncapsulateWithUDT)
301+
if (model.ConvertFieldsToUDTMembers)
304302
{
305303
if (model.StateUDTField?.IsExistingDeclaration ?? false) { return; }
306304

0 commit comments

Comments
 (0)