Skip to content

Commit 8494653

Browse files
Hosch250Hosch250
committed
Close #2116. Fix broken Encapsulate Public Field quick fix
1 parent c04f432 commit 8494653

File tree

5 files changed

+67
-14
lines changed

5 files changed

+67
-14
lines changed

‎RetailCoder.VBE/Refactorings/EncapsulateField/EncapsulateFieldModel.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public class EncapsulateFieldModel
1818
public string ParameterName { get; set; }
1919
public bool ImplementLetSetterType { get; set; }
2020
public bool ImplementSetSetterType { get; set; }
21+
public bool CanImplementLet { get; set; }
2122

2223
public EncapsulateFieldModel(RubberduckParserState state, QualifiedSelection selection)
2324
{

‎RetailCoder.VBE/Refactorings/EncapsulateField/EncapsulateFieldPresenter.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ public EncapsulateFieldModel Show()
9696
_model.PropertyName = _view.NewPropertyName;
9797
_model.ImplementLetSetterType = _view.MustImplementLetSetterType;
9898
_model.ImplementSetSetterType = _view.MustImplementSetSetterType;
99+
_model.CanImplementLet = _view.CanImplementLetSetterType;
99100

100101
_model.ParameterName = _view.ParameterName;
101102
return _model;

‎RetailCoder.VBE/Refactorings/EncapsulateField/EncapsulateFieldRefactoring.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,13 @@ public void Refactor()
5050

5151
public void Refactor(QualifiedSelection target)
5252
{
53+
_vbe.ActiveCodePane.CodeModule.SetSelection(target);
5354
Refactor();
5455
}
5556

5657
public void Refactor(Declaration target)
5758
{
59+
_vbe.ActiveCodePane.CodeModule.SetSelection(target.QualifiedSelection);
5860
Refactor();
5961
}
6062

@@ -188,7 +190,7 @@ private string GetPropertyText()
188190
var getterText = string.Join(Environment.NewLine,
189191
string.Format(Environment.NewLine + "Public Property Get {0}() As {1}", _model.PropertyName,
190192
_model.TargetDeclaration.AsTypeName),
191-
string.Format(" {0} = {1}", _model.PropertyName, _model.TargetDeclaration.IdentifierName),
193+
string.Format(" {0}{1} = {2}", !_model.CanImplementLet || _model.ImplementSetSetterType ? "Set " : string.Empty, _model.PropertyName, _model.TargetDeclaration.IdentifierName),
192194
"End Property" + Environment.NewLine);
193195

194196
var letterText = string.Join(Environment.NewLine,
@@ -200,7 +202,7 @@ private string GetPropertyText()
200202
var setterText = string.Join(Environment.NewLine,
201203
string.Format(Environment.NewLine + "Public Property Set {0}(ByVal {1} As {2})",
202204
_model.PropertyName, _model.ParameterName, _model.TargetDeclaration.AsTypeName),
203-
string.Format(" {0} = {1}", _model.TargetDeclaration.IdentifierName, _model.ParameterName),
205+
string.Format(" Set {0} = {1}", _model.TargetDeclaration.IdentifierName, _model.ParameterName),
204206
"End Property" + Environment.NewLine);
205207

206208
return string.Join(string.Empty,

‎RetailCoder.VBE/UI/Refactorings/EncapsulateFieldDialog.cs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,16 @@ private void VariableNameBox_TextChanged(object sender, EventArgs e)
127127

128128
private void UpdatePreview()
129129
{
130-
if (TargetDeclaration == null) { return; }
130+
PreviewBox.Text = GetPropertyText();
131+
}
132+
133+
private string GetPropertyText()
134+
{
135+
if (TargetDeclaration == null) { return string.Empty; }
131136

132137
var getterText = string.Join(Environment.NewLine,
133138
string.Format("Public Property Get {0}() As {1}", NewPropertyName, TargetDeclaration.AsTypeName),
134-
string.Format(" {0} = {1}", NewPropertyName, TargetDeclaration.IdentifierName),
139+
string.Format(" {0}{1} = {2}", MustImplementSetSetterType || !CanImplementLetSetterType ? "Set " : string.Empty, NewPropertyName, TargetDeclaration.IdentifierName),
135140
"End Property");
136141

137142
var letterText = string.Join(Environment.NewLine,
@@ -143,12 +148,12 @@ private void UpdatePreview()
143148
var setterText = string.Join(Environment.NewLine,
144149
string.Format(Environment.NewLine + Environment.NewLine + "Public Property Set {0}(ByVal {1} As {2})",
145150
NewPropertyName, ParameterName, TargetDeclaration.AsTypeName),
146-
string.Format(" {0} = {1}", TargetDeclaration.IdentifierName, ParameterName),
151+
string.Format(" Set {0} = {1}", TargetDeclaration.IdentifierName, ParameterName),
147152
"End Property");
148153

149-
PreviewBox.Text = getterText +
150-
(MustImplementLetSetterType ? letterText : string.Empty) +
151-
(MustImplementSetSetterType ? setterText : string.Empty);
154+
return getterText +
155+
(MustImplementLetSetterType ? letterText : string.Empty) +
156+
(MustImplementSetSetterType ? setterText : string.Empty);
152157
}
153158

154159
private void ValidatePropertyName()

‎RubberduckTests/Refactoring/EncapsulateFieldTests.cs

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ End Property
5959
{
6060
ImplementLetSetterType = true,
6161
ImplementSetSetterType = false,
62+
CanImplementLet = true,
6263
ParameterName = "value",
6364
PropertyName = "Name"
6465
};
@@ -117,6 +118,7 @@ End Property
117118
{
118119
ImplementLetSetterType = true,
119120
ImplementSetSetterType = false,
121+
CanImplementLet = true,
120122
ParameterName = "value",
121123
PropertyName = "Name"
122124
};
@@ -145,11 +147,11 @@ public void EncapsulatePublicField_WithSetter()
145147
@"Private fizz As Variant
146148
147149
Public Property Get Name() As Variant
148-
Name = fizz
150+
Set Name = fizz
149151
End Property
150152
151153
Public Property Set Name(ByVal value As Variant)
152-
fizz = value
154+
Set fizz = value
153155
End Property
154156
";
155157

@@ -172,6 +174,7 @@ End Property
172174
{
173175
ImplementLetSetterType = false,
174176
ImplementSetSetterType = true,
177+
CanImplementLet = true,
175178
ParameterName = "value",
176179
PropertyName = "Name"
177180
};
@@ -223,6 +226,7 @@ End Property
223226
{
224227
ImplementLetSetterType = false,
225228
ImplementSetSetterType = false,
229+
CanImplementLet = true,
226230
ParameterName = "value",
227231
PropertyName = "Name"
228232
};
@@ -291,6 +295,7 @@ Function Bar() As Integer
291295
{
292296
ImplementLetSetterType = true,
293297
ImplementSetSetterType = false,
298+
CanImplementLet = true,
294299
ParameterName = "value",
295300
PropertyName = "Name"
296301
};
@@ -365,6 +370,7 @@ Property Set Foo(ByVal vall As Variant)
365370
{
366371
ImplementLetSetterType = true,
367372
ImplementSetSetterType = false,
373+
CanImplementLet = true,
368374
ParameterName = "value",
369375
PropertyName = "Name"
370376
};
@@ -422,6 +428,7 @@ End Property
422428
{
423429
ImplementLetSetterType = true,
424430
ImplementSetSetterType = false,
431+
CanImplementLet = true,
425432
ParameterName = "value",
426433
PropertyName = "Name"
427434
};
@@ -453,15 +460,15 @@ public void EncapsulatePublicField_FieldDeclarationHasMultipleFields_MoveFirst()
453460
Private fizz As Variant
454461
455462
Public Property Get Name() As Variant
456-
Name = fizz
463+
Set Name = fizz
457464
End Property
458465
459466
Public Property Let Name(ByVal value As Variant)
460467
fizz = value
461468
End Property
462469
463470
Public Property Set Name(ByVal value As Variant)
464-
fizz = value
471+
Set fizz = value
465472
End Property
466473
"; // note: VBE removes excess spaces
467474

@@ -484,6 +491,7 @@ End Property
484491
{
485492
ImplementLetSetterType = true,
486493
ImplementSetSetterType = true,
494+
CanImplementLet = true,
487495
ParameterName = "value",
488496
PropertyName = "Name"
489497
};
@@ -542,6 +550,7 @@ End Property
542550
{
543551
ImplementLetSetterType = true,
544552
ImplementSetSetterType = false,
553+
CanImplementLet = true,
545554
ParameterName = "value",
546555
PropertyName = "Name"
547556
};
@@ -600,6 +609,7 @@ End Property
600609
{
601610
ImplementLetSetterType = true,
602611
ImplementSetSetterType = false,
612+
CanImplementLet = true,
603613
ParameterName = "value",
604614
PropertyName = "Name"
605615
};
@@ -655,6 +665,7 @@ End Property
655665
{
656666
ImplementLetSetterType = true,
657667
ImplementSetSetterType = false,
668+
CanImplementLet = true,
658669
ParameterName = "value",
659670
PropertyName = "Name"
660671
};
@@ -725,6 +736,7 @@ Sub Bar(ByVal name As Integer)
725736
{
726737
ImplementLetSetterType = true,
727738
ImplementSetSetterType = false,
739+
CanImplementLet = true,
728740
ParameterName = "value",
729741
PropertyName = "Name"
730742
};
@@ -814,6 +826,7 @@ Sub Bar(ByVal v As Integer)
814826
{
815827
ImplementLetSetterType = true,
816828
ImplementSetSetterType = false,
829+
CanImplementLet = true,
817830
ParameterName = "value",
818831
PropertyName = "Name"
819832
};
@@ -873,6 +886,7 @@ End Property
873886
{
874887
ImplementLetSetterType = true,
875888
ImplementSetSetterType = false,
889+
CanImplementLet = true,
876890
ParameterName = "value",
877891
PropertyName = "Name"
878892
};
@@ -889,7 +903,7 @@ End Property
889903
}
890904

891905
[TestMethod]
892-
public void RemoveParams_PresenterIsNull()
906+
public void EncapsulateField_PresenterIsNull()
893907
{
894908
//Input
895909
const string inputCode =
@@ -917,7 +931,7 @@ public void RemoveParams_PresenterIsNull()
917931
}
918932

919933
[TestMethod]
920-
public void RemoveParams_ModelIsNull()
934+
public void EncapsulateField_ModelIsNull()
921935
{
922936
//Input
923937
const string inputCode =
@@ -1133,6 +1147,36 @@ public void Presenter_Accept_ReturnsModelWithPropertyNameChanged()
11331147
Assert.AreEqual("MyProperty", presenter.Show().PropertyName);
11341148
}
11351149

1150+
[TestMethod]
1151+
public void Presenter_Accept_ReturnsModelWithCanImplementLetChanged()
1152+
{
1153+
//Input
1154+
const string inputCode =
1155+
@"Private fizz As Variant";
1156+
var selection = new Selection(1, 15, 1, 15);
1157+
1158+
//Arrange
1159+
var builder = new MockVbeBuilder();
1160+
VBComponent component;
1161+
var vbe = builder.BuildFromSingleStandardModule(inputCode, out component, selection);
1162+
var mockHost = new Mock<IHostApplication>();
1163+
mockHost.SetupAllProperties();
1164+
var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
1165+
1166+
parser.Parse(new CancellationTokenSource());
1167+
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
1168+
1169+
var view = new Mock<IEncapsulateFieldDialog>();
1170+
view.SetupProperty(v => v.CanImplementLetSetterType, true);
1171+
view.Setup(v => v.ShowDialog()).Returns(DialogResult.OK);
1172+
1173+
var factory = new EncapsulateFieldPresenterFactory(vbe.Object, parser.State, view.Object);
1174+
1175+
var presenter = factory.Create();
1176+
1177+
Assert.AreEqual(true, presenter.Show().CanImplementLet);
1178+
}
1179+
11361180
[TestMethod]
11371181
public void Presenter_Accept_ReturnsModelWithImplementLetChanged()
11381182
{

0 commit comments

Comments
 (0)