Move RegexSearchAndReplace awai from FindTarget and make it a bit more sane

MDoerner · MDoerner · commit 507f48b7f8f6 · 2019-10-15T00:01:44.000+02:00
This fixes the method to get the surrounding block in SearchInCurrentBlock and generally fixes how searching within a given selection works. Instead of filtering the results for the entire module, the search is performed only inside the selection. This makes a difference on the start and end line.

Moreover, this commits adds fixmes for the broken handling of code modules, whose root cause is saving the modules on the results. This should never be done.
diff --git a/Rubberduck.Core/Navigation/RegexSearchReplace/RegexSearchReplace.cs b/Rubberduck.Core/Navigation/RegexSearchReplace/RegexSearchReplace.cs
@@ -2,10 +2,8 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Text.RegularExpressions;
-using Antlr4.Runtime;
-using Rubberduck.Common;
 using Rubberduck.Parsing;
-using Rubberduck.Parsing.Symbols;
+using Rubberduck.Parsing.Grammar;
 using Rubberduck.Parsing.VBA;
 using Rubberduck.VBEditor;
 using Rubberduck.VBEditor.SafeComWrappers;
@@ -17,14 +15,12 @@ namespace Rubberduck.Navigation.RegexSearchReplace
     public class RegexSearchReplace : IRegexSearchReplace
     {
         private readonly IVBE _vbe;
-        private readonly IDeclarationFinderProvider _declarationFinderProvider;
         private readonly ISelectionService _selectionService;
         private readonly ISelectedDeclarationProvider _selectedDeclarationProvider;
 
-        public RegexSearchReplace(IVBE vbe, IDeclarationFinderProvider declarationFinderProvider, ISelectionService selectionService, ISelectedDeclarationProvider selectedDeclarationProvider)
+        public RegexSearchReplace(IVBE vbe, ISelectionService selectionService, ISelectedDeclarationProvider selectedDeclarationProvider)
         {
             _vbe = vbe;
-            _declarationFinderProvider = declarationFinderProvider;
             _selectionService = selectionService;
             _selectedDeclarationProvider = selectedDeclarationProvider;
             _search = new Dictionary<RegexSearchReplaceScope, Func<string, IEnumerable<RegexSearchResult>>>
@@ -84,22 +80,80 @@ private IEnumerable<RegexSearchResult> GetResultsFromModule(ICodeModule module,
             // VBA uses 1-based indexing
             for (var i = 1; i <= module.CountOfLines; i++)
             {
-                var matches =
-                    Regex.Matches(module.GetLines(i, 1), searchPattern)
-                        .OfType<Match>()
+                var codeLine = module.GetLines(i, 1);
+                var matches = LineMatches(codeLine, searchPattern)
                         .Select(m => new RegexSearchResult(m, module, i));
 
                 results.AddRange(matches);
             }
             return results;
         }
 
+        private IEnumerable<Match> LineMatches(string line, string searchPattern)
+        {
+            return Regex.Matches(line, searchPattern)
+                .OfType<Match>();
+        }
+
+        private IEnumerable<Match> LineMatches(string line, int startColumn, int? endColumn, string searchPattern)
+        {
+            var shortenedLine = endColumn.HasValue 
+                ? line.Substring(startColumn - 1, endColumn.Value - startColumn + 1)
+                : line.Substring(startColumn - 1);
+            return LineMatches(shortenedLine, searchPattern);
+        }
+
+        private IEnumerable<RegexSearchResult> GetResultsFromModule(ICodeModule module, string searchPattern, Selection selection)
+        {
+            var startLine = selection.StartLine > 1
+                ? selection.StartLine
+                : 1;
+
+            var moduleLines = module.CountOfLines;
+            var stopLine = selection.EndLine < moduleLines
+                ? selection.EndLine
+                : moduleLines;
+
+            if (startLine > stopLine)
+            {
+                return new List<RegexSearchResult>();
+            }
+
+            if (startLine == stopLine)
+            {
+                return LineMatches(module.GetLines(startLine, 1), selection.StartColumn, null, searchPattern)
+                    .Select(m => new RegexSearchResult(m, module, startLine, selection.StartColumn - 1))
+                    .ToList();
+            }
+
+            var results = new List<RegexSearchResult>();
+
+            var firstLineMatches = LineMatches(module.GetLines(startLine, 1), selection.StartColumn, selection.EndColumn, searchPattern)
+                .Select(m => new RegexSearchResult(m, module, startLine));
+            results.AddRange(firstLineMatches);
+
+            for (var lineIndex = startLine + 1; lineIndex < stopLine; lineIndex++)
+            {
+                var codeLine = module.GetLines(lineIndex, 1);
+                var matches = LineMatches(codeLine, searchPattern)
+                    .Select(m => new RegexSearchResult(m, module, lineIndex));
+
+                results.AddRange(matches);
+            }
+
+            var lastLineMatches = LineMatches(module.GetLines(stopLine, 1), 1, selection.EndColumn, searchPattern)
+                .Select(m => new RegexSearchResult(m, module, stopLine));
+            results.AddRange(lastLineMatches);
+
+            return results;
+        }
+
         private void SetSelection(RegexSearchResult item)
         {
             _selectionService.TrySetActiveSelection(item.Module.QualifiedModuleName, item.Selection);
         }
 
-        private List<RegexSearchResult> SearchSelection(string searchPattern)
+        private IEnumerable<RegexSearchResult> SearchSelection(string searchPattern)
         {
             using (var pane = _vbe.ActiveCodePane)
             {
@@ -110,23 +164,30 @@ private List<RegexSearchResult> SearchSelection(string searchPattern)
 
                 using (var module = pane.CodeModule)
                 {
-                    var results = GetResultsFromModule(module, searchPattern);
-                    return results.Where(r => pane.Selection.Contains(r.Selection)).ToList();
+                    return GetResultsFromModule(module, searchPattern, pane.Selection);
                 }
             }
         }
 
-        private List<RegexSearchResult> SearchCurrentBlock(string searchPattern)
+        private IEnumerable<RegexSearchResult> SearchCurrentBlock(string searchPattern)
         {
-            var declarationTypes = new[]
-                    {
-                        DeclarationType.Event,
-                        DeclarationType.Function,
-                        DeclarationType.Procedure,
-                        DeclarationType.PropertyGet,
-                        DeclarationType.PropertyLet,
-                        DeclarationType.PropertySet
-                    };
+            var activeSelection = _selectionService.ActiveSelection();
+            if (!activeSelection.HasValue)
+            {
+                return new List<RegexSearchResult>();
+            }
+            
+            var block = _selectedDeclarationProvider
+                .SelectedMember(activeSelection.Value)
+                ?.Context
+                .GetSmallestDescendentContainingSelection<VBAParser.BlockContext>(activeSelection.Value.Selection);
+
+            if (block == null)
+            {
+                return new List<RegexSearchResult>();
+            }
+
+            var blockSelection = block.GetSelection();
 
             using (var pane = _vbe.ActiveCodePane)
             {
@@ -137,24 +198,8 @@ private List<RegexSearchResult> SearchCurrentBlock(string searchPattern)
 
                 using (var module = pane.CodeModule)
                 {
-                    var results = GetResultsFromModule(module, searchPattern);
-
-                    var qualifiedSelection = pane.GetQualifiedSelection();
-
-                    if (!qualifiedSelection.HasValue)
-                    {
-                        return new List<RegexSearchResult>();
-                    }
-
-                    var block = (ParserRuleContext)_declarationFinderProvider.DeclarationFinder
-                        .AllDeclarations
-                        .FindTarget(qualifiedSelection.Value, declarationTypes)
-                        .Context
-                        .Parent;
-                    var selection = new Selection(block.Start.Line, block.Start.Column, block.Stop.Line,
-                        block.Stop.Column);
-
-                    return results.Where(r => selection.Contains(r.Selection)).ToList();
+                    //FIXME: This is a catastrophe waiting to happen since the module, which will get disposed, is saved on the result.
+                    return GetResultsFromModule(module, searchPattern, blockSelection);
                 }
             }
         }
diff --git a/Rubberduck.Core/Navigation/RegexSearchReplace/RegexSearchResult.cs b/Rubberduck.Core/Navigation/RegexSearchReplace/RegexSearchResult.cs
@@ -7,15 +7,17 @@ namespace Rubberduck.Navigation.RegexSearchReplace
     public class RegexSearchResult
     {
         public Match Match { get; }
+
+        //FIXME: We should not save COM wrappers anywhere.
         public ICodeModule Module { get; }
         public Selection Selection { get; }
         public string DisplayString => Match.Value;
 
-        public RegexSearchResult(Match match, ICodeModule module, int line)
+        public RegexSearchResult(Match match, ICodeModule module, int line, int columnOffset = 0)
         {
             Match = match;
             Module = module;
-            Selection = new Selection(line, match.Index + 1, line, match.Index + match.Length + 1); // adjust columns for VBE 1-based indexing
+            Selection = new Selection(line, match.Index + columnOffset + 1, line, match.Index + match.Length + columnOffset  + 1); // adjust columns for VBE 1-based indexing
         }
     }
 }
diff --git a/Rubberduck.Parsing/ParserRuleContextExtensions.cs b/Rubberduck.Parsing/ParserRuleContextExtensions.cs
@@ -288,14 +288,22 @@ public static bool IsOptionCompareBinary(this ParserRuleContext context)
             return (optionContext is null) || !(optionContext.BINARY() is null);
         }
 
-        /// Returns the context's first descendent of the generic type containing the token with the specified token index.
+        /// Returns the context's widest descendent of the generic type containing the token with the specified token index.
         /// </summary>
-        public static TContext GetDescendentContainingTokenIndex<TContext>(this ParserRuleContext context, int tokenIndex) where TContext : ParserRuleContext
+        public static TContext GetWidestDescendentContainingTokenIndex<TContext>(this ParserRuleContext context, int tokenIndex) where TContext : ParserRuleContext
         {
             var descendents = GetDescendentsContainingTokenIndex<TContext>(context, tokenIndex);
             return descendents.FirstOrDefault();
         }
 
+        /// Returns the context's smallest descendent of the generic type containing the token with the specified token index.
+        /// </summary>
+        public static TContext GetSmallestDescendentContainingTokenIndex<TContext>(this ParserRuleContext context, int tokenIndex) where TContext : ParserRuleContext
+        {
+            var descendents = GetDescendentsContainingTokenIndex<TContext>(context, tokenIndex);
+            return descendents.LastOrDefault();
+        }
+
         /// <summary>
         /// Returns all the context's descendents of the generic type containing the token with the specified token index.
         /// If there are multiple matches, they are ordered from outermost to innermost context.
@@ -325,6 +333,51 @@ public static IEnumerable<TContext> GetDescendentsContainingTokenIndex<TContext>
             return matches;
         }
 
+        /// Returns the context's widest descendent of the generic type containing the specified selection.
+        /// </summary>
+        public static TContext GetWidestDescendentContainingSelection<TContext>(this ParserRuleContext context, Selection selection) where TContext : ParserRuleContext
+        {
+            var descendents = GetDescendentsContainingSelection<TContext>(context, selection);
+            return descendents.FirstOrDefault();
+        }
+
+        /// Returns the context's smallest descendent of the generic type containing the specified selection.
+        /// </summary>
+        public static TContext GetSmallestDescendentContainingSelection<TContext>(this ParserRuleContext context, Selection selection) where TContext : ParserRuleContext
+        {
+            var descendents = GetDescendentsContainingSelection<TContext>(context, selection);
+            return descendents.LastOrDefault();
+        }
+
+        /// <summary>
+        /// Returns all the context's descendents of the generic type containing the specified selection.
+        /// If there are multiple matches, they are ordered from outermost to innermost context.
+        /// </summary>
+        public static IEnumerable<TContext> GetDescendentsContainingSelection<TContext>(this ParserRuleContext context, Selection selection) where TContext : ParserRuleContext
+        {
+            if (context == null || !context.GetSelection().Contains(selection))
+            {
+                return new List<TContext>();
+            }
+
+            var matches = new List<TContext>();
+            if (context is TContext match)
+            {
+                matches.Add(match);
+            }
+
+            foreach (var child in context.children)
+            {
+                if (child is ParserRuleContext childContext && childContext.GetSelection().Contains(selection))
+                {
+                    matches.AddRange(childContext.GetDescendentsContainingSelection<TContext>(selection));
+                    break;  //Only one child can contain the selection.
+                }
+            }
+
+            return matches;
+        }
+
         /// <summary>
         /// Returns the context containing the token preceding the context provided it is of the specified generic type.
         /// </summary>
@@ -344,7 +397,7 @@ public static bool TryGetPrecedingContext<TContext>(this ParserRuleContext conte
                 return false;
             }
 
-            precedingContext = ancestorContainingPrecedingIndex.GetDescendentContainingTokenIndex<TContext>(precedingTokenIndex);
+            precedingContext = ancestorContainingPrecedingIndex.GetWidestDescendentContainingTokenIndex<TContext>(precedingTokenIndex);
             return precedingContext != null;
         }
 
@@ -367,7 +420,7 @@ public static bool TryGetFollowingContext<TContext>(this ParserRuleContext conte
                 return false;
             }
 
-            followingContext = ancestorContainingFollowingIndex.GetDescendentContainingTokenIndex<TContext>(followingTokenIndex);
+            followingContext = ancestorContainingFollowingIndex.GetWidestDescendentContainingTokenIndex<TContext>(followingTokenIndex);
             return followingContext != null;
         }
 
diff --git a/Rubberduck.Parsing/Symbols/DeclarationEnumerableExtensions.cs b/Rubberduck.Parsing/Symbols/DeclarationEnumerableExtensions.cs
@@ -0,0 +1,24 @@
+﻿using System.Collections.Generic;
+using System.Linq;
+
+namespace Rubberduck.Parsing.Symbols
+{
+    public static class DeclarationEnumerableExtensions
+    {
+        /// <summary>
+        /// Gets all declarations of the specified <see cref="DeclarationType"/>.
+        /// </summary>
+        public static IEnumerable<Declaration> OfType(this IEnumerable<Declaration> declarations, DeclarationType declarationType)
+        {
+            return declarations.Where(declaration => declaration.DeclarationType.HasFlag(declarationType));
+        }
+
+        /// <summary>
+        /// Gets the declaration for all identifiers declared in or below the specified scope.
+        /// </summary>
+        public static IEnumerable<Declaration> InScope(this IEnumerable<Declaration> declarations, Declaration parent)
+        {
+            return declarations.Where(declaration => declaration.ParentScope == parent.Scope);
+        }
+    }
+}
diff --git a/Rubberduck.Refactorings/Common/DeclarationExtensions.cs b/Rubberduck.Refactorings/Common/DeclarationExtensions.cs

Original file line number	Diff line number	Diff line change
`@@ -7,15 +7,17 @@ namespace Rubberduck.Navigation.RegexSearchReplace`
`7`	`7`	`public class RegexSearchResult`
`8`	`8`	`{`
`9`	`9`	`public Match Match { get; }`
	`10`	`+`
	`11`	`+ //FIXME: We should not save COM wrappers anywhere.`
`10`	`12`	`public ICodeModule Module { get; }`
`11`	`13`	`public Selection Selection { get; }`
`12`	`14`	`public string DisplayString => Match.Value;`
`13`	`15`
`14`		`- public RegexSearchResult(Match match, ICodeModule module, int line)`
	`16`	`+ public RegexSearchResult(Match match, ICodeModule module, int line, int columnOffset = 0)`
`15`	`17`	`{`
`16`	`18`	`Match = match;`
`17`	`19`	`Module = module;`
`18`		`- Selection = new Selection(line, match.Index + 1, line, match.Index + match.Length + 1); // adjust columns for VBE 1-based indexing`
	`20`	`+ Selection = new Selection(line, match.Index + columnOffset + 1, line, match.Index + match.Length + columnOffset + 1); // adjust columns for VBE 1-based indexing`
`19`	`21`	`}`
`20`	`22`	`}`
`21`	`23`	`}`