Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix statement separator handling in MoveCloserToUsageRefactoring. Closes #2753 #2758

Merged
merged 4 commits into from Mar 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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