Found failing scenario...added test and resolution

BZngr · BZngr · commit 830a34912d27 · 2020-01-14T15:58:43.000-08:00
diff --git a/Rubberduck.Core/UI/Refactorings/EncapsulateField/EncapsulateFieldViewModel.cs b/Rubberduck.Core/UI/Refactorings/EncapsulateField/EncapsulateFieldViewModel.cs
@@ -92,7 +92,7 @@ public IObjectStateUDT SelectedObjectStateUDT
             set
             {
                 _selectedObjectStateUDT = value;
-                Model.StateUDTField = _selectedObjectStateUDT;
+                Model.ObjectStateUDTField = _selectedObjectStateUDT;
                 SetObjectStateUDT();
             }
         }
diff --git a/Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldModel.cs b/Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldModel.cs
@@ -1,5 +1,6 @@
 ﻿using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.Parsing.VBA;
@@ -41,7 +42,7 @@ public EncapsulateFieldModel(
             _objStateCandidates.Add(_newObjectStateUDT);
 
             EncapsulateFieldStrategy = EncapsulateFieldStrategy.UseBackingFields;
-            _activeObjectStateUDT = StateUDTField;
+            _activeObjectStateUDT = ObjectStateUDTField;
         }
 
         public QualifiedModuleName QualifiedModuleName => _targetQMN;
@@ -86,10 +87,9 @@ public List<IEncapsulateFieldCandidate> EncapsulationCandidates
                     _convertedFields = new List<IEncapsulateFieldCandidate>();
                     foreach (var field in _useBackingFieldCandidates)
                     {
-                        _convertedFields.Add(new ConvertToUDTMember(field, StateUDTField));
+                        _convertedFields.Add(new ConvertToUDTMember(field, ObjectStateUDTField));
                     }
                 }
-
                 return _convertedFields;
             }
         } 
@@ -114,7 +114,7 @@ public IEncapsulateFieldCandidate this[Declaration fieldDeclaration]
             => EncapsulationCandidates.Where(c => c.Declaration == fieldDeclaration).Single();
         
         private IObjectStateUDT _activeObjectStateUDT;
-        public IObjectStateUDT StateUDTField
+        public IObjectStateUDT ObjectStateUDTField
         {
             get
             {
@@ -143,7 +143,15 @@ public IObjectStateUDT StateUDTField
 
                 if (EncapsulateFieldStrategy == EncapsulateFieldStrategy.ConvertFieldsToUDTMembers)
                 {
-                    AssignNoConflictIdentifiers();
+                    foreach (var field in EncapsulationCandidates)
+                    {
+                        if (field is IConvertToUDTMember convertedField)
+                        {
+                            convertedField.ObjectStateUDT = _activeObjectStateUDT;
+                            convertedField.ConflictFinder = _validationsProvider.ConflictDetector(EncapsulateFieldStrategy, _declarationFinderProvider);
+                            convertedField.ConflictFinder.AssignNoConflictIdentifiers(convertedField);
+                        }
+                    }
                 }
             }
         }
@@ -166,26 +174,20 @@ private void UpdateFieldCandidatesForUseBackingFieldsStrategy()
                         candidate.NameValidator = EncapsulateFieldValidationsProvider.NameOnlyValidator(NameValidators.Default);
                         break;
                 }
-                AssignNoConflictIdentifiers();
+                candidate.ConflictFinder = _validationsProvider.ConflictDetector(EncapsulateFieldStrategy, _declarationFinderProvider);
+                candidate.ConflictFinder.AssignNoConflictIdentifiers(candidate);
             }
         }
 
         private void UpdateFieldCandidatesForConvertFieldsToUDTMembersStrategy()
         {
-            foreach (var candidate in EncapsulationCandidates)
+            foreach (var candidate in EncapsulationCandidates.Cast<IConvertToUDTMember>())
             {
+                candidate.ObjectStateUDT = ObjectStateUDTField;
                 candidate.NameValidator = candidate.Declaration.IsArray
                     ? EncapsulateFieldValidationsProvider.NameOnlyValidator(NameValidators.UserDefinedTypeMemberArray)
                     : EncapsulateFieldValidationsProvider.NameOnlyValidator(NameValidators.UserDefinedTypeMember);
 
-                AssignNoConflictIdentifiers();
-            }
-        }
-
-        private void AssignNoConflictIdentifiers()
-        {
-            foreach (var candidate in EncapsulationCandidates)
-            {
                 candidate.ConflictFinder = _validationsProvider.ConflictDetector(EncapsulateFieldStrategy, _declarationFinderProvider);
                 candidate.ConflictFinder.AssignNoConflictIdentifiers(candidate);
             }
diff --git a/Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs b/Rubberduck.Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs
@@ -91,7 +91,7 @@ protected override EncapsulateFieldModel InitializeModel(Declaration target)
             if (builder.ObjectStateUDT != null)
             {
                 model.EncapsulateFieldStrategy = EncapsulateFieldStrategy.ConvertFieldsToUDTMembers;
-                model.StateUDTField = builder.ObjectStateUDT;
+                model.ObjectStateUDTField = builder.ObjectStateUDT;
             }
 
             return model;
diff --git a/Rubberduck.Refactorings/EncapsulateField/EncapsulationStrategies/ConvertFieldsToUDTMembers.cs b/Rubberduck.Refactorings/EncapsulateField/EncapsulationStrategies/ConvertFieldsToUDTMembers.cs
@@ -20,7 +20,7 @@ public class ConvertFieldsToUDTMembers : EncapsulateFieldStrategyBase
         public ConvertFieldsToUDTMembers(IDeclarationFinderProvider declarationFinderProvider, EncapsulateFieldModel model, IIndenter indenter)
             : base(declarationFinderProvider, model, indenter)
         {
-            _stateUDTField = model.StateUDTField;
+            _stateUDTField = model.ObjectStateUDTField;
         }
 
         protected override void ModifyFields(IEncapsulateFieldRewriteSession refactorRewriteSession)
@@ -62,9 +62,9 @@ protected override void LoadNewDeclarationBlocks()
             return;
         }
 
-        protected override void LoadNewPropertyBlocks(/*EncapsulateFieldModel model*/)
+        protected override void LoadNewPropertyBlocks()
         {
-            var propertyGenerationSpecs = /*model.SelectedFieldCandidates*/SelectedFields.SelectMany(f => f.PropertyAttributeSets);
+            var propertyGenerationSpecs = SelectedFields.SelectMany(f => f.PropertyAttributeSets);
 
             var generator = new PropertyGenerator();
             foreach (var spec in propertyGenerationSpecs)
diff --git a/Rubberduck.Refactorings/EncapsulateField/FieldCandidates/ConvertToUDTMemberCandidate.cs b/Rubberduck.Refactorings/EncapsulateField/FieldCandidates/ConvertToUDTMemberCandidate.cs
@@ -16,6 +16,7 @@ public interface IConvertToUDTMember : IEncapsulateFieldCandidate
     {
         string UDTMemberDeclaration { get; }
         IEncapsulateFieldCandidate WrappedCandidate { get; }
+        IObjectStateUDT ObjectStateUDT { set; get; }
     }
 
     public class ConvertToUDTMember : IConvertToUDTMember
@@ -46,7 +47,7 @@ public virtual string UDTMemberDeclaration
 
         public IEncapsulateFieldCandidate WrappedCandidate => _wrapped;
 
-        public IObjectStateUDT ObjectStateUDT { private set; get; }
+        public IObjectStateUDT ObjectStateUDT { set; get; }
 
         public string TargetID => _wrapped.TargetID;
 
diff --git a/Rubberduck.Refactorings/EncapsulateField/ObjectStateUDT.cs b/Rubberduck.Refactorings/EncapsulateField/ObjectStateUDT.cs
@@ -120,7 +120,8 @@ public void AddMembers(IEnumerable<IConvertToUDTMember> fields)
             {
                 foreach (var member in _wrappedUDT.Members)
                 {
-                    _convertedMembers.Add(new ConvertToUDTMember(member, this));
+                    var convertedMember = new ConvertToUDTMember(member, this) { EncapsulateFlag = false };
+                    _convertedMembers.Add(convertedMember);
                 }
             }
             _convertedMembers.AddRange(fields);
diff --git a/RubberduckTests/Refactoring/EncapsulateField/EncapsulateFIeldTestSupport.cs b/RubberduckTests/Refactoring/EncapsulateField/EncapsulateFIeldTestSupport.cs
@@ -74,7 +74,7 @@ public Func<EncapsulateFieldModel, EncapsulateFieldModel> SetParameters(UserInpu
 
                     if (stateUDT != null)
                     {
-                        model.StateUDTField = stateUDT;
+                        model.ObjectStateUDTField = stateUDT;
                     }
                 }
                 else
diff --git a/RubberduckTests/Refactoring/EncapsulateField/EncapsulateUsingStateUDTTests.cs b/RubberduckTests/Refactoring/EncapsulateField/EncapsulateUsingStateUDTTests.cs
@@ -657,6 +657,56 @@ End Sub
             StringAssert.Contains($"  .MyProperty = bar", referencingClassCode);
         }
 
+        [Test]
+        [Category("Refactorings")]
+        [Category("Encapsulate Field")]
+        public void PrivateUDT_SelectedOtherThanObjectStateUDT()
+        {
+            string inputCode =
+$@"
+
+Private Type TTest
+    TestValue As String
+    TestNumber As Long
+End Type
+
+Private Type TTestModule1
+    SomeValue As Long
+End Type
+
+Private mTest As TTest
+
+Private this As TTestModule1
+
+Private the|Target As Variant
+
+Public Property Get SomeValue() As Long
+    SomeValue = this.SomeValue
+End Property
+
+Public Property Let SomeValue(ByVal value As Long)
+    this.SomeValue = value
+End Property
+
+Public Sub Foo(arg As Long)
+    SomeValue = arg * 4
+End Sub
+";
+
+            var userInput = new UserInputDataObject()
+                .UserSelectsField("theTarget");
+
+            userInput.EncapsulateUsingUDTField("mTest");
+
+            var presenterAction = Support.SetParameters(userInput);
+
+            var actualCode = Support.RefactoredCode(inputCode.ToCodeString(), presenterAction);
+
+            StringAssert.DoesNotContain("TheTarget = this.TheTarget", actualCode);
+            StringAssert.Contains("TheTarget = mTest.TheTarget", actualCode);
+            StringAssert.Contains("TheTarget As Variant", actualCode);
+        }
+
         protected override IRefactoring TestRefactoring(IRewritingManager rewritingManager, RubberduckParserState state, IRefactoringPresenterFactory factory, ISelectionService selectionService)
         {
             return Support.SupportTestRefactoring(rewritingManager, state, factory, selectionService);

Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,7 @@ public IObjectStateUDT SelectedObjectStateUDT`
`92`	`92`	`set`
`93`	`93`	`{`
`94`	`94`	`_selectedObjectStateUDT = value;`
`95`		`- Model.StateUDTField = _selectedObjectStateUDT;`
	`95`	`+ Model.ObjectStateUDTField = _selectedObjectStateUDT;`
`96`	`96`	`SetObjectStateUDT();`
`97`	`97`	`}`
`98`	`98`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`using System;`
`2`	`2`	`using System.Collections.Generic;`
	`3`	`+using System.Diagnostics;`
`3`	`4`	`using System.Linq;`
`4`	`5`	`using Rubberduck.Parsing.Symbols;`
`5`	`6`	`using Rubberduck.Parsing.VBA;`
`@@ -41,7 +42,7 @@ public EncapsulateFieldModel(`
`41`	`42`	`_objStateCandidates.Add(_newObjectStateUDT);`
`42`	`43`
`43`	`44`	`EncapsulateFieldStrategy = EncapsulateFieldStrategy.UseBackingFields;`
`44`		`- _activeObjectStateUDT = StateUDTField;`
	`45`	`+ _activeObjectStateUDT = ObjectStateUDTField;`
`45`	`46`	`}`
`46`	`47`
`47`	`48`	`public QualifiedModuleName QualifiedModuleName => _targetQMN;`
`@@ -86,10 +87,9 @@ public List<IEncapsulateFieldCandidate> EncapsulationCandidates`
`86`	`87`	`_convertedFields = new List<IEncapsulateFieldCandidate>();`
`87`	`88`	`foreach (var field in _useBackingFieldCandidates)`
`88`	`89`	`{`
`89`		`- _convertedFields.Add(new ConvertToUDTMember(field, StateUDTField));`
	`90`	`+ _convertedFields.Add(new ConvertToUDTMember(field, ObjectStateUDTField));`
`90`	`91`	`}`
`91`	`92`	`}`
`92`		`-`
`93`	`93`	`return _convertedFields;`
`94`	`94`	`}`
`95`	`95`	`}`
`@@ -114,7 +114,7 @@ public IEncapsulateFieldCandidate this[Declaration fieldDeclaration]`
`114`	`114`	`=> EncapsulationCandidates.Where(c => c.Declaration == fieldDeclaration).Single();`
`115`	`115`
`116`	`116`	`private IObjectStateUDT _activeObjectStateUDT;`
`117`		`- public IObjectStateUDT StateUDTField`
	`117`	`+ public IObjectStateUDT ObjectStateUDTField`
`118`	`118`	`{`
`119`	`119`	`get`
`120`	`120`	`{`
`@@ -143,7 +143,15 @@ public IObjectStateUDT StateUDTField`
`143`	`143`
`144`	`144`	`if (EncapsulateFieldStrategy == EncapsulateFieldStrategy.ConvertFieldsToUDTMembers)`
`145`	`145`	`{`
`146`		`- AssignNoConflictIdentifiers();`
	`146`	`+ foreach (var field in EncapsulationCandidates)`
	`147`	`+ {`
	`148`	`+ if (field is IConvertToUDTMember convertedField)`
	`149`	`+ {`
	`150`	`+ convertedField.ObjectStateUDT = _activeObjectStateUDT;`
	`151`	`+ convertedField.ConflictFinder = _validationsProvider.ConflictDetector(EncapsulateFieldStrategy, _declarationFinderProvider);`
	`152`	`+ convertedField.ConflictFinder.AssignNoConflictIdentifiers(convertedField);`
	`153`	`+ }`
	`154`	`+ }`
`147`	`155`	`}`
`148`	`156`	`}`
`149`	`157`	`}`
`@@ -166,26 +174,20 @@ private void UpdateFieldCandidatesForUseBackingFieldsStrategy()`
`166`	`174`	`candidate.NameValidator = EncapsulateFieldValidationsProvider.NameOnlyValidator(NameValidators.Default);`
`167`	`175`	`break;`
`168`	`176`	`}`
`169`		`- AssignNoConflictIdentifiers();`
	`177`	`+ candidate.ConflictFinder = _validationsProvider.ConflictDetector(EncapsulateFieldStrategy, _declarationFinderProvider);`
	`178`	`+ candidate.ConflictFinder.AssignNoConflictIdentifiers(candidate);`
`170`	`179`	`}`
`171`	`180`	`}`
`172`	`181`
`173`	`182`	`private void UpdateFieldCandidatesForConvertFieldsToUDTMembersStrategy()`
`174`	`183`	`{`
`175`		`- foreach (var candidate in EncapsulationCandidates)`
	`184`	`+ foreach (var candidate in EncapsulationCandidates.Cast<IConvertToUDTMember>())`
`176`	`185`	`{`
	`186`	`+ candidate.ObjectStateUDT = ObjectStateUDTField;`
`177`	`187`	`candidate.NameValidator = candidate.Declaration.IsArray`
`178`	`188`	`? EncapsulateFieldValidationsProvider.NameOnlyValidator(NameValidators.UserDefinedTypeMemberArray)`
`179`	`189`	`: EncapsulateFieldValidationsProvider.NameOnlyValidator(NameValidators.UserDefinedTypeMember);`
`180`	`190`
`181`		`- AssignNoConflictIdentifiers();`
`182`		`- }`
`183`		`- }`
`184`		`-`
`185`		`- private void AssignNoConflictIdentifiers()`
`186`		`- {`
`187`		`- foreach (var candidate in EncapsulationCandidates)`
`188`		`- {`
`189`	`191`	`candidate.ConflictFinder = _validationsProvider.ConflictDetector(EncapsulateFieldStrategy, _declarationFinderProvider);`
`190`	`192`	`candidate.ConflictFinder.AssignNoConflictIdentifiers(candidate);`
`191`	`193`	`}`
Original file line number	Diff line number	Diff line change
`@@ -91,7 +91,7 @@ protected override EncapsulateFieldModel InitializeModel(Declaration target)`
`91`	`91`	`if (builder.ObjectStateUDT != null)`
`92`	`92`	`{`
`93`	`93`	`model.EncapsulateFieldStrategy = EncapsulateFieldStrategy.ConvertFieldsToUDTMembers;`
`94`		`- model.StateUDTField = builder.ObjectStateUDT;`
	`94`	`+ model.ObjectStateUDTField = builder.ObjectStateUDT;`
`95`	`95`	`}`
`96`	`96`
`97`	`97`	`return model;`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ public class ConvertFieldsToUDTMembers : EncapsulateFieldStrategyBase`
`20`	`20`	`public ConvertFieldsToUDTMembers(IDeclarationFinderProvider declarationFinderProvider, EncapsulateFieldModel model, IIndenter indenter)`
`21`	`21`	`: base(declarationFinderProvider, model, indenter)`
`22`	`22`	`{`
`23`		`- _stateUDTField = model.StateUDTField;`
	`23`	`+ _stateUDTField = model.ObjectStateUDTField;`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`protected override void ModifyFields(IEncapsulateFieldRewriteSession refactorRewriteSession)`
`@@ -62,9 +62,9 @@ protected override void LoadNewDeclarationBlocks()`
`62`	`62`	`return;`
`63`	`63`	`}`
`64`	`64`
`65`		`- protected override void LoadNewPropertyBlocks(/EncapsulateFieldModel model/)`
	`65`	`+ protected override void LoadNewPropertyBlocks()`
`66`	`66`	`{`
`67`		`- var propertyGenerationSpecs = /model.SelectedFieldCandidates/SelectedFields.SelectMany(f => f.PropertyAttributeSets);`
	`67`	`+ var propertyGenerationSpecs = SelectedFields.SelectMany(f => f.PropertyAttributeSets);`
`68`	`68`
`69`	`69`	`var generator = new PropertyGenerator();`
`70`	`70`	`foreach (var spec in propertyGenerationSpecs)`
Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@ public interface IConvertToUDTMember : IEncapsulateFieldCandidate`
`16`	`16`	`{`
`17`	`17`	`string UDTMemberDeclaration { get; }`
`18`	`18`	`IEncapsulateFieldCandidate WrappedCandidate { get; }`
	`19`	`+ IObjectStateUDT ObjectStateUDT { set; get; }`
`19`	`20`	`}`
`20`	`21`
`21`	`22`	`public class ConvertToUDTMember : IConvertToUDTMember`
`@@ -46,7 +47,7 @@ public virtual string UDTMemberDeclaration`
`46`	`47`
`47`	`48`	`public IEncapsulateFieldCandidate WrappedCandidate => _wrapped;`
`48`	`49`
`49`		`- public IObjectStateUDT ObjectStateUDT { private set; get; }`
	`50`	`+ public IObjectStateUDT ObjectStateUDT { set; get; }`
`50`	`51`
`51`	`52`	`public string TargetID => _wrapped.TargetID;`
`52`	`53`
Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,8 @@ public void AddMembers(IEnumerable<IConvertToUDTMember> fields)`
`120`	`120`	`{`
`121`	`121`	`foreach (var member in _wrappedUDT.Members)`
`122`	`122`	`{`
`123`		`- _convertedMembers.Add(new ConvertToUDTMember(member, this));`
	`123`	`+ var convertedMember = new ConvertToUDTMember(member, this) { EncapsulateFlag = false };`
	`124`	`+ _convertedMembers.Add(convertedMember);`
`124`	`125`	`}`
`125`	`126`	`}`
`126`	`127`	`_convertedMembers.AddRange(fields);`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ public Func<EncapsulateFieldModel, EncapsulateFieldModel> SetParameters(UserInpu`
`74`	`74`
`75`	`75`	`if (stateUDT != null)`
`76`	`76`	`{`
`77`		`- model.StateUDTField = stateUDT;`
	`77`	`+ model.ObjectStateUDTField = stateUDT;`
`78`	`78`	`}`
`79`	`79`	`}`
`80`	`80`	`else`