Merge pull request #2698 from comintern/next

retailcoder · web-flow · commit 1c2a2a795ba2 · 2017-02-16T22:18:15.000-05:00
Fix thread safety in SubClassWindow, Indenter fix, focus switching fixes.
diff --git a/RetailCoder.VBE/App.cs b/RetailCoder.VBE/App.cs
@@ -94,7 +94,15 @@ private void _vbe_FocusChanged(object sender, WindowChangedEventArgs e)
         {
             if (e.EventType == WindowChangedEventArgs.FocusType.GotFocus)
             {
-                RefreshSelection(e.Window);
+                switch (e.Window.Type)
+                {
+                    case WindowKind.Designer:
+                        RefreshSelection(e.Window);
+                        break;
+                    case WindowKind.CodeWindow:
+                        RefreshSelection(e.CodePane);
+                        break;
+                }              
             }          
         }
 
diff --git a/Rubberduck.Parsing/VBA/RubberduckParserState.cs b/Rubberduck.Parsing/VBA/RubberduckParserState.cs
@@ -72,7 +72,7 @@ internal void RefreshFinder(IHostApplication host)
             DeclarationFinder = new DeclarationFinder(AllDeclarations, AllAnnotations, host);
         }
 
-        private IVBE _vbe;
+        private readonly IVBE _vbe;
         public RubberduckParserState(IVBE vbe)
         {
             var values = Enum.GetValues(typeof(ParserState));
@@ -82,32 +82,27 @@ public RubberduckParserState(IVBE vbe)
             }
 
             _vbe = vbe;
-
-            if (_vbe != null && _vbe.VBProjects != null)
-            {
-                VBProjects.ProjectAdded += Sinks_ProjectAdded;
-                VBProjects.ProjectRemoved += Sinks_ProjectRemoved;
-                VBProjects.ProjectRenamed += Sinks_ProjectRenamed;
-                foreach (var project in _vbe.VBProjects.Where(proj => proj.VBComponents != null))
-                {
-                    AddComponentEventHandlers(project);
-                }
-            }
-
+            AddEventHandlers();
             IsEnabled = true;
         }
 
         #region Event Handling
 
-        private void AddComponentEventHandlers(IVBProject project)
+        private void AddEventHandlers()
         {
+            VBProjects.ProjectAdded += Sinks_ProjectAdded;
+            VBProjects.ProjectRemoved += Sinks_ProjectRemoved;
+            VBProjects.ProjectRenamed += Sinks_ProjectRenamed;
             VBComponents.ComponentAdded += Sinks_ComponentAdded;
             VBComponents.ComponentRemoved += Sinks_ComponentRemoved;
             VBComponents.ComponentRenamed += Sinks_ComponentRenamed;           
         }
 
-        private void RemoveComponentEventHandlers(IVBProject project)
+        private void RemoveEventHandlers()
         {
+            VBProjects.ProjectAdded += Sinks_ProjectAdded;
+            VBProjects.ProjectRemoved += Sinks_ProjectRemoved;
+            VBProjects.ProjectRenamed += Sinks_ProjectRenamed;
             VBComponents.ComponentAdded -= Sinks_ComponentAdded;
             VBComponents.ComponentRemoved -= Sinks_ComponentRemoved;
             VBComponents.ComponentRenamed -= Sinks_ComponentRenamed;
@@ -118,8 +113,6 @@ private void Sinks_ProjectAdded(object sender, ProjectEventArgs e)
             if (!e.Project.VBE.IsInDesignMode) { return; }
 
             Logger.Debug("Project '{0}' was added.", e.ProjectId);
-            AddComponentEventHandlers(e.Project);
-
             RefreshProjects(_vbe); // note side-effect: assigns ProjectId/HelpFile
             OnParseRequested(sender);
         }
@@ -129,8 +122,6 @@ private void Sinks_ProjectRemoved(object sender, ProjectEventArgs e)
             if (!e.Project.VBE.IsInDesignMode) { return; }
             
             Debug.Assert(e.ProjectId != null);
-            RemoveComponentEventHandlers(e.Project);
-
             RemoveProject(e.ProjectId, true);
             OnParseRequested(sender);
         }
@@ -1110,16 +1101,7 @@ public void Dispose()
                 CoClasses.Clear();
             }
 
-            if (_vbe != null && _vbe.VBProjects != null)
-            {
-                VBProjects.ProjectAdded -= Sinks_ProjectAdded;
-                VBProjects.ProjectRemoved -= Sinks_ProjectRemoved;
-                VBProjects.ProjectRenamed -= Sinks_ProjectRenamed;
-                foreach (var project in _vbe.VBProjects.Where(proj => proj.VBComponents != null))
-                {
-                    RemoveComponentEventHandlers(project);
-                }
-            }
+            RemoveEventHandlers();
 
             _moduleStates.Clear();
             _declarationSelections.Clear();
diff --git a/Rubberduck.SmartIndenter/AbsoluteCodeLine.cs b/Rubberduck.SmartIndenter/AbsoluteCodeLine.cs
@@ -10,8 +10,6 @@ namespace Rubberduck.SmartIndenter
     internal class AbsoluteCodeLine
     {
         private const string StupidLineEnding = ": _";
-        private static readonly Regex StringReplaceRegex = new Regex(StringLiteralAndBracketEscaper.StringPlaceholder.ToString(CultureInfo.InvariantCulture));
-        private static readonly Regex BracketReplaceRegex = new Regex(StringLiteralAndBracketEscaper.BracketPlaceholder.ToString(CultureInfo.InvariantCulture));
         private static readonly Regex LineNumberRegex = new Regex(@"^(?<number>(-?\d+)|(&H[0-9A-F]{1,8}))\s+(?<code>.*)", RegexOptions.ExplicitCapture);
         private static readonly Regex EndOfLineCommentRegex = new Regex(@"^(?!(Rem\s)|('))(?<code>[^']*)(\s(?<comment>'.*))$", RegexOptions.ExplicitCapture);      
         private static readonly Regex ProcedureStartRegex = new Regex(@"^(Public\s|Private\s|Friend\s)?(Static\s)?(Sub|Function|Property\s(Let|Get|Set))\s");
@@ -33,8 +31,6 @@ internal class AbsoluteCodeLine
         private readonly bool _stupidLineEnding;
         private readonly string[] _segments;
         private readonly StringLiteralAndBracketEscaper _escaper;
-        //private List<string> _strings;
-        //private List<string> _brackets;
 
         public AbsoluteCodeLine(string code, IIndenterSettings settings) : this(code, settings, null) { }
 
@@ -61,8 +57,6 @@ public AbsoluteCodeLine(string code, IIndenterSettings settings, AbsoluteCodeLin
             ExtractLineNumber();
             ExtractEndOfLineComment();
 
-            _code = Regex.Replace(_code, StringLiteralAndBracketEscaper.StringPlaceholder + "+", StringLiteralAndBracketEscaper.StringPlaceholder.ToString(CultureInfo.InvariantCulture));
-            _code = Regex.Replace(_code, StringLiteralAndBracketEscaper.BracketPlaceholder + "+", StringLiteralAndBracketEscaper.BracketPlaceholder.ToString(CultureInfo.InvariantCulture)).Trim();
             _segments = _code.Split(new[] { ": " }, StringSplitOptions.None);
         }
 
@@ -267,38 +261,29 @@ public string Indent(int indents, bool atProcStart, bool absolute = false)
             }
 
             var code = string.Join(": ", _segments);
-            if (_escaper.EscapedStrings.Any())
-            {
-                code = _escaper.EscapedStrings.Aggregate(code, (current, literal) => StringReplaceRegex.Replace(current, literal, 1));
-            }
-            if (_escaper.EscapedBrackets.Any())
-            {
-                code = _escaper.EscapedBrackets.Aggregate(code, (current, expr) => BracketReplaceRegex.Replace(current, expr, 1));
-            }
-
             code = string.Join(string.Empty, number, new string(' ', gap), code);
             if (string.IsNullOrEmpty(EndOfLineComment))
             {
-                return code + (_stupidLineEnding ? StupidLineEnding : string.Empty);
+                return _escaper.UnescapeIndented(code + (_stupidLineEnding ? StupidLineEnding : string.Empty));
             }
 
             var position = Original.LastIndexOf(EndOfLineComment, StringComparison.Ordinal);
             switch (_settings.EndOfLineCommentStyle)
             {
                 case EndOfLineCommentStyle.Absolute:
-                    return string.Format("{0}{1}{2}{3}", code, new string(' ', Math.Max(position - code.Length, 1)),
-                                                         EndOfLineComment, _stupidLineEnding ? StupidLineEnding : string.Empty);
+                    return _escaper.UnescapeIndented(string.Format("{0}{1}{2}{3}", code, new string(' ', Math.Max(position - code.Length, 1)),
+                                                     EndOfLineComment, _stupidLineEnding ? StupidLineEnding : string.Empty));
                 case EndOfLineCommentStyle.SameGap:
                     var uncommented = Original.Substring(0, position - 1);
-                    return string.Format("{0}{1}{2}{3}", code, new string(' ', uncommented.Length - uncommented.TrimEnd().Length + 1), 
-                                                         EndOfLineComment, _stupidLineEnding ? StupidLineEnding : string.Empty);
+                    return _escaper.UnescapeIndented(string.Format("{0}{1}{2}{3}", code, new string(' ', uncommented.Length - uncommented.TrimEnd().Length + 1), 
+                                                     EndOfLineComment, _stupidLineEnding ? StupidLineEnding : string.Empty));
                 case EndOfLineCommentStyle.StandardGap:
-                    return string.Format("{0}{1}{2}{3}", code, new string(' ', _settings.IndentSpaces * 2), EndOfLineComment,
-                                                        _stupidLineEnding ? StupidLineEnding : string.Empty);
+                    return _escaper.UnescapeIndented(string.Format("{0}{1}{2}{3}", code, new string(' ', _settings.IndentSpaces * 2), EndOfLineComment,
+                                                     _stupidLineEnding ? StupidLineEnding : string.Empty));
                 case EndOfLineCommentStyle.AlignInColumn:
                     var align = _settings.EndOfLineCommentColumnSpaceAlignment - code.Length;
-                    return string.Format("{0}{1}{2}{3}", code, new string(' ', Math.Max(align - 1, 1)), EndOfLineComment,
-                                                        _stupidLineEnding ? StupidLineEnding : string.Empty);
+                    return _escaper.UnescapeIndented(string.Format("{0}{1}{2}{3}", code, new string(' ', Math.Max(align - 1, 1)), EndOfLineComment,
+                                                     _stupidLineEnding ? StupidLineEnding : string.Empty));
                 default:
                     throw new InvalidEnumArgumentException();
             }
diff --git a/Rubberduck.SmartIndenter/Rubberduck.SmartIndenter.csproj b/Rubberduck.SmartIndenter/Rubberduck.SmartIndenter.csproj
@@ -56,7 +56,6 @@
     <Compile Include="IndenterSettings.cs" />
     <Compile Include="LogicalCodeLine.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
-    <Compile Include="Selection.cs" />
     <Compile Include="StringLiteralAndBracketEscaper.cs" />
   </ItemGroup>
   <ItemGroup>
diff --git a/Rubberduck.SmartIndenter/Selection.cs b/Rubberduck.SmartIndenter/Selection.cs
diff --git a/Rubberduck.SmartIndenter/StringLiteralAndBracketEscaper.cs b/Rubberduck.SmartIndenter/StringLiteralAndBracketEscaper.cs
@@ -1,12 +1,16 @@
 ﻿using System.Collections.Generic;
 using System.Linq;
+using System.Text.RegularExpressions;
 
 namespace Rubberduck.SmartIndenter
 {
     internal class StringLiteralAndBracketEscaper
     {
         public const char StringPlaceholder = '\a';
-        public const char BracketPlaceholder = '\x2';
+        public const char BracketPlaceholder = '\x02';
+
+        private static readonly Regex StringReplaceRegex = new Regex("\a+");
+        private static readonly Regex BracketReplaceRegex = new Regex("\x02+");
 
         private readonly List<string> _strings = new List<string>();
         private readonly List<string> _brackets = new List<string>();
@@ -18,6 +22,20 @@ internal class StringLiteralAndBracketEscaper
         public IEnumerable<string> EscapedStrings { get { return _strings; } }
         public IEnumerable<string> EscapedBrackets { get { return _brackets; } }
 
+        public string UnescapeIndented(string indented)
+        {
+            var code = indented;
+            if (_strings.Any())
+            {
+                code = _strings.Aggregate(code, (current, literal) => StringReplaceRegex.Replace(current, literal, 1));
+            }
+            if (_brackets.Any())
+            {
+                code = _brackets.Aggregate(code, (current, expr) => BracketReplaceRegex.Replace(current, expr, 1));
+            }
+            return code;
+        }
+
         public StringLiteralAndBracketEscaper(string code)
         {
             _unescaped = code;
@@ -63,8 +81,8 @@ public StringLiteralAndBracketEscaper(string code)
                         continue;
                     }
                     bracketed = false;
-                    _brackets.Add(new string(chars.Skip(brkpos).Take(c - brkpos).ToArray()));
-                    for (var e = brkpos; e < c; e++)
+                    _brackets.Add(new string(chars.Skip(brkpos).Take(c - brkpos + 1).ToArray()));
+                    for (var e = brkpos; e <= c; e++)
                     {
                         chars[e] = BracketPlaceholder;
                     }
diff --git a/Rubberduck.VBEEditor/Events/VBEEvents.cs b/Rubberduck.VBEEditor/Events/VBEEvents.cs
@@ -92,7 +92,9 @@ private static void AttachWindow(IntPtr hwnd)
                 Debug.Assert(!TrackedWindows.ContainsKey(hwnd));
                 var window = GetWindowFromHwnd(hwnd);
                 if (window == null) return;
-                var source = window.Type == WindowKind.CodeWindow ? new CodePaneSubclass(hwnd) as IWindowEventProvider: new DesignerWindowSubclass(hwnd);
+                var source = window.Type == WindowKind.CodeWindow
+                    ? new CodePaneSubclass(hwnd, GetCodePaneFromHwnd(hwnd)) as IWindowEventProvider
+                    : new DesignerWindowSubclass(hwnd);
                 var info = new WindowInfo(hwnd, window, source);
                 source.FocusChange += FocusDispatcher;
                 TrackedWindows.Add(hwnd, info);
diff --git a/Rubberduck.VBEEditor/Events/WindowChangedEventArgs.cs b/Rubberduck.VBEEditor/Events/WindowChangedEventArgs.cs
@@ -13,12 +13,14 @@ public enum FocusType
 
         public IntPtr Hwnd { get; private set; }
         public IWindow Window { get; private set; }
+        public ICodePane CodePane { get; private set; }
         public FocusType EventType { get; private set; }
 
-        public WindowChangedEventArgs(IntPtr hwnd, IWindow window, FocusType type)
+        public WindowChangedEventArgs(IntPtr hwnd, IWindow window, ICodePane pane, FocusType type)
         {
             Hwnd = hwnd;
             Window = window;
+            CodePane = pane;
             EventType = type;
         }
     }
diff --git a/Rubberduck.VBEEditor/WindowsApi/CodePaneSubclass.cs b/Rubberduck.VBEEditor/WindowsApi/CodePaneSubclass.cs
@@ -1,10 +1,29 @@
 ﻿using System;
+using Rubberduck.VBEditor.Events;
+using Rubberduck.VBEditor.SafeComWrappers.Abstract;
 
 namespace Rubberduck.VBEditor.WindowsApi
 {
+    //Stub for code pane replacement.  :-)
     internal class CodePaneSubclass : FocusSource
     {
-        //Stub for code pane replacement.  :-)
-        internal CodePaneSubclass(IntPtr hwnd) : base(hwnd) { }
+        private readonly ICodePane _pane;
+
+        public ICodePane CodePane { get { return _pane; } }
+
+        internal CodePaneSubclass(IntPtr hwnd, ICodePane pane) : base(hwnd)
+        {
+            _pane = pane;
+        }
+
+        protected override void DispatchFocusEvent(WindowChangedEventArgs.FocusType type)
+        {
+            var window = VBEEvents.GetWindowInfoFromHwnd(Hwnd);
+            if (window == null)
+            {
+                return;
+            }
+            OnFocusChange(new WindowChangedEventArgs(window.Value.Hwnd, window.Value.Window, _pane, type));
+        }
     }
 }
diff --git a/Rubberduck.VBEEditor/WindowsApi/FocusSource.cs b/Rubberduck.VBEEditor/WindowsApi/FocusSource.cs
@@ -9,28 +9,34 @@ internal abstract class FocusSource : SubclassingWindow, IWindowEventProvider
         protected FocusSource(IntPtr hwnd) : base(hwnd, hwnd) { }
 
         public event EventHandler<WindowChangedEventArgs> FocusChange;
-        private void OnFocusChange(WindowChangedEventArgs.FocusType type)
+        protected void OnFocusChange(WindowChangedEventArgs eventArgs)
         {
             if (FocusChange != null)
             {
-                var window = VBEEvents.GetWindowInfoFromHwnd(Hwnd);
-                if (window == null)
-                {
-                    return;
-                }
-                FocusChange.Invoke(this, new WindowChangedEventArgs(window.Value.Hwnd, window.Value.Window, type));
+                FocusChange.Invoke(this, eventArgs);
             }
-        } 
+        }
+
+        protected virtual void DispatchFocusEvent(WindowChangedEventArgs.FocusType type)
+        {
+            var window = VBEEvents.GetWindowInfoFromHwnd(Hwnd);
+            if (window == null)
+            {
+                return;
+            }
+            OnFocusChange(new WindowChangedEventArgs(Hwnd, window.Value.Window, null, type));
+        }
 
         public override int SubClassProc(IntPtr hWnd, IntPtr msg, IntPtr wParam, IntPtr lParam, IntPtr uIdSubclass, IntPtr dwRefData)
         {
             switch ((uint)msg)
             {
                 case (uint)WM.SETFOCUS:
-                    OnFocusChange(WindowChangedEventArgs.FocusType.GotFocus);
+
+                    DispatchFocusEvent(WindowChangedEventArgs.FocusType.GotFocus);
                     break;
                 case (uint)WM.KILLFOCUS:
-                    OnFocusChange(WindowChangedEventArgs.FocusType.LostFocus);
+                    DispatchFocusEvent(WindowChangedEventArgs.FocusType.LostFocus);
                     break;
             }
             return base.SubClassProc(hWnd, msg, wParam, lParam, uIdSubclass, dwRefData);
diff --git a/Rubberduck.VBEEditor/WindowsApi/SubclassingWindow.cs b/Rubberduck.VBEEditor/WindowsApi/SubclassingWindow.cs
@@ -13,8 +13,7 @@ public abstract class SubclassingWindow : IDisposable
         private readonly SubClassCallback _wndProc;
         private bool _listening;
 
-        private static readonly ConcurrentBag<SubClassCallback> RubberduckProcs = new ConcurrentBag<SubClassCallback>();
-        private static readonly object SubclassLock = new object();
+        private readonly object _subclassLock = new object();
 
         public delegate int SubClassCallback(IntPtr hWnd, IntPtr msg, IntPtr wParam, IntPtr lParam, IntPtr uIdSubclass, IntPtr dwRefData);
 
@@ -40,10 +39,6 @@ protected SubclassingWindow(IntPtr subclassId, IntPtr hWnd)
             _wndProc = SubClassProc;
             AssignHandle();
         }
-        ~SubclassingWindow()
-        {
-            Debug.Assert(false, "Dispose() not called.");
-        }
 
         public void Dispose()
         {
@@ -53,9 +48,8 @@ public void Dispose()
 
         private void AssignHandle()
         {
-            lock (SubclassLock)
+            lock (_subclassLock)
             {
-                RubberduckProcs.Add(_wndProc);
                 var result = SetWindowSubclass(_hwnd, _wndProc, _subclassId, IntPtr.Zero);
                 if (result != 1)
                 {
@@ -67,13 +61,13 @@ private void AssignHandle()
 
         private void ReleaseHandle()
         {
-            if (!_listening)
+            lock (_subclassLock)
             {
-                return;
-            }
+                if (!_listening)
+                {
+                    return;
+                }
 
-            lock (SubclassLock)
-            {
                 var result = RemoveWindowSubclass(_hwnd, _wndProc, _subclassId);
                 if (result != 1)
                 {
diff --git a/RubberduckTests/SmartIndenter/MiscAndCornerCaseTests.cs b/RubberduckTests/SmartIndenter/MiscAndCornerCaseTests.cs

Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,15 @@ private void _vbe_FocusChanged(object sender, WindowChangedEventArgs e)`
`94`	`94`	`{`
`95`	`95`	`if (e.EventType == WindowChangedEventArgs.FocusType.GotFocus)`
`96`	`96`	`{`
`97`		`- RefreshSelection(e.Window);`
	`97`	`+ switch (e.Window.Type)`
	`98`	`+ {`
	`99`	`+ case WindowKind.Designer:`
	`100`	`+ RefreshSelection(e.Window);`
	`101`	`+ break;`
	`102`	`+ case WindowKind.CodeWindow:`
	`103`	`+ RefreshSelection(e.CodePane);`
	`104`	`+ break;`
	`105`	`+ }`
`98`	`106`	`}`
`99`	`107`	`}`
`100`	`108`
Original file line number	Diff line number	Diff line change
`@@ -13,12 +13,14 @@ public enum FocusType`
`13`	`13`
`14`	`14`	`public IntPtr Hwnd { get; private set; }`
`15`	`15`	`public IWindow Window { get; private set; }`
	`16`	`+ public ICodePane CodePane { get; private set; }`
`16`	`17`	`public FocusType EventType { get; private set; }`
`17`	`18`
`18`		`- public WindowChangedEventArgs(IntPtr hwnd, IWindow window, FocusType type)`
	`19`	`+ public WindowChangedEventArgs(IntPtr hwnd, IWindow window, ICodePane pane, FocusType type)`
`19`	`20`	`{`
`20`	`21`	`Hwnd = hwnd;`
`21`	`22`	`Window = window;`
	`23`	`+ CodePane = pane;`
`22`	`24`	`EventType = type;`
`23`	`25`	`}`
`24`	`26`	`}`