Skip to content

Commit ef1c9cc

Browse files
committed
Remove blank lines left by declaration removals
Incorporates IModuleRewriter extension method to clean up newlines left by declaration removals.
1 parent c935798 commit ef1c9cc

File tree

3 files changed

+92
-20
lines changed

3 files changed

+92
-20
lines changed

Rubberduck.Refactorings/MoveCloserToUsage/MoveCloserToUsageRefactoringAction.cs

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using Rubberduck.Parsing.Grammar;
55
using Rubberduck.Parsing.Rewriter;
66
using Rubberduck.Parsing.Symbols;
7+
using Rubberduck.Refactorings.Common;
78

89
namespace Rubberduck.Refactorings.MoveCloserToUsage
910
{
@@ -16,12 +17,17 @@ public MoveCloserToUsageRefactoringAction(IRewritingManager rewritingManager)
1617
protected override void Refactor(MoveCloserToUsageModel model, IRewriteSession rewriteSession)
1718
{
1819
var target = model.Target;
19-
InsertNewDeclaration(target, rewriteSession);
20-
RemoveOldDeclaration(target, rewriteSession);
21-
UpdateQualifiedCalls(target, rewriteSession);
20+
if (target is VariableDeclaration variable)
21+
{
22+
InsertNewDeclaration(variable, rewriteSession);
23+
RemoveOldDeclaration(variable, rewriteSession);
24+
UpdateQualifiedCalls(variable, rewriteSession);
25+
return;
26+
}
27+
throw new ArgumentException("Invalid target declaration type");
2228
}
2329

24-
private void InsertNewDeclaration(Declaration target, IRewriteSession rewriteSession)
30+
private void InsertNewDeclaration(VariableDeclaration target, IRewriteSession rewriteSession)
2531
{
2632
var subscripts = target.Context.GetDescendent<VBAParser.SubscriptsContext>()?.GetText() ?? string.Empty;
2733
var identifier = target.IsArray ? $"{target.IdentifierName}({subscripts})" : target.IdentifierName;
@@ -75,13 +81,22 @@ private string PaddedDeclaration(string declarationText, VBAParser.BlockStmtCont
7581
return $"{declarationText}{Environment.NewLine}";
7682
}
7783

78-
private void RemoveOldDeclaration(Declaration target, IRewriteSession rewriteSession)
84+
private void RemoveOldDeclaration(VariableDeclaration target, IRewriteSession rewriteSession)
7985
{
8086
var rewriter = rewriteSession.CheckOutModuleRewriter(target.QualifiedModuleName);
81-
rewriter.Remove(target);
87+
88+
//If a label precedes the declaration, then delete just the variable so that the line and label are retained.
89+
if (target.Context.TryGetAncestor<VBAParser.BlockStmtContext>(out var blockContext)
90+
&& blockContext.children.Any(c => c is VBAParser.StatementLabelDefinitionContext))
91+
{
92+
rewriter.Remove(target);
93+
return;
94+
}
95+
96+
rewriter.RemoveVariables(new VariableDeclaration[] { target });
8297
}
8398

84-
private void UpdateQualifiedCalls(Declaration target, IRewriteSession rewriteSession)
99+
private void UpdateQualifiedCalls(VariableDeclaration target, IRewriteSession rewriteSession)
85100
{
86101
var references = target.References.ToList();
87102
var rewriter = rewriteSession.CheckOutModuleRewriter(references.First().QualifiedModuleName);

RubberduckTests/QuickFixes/MoveFieldCloserToUsageQuickFixTests.cs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using Rubberduck.CodeAnalysis.Inspections.Concrete;
66
using Rubberduck.CodeAnalysis.QuickFixes.Concrete.Refactoring;
77
using RubberduckTests.Mocks;
8-
using Rubberduck.Interaction;
98
using Rubberduck.Parsing.VBA;
109
using Rubberduck.Refactorings.MoveCloserToUsage;
1110
using Rubberduck.VBEditor;
@@ -55,8 +54,7 @@ End Sub
5554
";
5655

5756
const string expectedCode =
58-
@"
59-
Public Sub Foo()
57+
@"Public Sub Foo()
6058
Dim bar As String
6159
If bar = ""test"" Then Baz Else Foobar
6260
End Sub
@@ -84,8 +82,7 @@ Public Sub Foo()
8482
End Sub";
8583

8684
const string expectedCode =
87-
@"
88-
Public Sub Foo()
85+
@"Public Sub Foo()
8986
Dim bar As String
9087
If True Then bar = ""test""
9188
End Sub";
@@ -106,8 +103,7 @@ Public Sub Foo()
106103
End Sub";
107104

108105
const string expectedCode =
109-
@"
110-
Public Sub Foo()
106+
@"Public Sub Foo()
111107
Dim bar As String
112108
If True Then Else bar = ""test""
113109
End Sub";

RubberduckTests/Refactoring/MoveCloserToUsageTests.cs

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,68 @@ namespace RubberduckTests.Refactoring
1717
[TestFixture]
1818
public class MoveCloserToUsageTests : RefactoringTestBase
1919
{
20+
[Test]
21+
[Category("Refactorings")]
22+
[Category("Move Closer")]
23+
public void MoveCloserToUsageRefactoring_ModuleVariable_ClearsResidualNewLines()
24+
{
25+
//Input
26+
const string inputCode =
27+
@"Private bar As Boolean
28+
29+
30+
31+
32+
33+
Private Sub Foo()
34+
bar = True
35+
End Sub";
36+
var selection = new Selection(1, 1);
37+
38+
//Expectation
39+
const string expectedCode =
40+
@"Private Sub Foo()
41+
Dim bar As Boolean
42+
bar = True
43+
End Sub";
44+
45+
var actualCode = RefactoredCode(inputCode, selection);
46+
Assert.AreEqual(expectedCode, actualCode);
47+
}
48+
49+
[Test]
50+
[Category("Refactorings")]
51+
[Category("Move Closer")]
52+
public void MoveCloserToUsageRefactoring_LocalVariable_ClearsResidualNewLines()
53+
{
54+
//Input
55+
const string inputCode =
56+
@"Private Sub Foo()
57+
Dim bar As Boolean
58+
59+
Dim var1 As Long
60+
61+
Dim var2 As String
62+
63+
bar = True
64+
End Sub";
65+
var selection = new Selection(2, 10);
66+
67+
//Expectation
68+
const string expectedCode =
69+
@"Private Sub Foo()
70+
Dim var1 As Long
71+
72+
Dim var2 As String
73+
74+
Dim bar As Boolean
75+
bar = True
76+
End Sub";
77+
78+
var actualCode = RefactoredCode(inputCode, selection);
79+
Assert.AreEqual(expectedCode, actualCode);
80+
}
81+
2082
[Test]
2183
[Category("Refactorings")]
2284
[Category("Move Closer")]
@@ -25,6 +87,8 @@ public void MoveCloserToUsageRefactoring_Field()
2587
//Input
2688
const string inputCode =
2789
@"Private bar As Boolean
90+
91+
2892
Private Sub Foo()
2993
bar = True
3094
End Sub";
@@ -550,7 +614,6 @@ Debug.Print someParam
550614
var selection = new Selection(2, 1);
551615
const string expectedCode =
552616
@"
553-
554617
Public Sub Test()
555618
Dim foo As Long
556619
SomeSub someParam:=foo
@@ -581,8 +644,8 @@ Debug.Print someParam
581644

582645
var selection = new Selection(1, 1);
583646
const string expectedCode =
584-
@"
585-
Public Sub Test(): Dim foo As Long : SomeSub someParam:=foo: End Sub
647+
648+
@"Public Sub Test(): Dim foo As Long : SomeSub someParam:=foo: End Sub
586649
587650
Public Sub SomeSub(ByVal someParam As Long)
588651
Debug.Print someParam
@@ -929,9 +992,7 @@ Public Sub Test()
929992

930993
var selection = new Selection(1, 1);
931994

932-
const string expectedCode = @"
933-
934-
Public Sub Test()
995+
const string expectedCode = @"Public Sub Test()
935996
Debug.Print ""Some statements between""
936997
Debug.Print ""Declaration and first usage!""
937998
Dim foo As Class1

0 commit comments

Comments
 (0)