Skip to content

Commit

Permalink
Merge pull request #2758 from comintern/next
Browse files Browse the repository at this point in the history
Fix statement separator handling in MoveCloserToUsageRefactoring. Closes #2753
  • Loading branch information
retailcoder committed Mar 1, 2017
2 parents 359cbcf + aa59e31 commit a93fce9
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 11 deletions.
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
using System.Windows.Forms;
using Rubberduck.Common;
using Rubberduck.Parsing;
Expand Down Expand Up @@ -170,7 +171,8 @@ private void InsertDeclaration()

var newLinesWithoutStringLiterals = newLines.StripStringLiterals();

var lastIndexOfColon = newLinesWithoutStringLiterals.LastIndexOf(':');
var lastIndexOfColon = GetIndexOfLastStatementSeparator(newLinesWithoutStringLiterals);
// ReSharper disable once StringLastIndexOfIsCultureSpecific.1
while (lastIndexOfColon != -1)
{
var numberOfCharsToRemove = lastIndexOfColon == newLines.Length - 1 || newLines[lastIndexOfColon + 1] != ' '
Expand All @@ -185,14 +187,21 @@ private void InsertDeclaration()
.Remove(lastIndexOfColon, numberOfCharsToRemove)
.Insert(lastIndexOfColon, Environment.NewLine);

lastIndexOfColon = newLinesWithoutStringLiterals.LastIndexOf(':');
lastIndexOfColon = GetIndexOfLastStatementSeparator(newLinesWithoutStringLiterals);
}

module.DeleteLines(beginningOfInstructionSelection.StartLine, beginningOfInstructionSelection.LineCount);
module.InsertLines(beginningOfInstructionSelection.StartLine, newLines);
}
}

private static readonly Regex StatementSeparatorRegex = new Regex(":[^=]", RegexOptions.RightToLeft);
private static int GetIndexOfLastStatementSeparator(string input)
{
var matches = StatementSeparatorRegex.Matches(input);
return matches.Count == 0 ? -1 : matches[0].Index;
}

private Selection GetBeginningOfInstructionSelection(IdentifierReference reference)
{
var referenceSelection = reference.Selection;
Expand All @@ -201,7 +210,7 @@ private Selection GetBeginningOfInstructionSelection(IdentifierReference referen
var currentLine = referenceSelection.StartLine;

var codeLine = module.GetLines(currentLine, 1).StripStringLiterals();
while (codeLine.Remove(referenceSelection.StartColumn).LastIndexOf(':') == -1)
while (GetIndexOfLastStatementSeparator(codeLine.Remove(referenceSelection.StartColumn)) == -1)
{
codeLine = module.GetLines(--currentLine, 1).StripStringLiterals();
if (!codeLine.EndsWith(" _"))
Expand All @@ -210,7 +219,7 @@ private Selection GetBeginningOfInstructionSelection(IdentifierReference referen
}
}

var index = codeLine.Remove(referenceSelection.StartColumn).LastIndexOf(':') + 1;
var index = GetIndexOfLastStatementSeparator(codeLine.Remove(referenceSelection.StartColumn)) + 1;
return new Selection(currentLine, index, currentLine, index);
}
}
Expand Down
9 changes: 4 additions & 5 deletions RetailCoder.VBE/UI/SelectionChangeService.cs
Expand Up @@ -81,8 +81,7 @@ private void DispatchSelectionChanged(DeclarationChangedEventArgs eventArgs)
}
SelectionChanged.Invoke(null, eventArgs);
}



private void DispatchSelectedDeclaration(DeclarationChangedEventArgs eventArgs)
{
DispatchSelectionChanged(eventArgs);
Expand Down Expand Up @@ -111,7 +110,7 @@ private void DispatchSelectedDesignerDeclaration(IVBComponent component)
{
var name = component.SelectedControls.First().Name;
var control =
_parser.State.AllUserDeclarations.SingleOrDefault(decl =>
_parser.State.DeclarationFinder.UserDeclarations(DeclarationType.Control).SingleOrDefault(decl =>
decl.IdentifierName.Equals(name) &&
decl.ParentDeclaration.IdentifierName.Equals(component.Name) &&
decl.ProjectId.Equals(component.ParentProject.ProjectId));
Expand All @@ -120,7 +119,7 @@ private void DispatchSelectedDesignerDeclaration(IVBComponent component)
return;
}
var form =
_parser.State.AllUserDeclarations.SingleOrDefault(decl =>
_parser.State.DeclarationFinder.UserDeclarations(DeclarationType.UserForm).SingleOrDefault(decl =>
decl.IdentifierName.Equals(component.Name) &&
decl.ProjectId.Equals(component.ParentProject.ProjectId));

Expand All @@ -144,7 +143,7 @@ private void DispatchSelectedProjectNodeDeclaration(IVBComponent component)
}
else if (component != null)
{
//The user might have selected the project node in Project Explorer. If they've chosen a folder, we'll return the project anyway.

var module =
_parser.State.AllUserDeclarations.SingleOrDefault(
decl => decl.DeclarationType.HasFlag(DeclarationType.Module) &&
Expand Down
2 changes: 1 addition & 1 deletion Rubberduck.VBEEditor/WindowsApi/CodePaneSubclass.cs
Expand Up @@ -19,7 +19,7 @@ internal CodePaneSubclass(IntPtr hwnd, ICodePane pane) : base(hwnd)
protected override void DispatchFocusEvent(FocusType type)
{
var window = VBENativeServices.GetWindowInfoFromHwnd(Hwnd);
if (window == null)
if (!window.HasValue)
{
return;
}
Expand Down
3 changes: 2 additions & 1 deletion Rubberduck.VBEEditor/WindowsApi/FocusSource.cs
@@ -1,6 +1,7 @@
using System;
using Rubberduck.Common.WinAPI;
using Rubberduck.VBEditor.Events;
using Rubberduck.VBEditor.SafeComWrappers.MSForms;

namespace Rubberduck.VBEditor.WindowsApi
{
Expand All @@ -20,7 +21,7 @@ protected void OnFocusChange(WindowChangedEventArgs eventArgs)
protected virtual void DispatchFocusEvent(FocusType type)
{
var window = VBENativeServices.GetWindowInfoFromHwnd(Hwnd);
if (window == null)
if (!window.HasValue)
{
return;
}
Expand Down
100 changes: 100 additions & 0 deletions RubberduckTests/Refactoring/MoveCloserToUsageTests.cs
Expand Up @@ -849,6 +849,106 @@ End Sub
Assert.AreEqual(expectedCode, module.Content());
}

[TestMethod]
public void MoveCloserToUsageRefactoring_WorksWithNamedParameters()
{
//Input
const string inputCode =
@"Private foo As Long
Public Sub Test()
SomeSub someParam:=foo
End Sub
Public Sub SomeSub(ByVal someParam As Long)
Debug.Print someParam
End Sub";

var selection = new Selection(1, 1, 1, 1);
const string expectedCode =
@"
Public Sub Test()
Dim foo As Long
SomeSub someParam:=foo
End Sub
Public Sub SomeSub(ByVal someParam As Long)
Debug.Print someParam
End Sub";

//Arrange
var builder = new MockVbeBuilder();
IVBComponent component;
var vbe = builder.BuildFromSingleStandardModule(inputCode, out component, selection);
var project = vbe.Object.VBProjects[0];
var module = project.VBComponents[0].CodeModule;
var mockHost = new Mock<IHostApplication>();
mockHost.SetupAllProperties();
var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));

parser.Parse(new CancellationTokenSource());
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }

var qualifiedSelection = new QualifiedSelection(new QualifiedModuleName(component), selection);

//Act
var refactoring = new MoveCloserToUsageRefactoring(vbe.Object, parser.State, null);
refactoring.Refactor(qualifiedSelection);

//Assert
Assert.AreEqual(expectedCode, module.Content());
}

[TestMethod]
public void MoveCloserToUsageRefactoring_WorksWithNamedParametersAndStatementSeparaters()
{
//Input
const string inputCode =
@"Private foo As Long
Public Sub Test(): SomeSub someParam:=foo: End Sub
Public Sub SomeSub(ByVal someParam As Long)
Debug.Print someParam
End Sub";

var selection = new Selection(1, 1, 1, 1);
const string expectedCode =
@"
Public Sub Test()
Dim foo As Long
SomeSub someParam:=foo
End Sub
Public Sub SomeSub(ByVal someParam As Long)
Debug.Print someParam
End Sub";

//Arrange
var builder = new MockVbeBuilder();
IVBComponent component;
var vbe = builder.BuildFromSingleStandardModule(inputCode, out component, selection);
var project = vbe.Object.VBProjects[0];
var module = project.VBComponents[0].CodeModule;
var mockHost = new Mock<IHostApplication>();
mockHost.SetupAllProperties();
var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));

parser.Parse(new CancellationTokenSource());
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }

var qualifiedSelection = new QualifiedSelection(new QualifiedModuleName(component), selection);

//Act
var refactoring = new MoveCloserToUsageRefactoring(vbe.Object, parser.State, null);
refactoring.Refactor(qualifiedSelection);

//Assert
Assert.AreEqual(expectedCode, module.Content());
}

[TestMethod]
public void IntroduceFieldRefactoring_PassInTarget_Nonvariable()
{
Expand Down

0 comments on commit a93fce9

Please sign in to comment.