From b11ed83b7184e33488200f34a1b567773b32a00b Mon Sep 17 00:00:00 2001 From: comintern Date: Tue, 28 Feb 2017 18:09:24 -0600 Subject: [PATCH 1/2] Fix handling of statement separators in MoveCloserToUsageRefactoring, cover with tests. --- .../MoveCloserToUsageRefactoring.cs | 17 ++- RetailCoder.VBE/UI/SelectionChangeService.cs | 9 +- .../Refactoring/MoveCloserToUsageTests.cs | 100 ++++++++++++++++++ 3 files changed, 117 insertions(+), 9 deletions(-) diff --git a/RetailCoder.VBE/Refactorings/MoveCloserToUsage/MoveCloserToUsageRefactoring.cs b/RetailCoder.VBE/Refactorings/MoveCloserToUsage/MoveCloserToUsageRefactoring.cs index 13608e67a2..8bbd016880 100644 --- a/RetailCoder.VBE/Refactorings/MoveCloserToUsage/MoveCloserToUsageRefactoring.cs +++ b/RetailCoder.VBE/Refactorings/MoveCloserToUsage/MoveCloserToUsageRefactoring.cs @@ -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; @@ -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] != ' ' @@ -185,7 +187,7 @@ private void InsertDeclaration() .Remove(lastIndexOfColon, numberOfCharsToRemove) .Insert(lastIndexOfColon, Environment.NewLine); - lastIndexOfColon = newLinesWithoutStringLiterals.LastIndexOf(':'); + lastIndexOfColon = GetIndexOfLastStatementSeparator(newLinesWithoutStringLiterals); } module.DeleteLines(beginningOfInstructionSelection.StartLine, beginningOfInstructionSelection.LineCount); @@ -193,6 +195,13 @@ private void InsertDeclaration() } } + 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; @@ -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(" _")) @@ -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); } } diff --git a/RetailCoder.VBE/UI/SelectionChangeService.cs b/RetailCoder.VBE/UI/SelectionChangeService.cs index 50f45fd5cf..df4d85fd92 100644 --- a/RetailCoder.VBE/UI/SelectionChangeService.cs +++ b/RetailCoder.VBE/UI/SelectionChangeService.cs @@ -81,8 +81,7 @@ private void DispatchSelectionChanged(DeclarationChangedEventArgs eventArgs) } SelectionChanged.Invoke(null, eventArgs); } - - + private void DispatchSelectedDeclaration(DeclarationChangedEventArgs eventArgs) { DispatchSelectionChanged(eventArgs); @@ -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)); @@ -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)); @@ -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) && diff --git a/RubberduckTests/Refactoring/MoveCloserToUsageTests.cs b/RubberduckTests/Refactoring/MoveCloserToUsageTests.cs index 855a72f713..aa5d3c5cf2 100644 --- a/RubberduckTests/Refactoring/MoveCloserToUsageTests.cs +++ b/RubberduckTests/Refactoring/MoveCloserToUsageTests.cs @@ -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(); + 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(); + 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() { From 50ec8a5b69c0c137bfafff2b3f7ee2632d700150 Mon Sep 17 00:00:00 2001 From: comintern Date: Tue, 28 Feb 2017 19:50:20 -0600 Subject: [PATCH 2/2] Fix null checking in focus events. --- Rubberduck.VBEEditor/WindowsApi/CodePaneSubclass.cs | 2 +- Rubberduck.VBEEditor/WindowsApi/FocusSource.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Rubberduck.VBEEditor/WindowsApi/CodePaneSubclass.cs b/Rubberduck.VBEEditor/WindowsApi/CodePaneSubclass.cs index 46bb11e003..398d8845c8 100644 --- a/Rubberduck.VBEEditor/WindowsApi/CodePaneSubclass.cs +++ b/Rubberduck.VBEEditor/WindowsApi/CodePaneSubclass.cs @@ -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; } diff --git a/Rubberduck.VBEEditor/WindowsApi/FocusSource.cs b/Rubberduck.VBEEditor/WindowsApi/FocusSource.cs index 9b4e1f35f4..ef73d800cd 100644 --- a/Rubberduck.VBEEditor/WindowsApi/FocusSource.cs +++ b/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 { @@ -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; }