From 58197da5bb36b94fcbd2ec3ef5865b1e50b3f1e7 Mon Sep 17 00:00:00 2001 From: Vogel612 Date: Thu, 29 Dec 2016 02:16:26 +0100 Subject: [PATCH 1/9] implement result description for AggregateInspectionResult removed attempt at quickfixing AggregateInspectionResult adds a format-string to display the inspection meta and result count for the overview --- .../Resources/InspectionsUI.Designer.cs | 9 +++++ .../Inspections/Resources/InspectionsUI.resx | 4 +++ .../Results/AggregateInspectionResult.cs | 36 +++++++++++++++++++ RetailCoder.VBE/Rubberduck.csproj | 13 +++---- 4 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs index c280ba5b72..21a3c5bca8 100644 --- a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs +++ b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs @@ -60,6 +60,15 @@ internal InspectionsUI() { } } + /// + /// Looks up a localized string similar to Warning {0}: {1} results. + /// + public static string AggregateInspectionResultFormat { + get { + return ResourceManager.GetString("AggregateInspectionResultFormat", resourceCulture); + } + } + /// /// Looks up a localized string similar to Parameter is passed by value, but is assigned a new value/reference. Consider making a local copy instead if the caller isn't supposed to know the new value. If the caller should see the new value, the parameter should be passed ByRef instead, and you have a bug.. /// diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx index 03a1a6b185..442838eabe 100644 --- a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx +++ b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx @@ -602,4 +602,8 @@ If the parameter can be null, ignore this inspection result; passing a null valu Assignment to '{0}' implicitly assigns default member of class '{1}' + + Warning {0}: {1} results + {0} inpection meta, {1} result count + \ No newline at end of file diff --git a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs new file mode 100644 index 0000000000..5db676b82b --- /dev/null +++ b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs @@ -0,0 +1,36 @@ +using Rubberduck.Inspections.Abstract; +using Rubberduck.Inspections.Resources; +using Rubberduck.VBEditor; +using System; +using System.Collections.Generic; + +namespace Rubberduck.Inspections.Results +{ + class AggregateInspectionResult: InspectionResultBase + { + private readonly IEnumerable _quickFixes; + private readonly List _results; + + public AggregateInspectionResult(List encapsulatedResults) + : base(null, new QualifiedModuleName(), null) + { + if (encapsulatedResults.Count == 0) + { + throw new InvalidOperationException("Cannot aggregate results without results, wtf?"); + } + _results = encapsulatedResults; + _quickFixes = new QuickFixBase[]{}; + // If there were any reasonable way to provide this mess with a fix, we'd put it here + } + + public override string Description + { + get + { + return string.Format(InspectionsUI.AggregateInspectionResultFormat, _results[0].Inspection.Meta, _results.Count); + } + } + + public override IEnumerable QuickFixes { get { return _quickFixes; } } + } +} diff --git a/RetailCoder.VBE/Rubberduck.csproj b/RetailCoder.VBE/Rubberduck.csproj index 361dae4af7..406b3c85ac 100644 --- a/RetailCoder.VBE/Rubberduck.csproj +++ b/RetailCoder.VBE/Rubberduck.csproj @@ -366,6 +366,12 @@ + + True + True + InspectionsUI.resx + + @@ -519,11 +525,6 @@ - - True - True - InspectionsUI.resx - @@ -1038,8 +1039,8 @@ PublicResXFileCodeGenerator - InspectionsUI.Designer.cs Designer + InspectionsUI.Designer.cs ResXFileCodeGenerator From 93969fecc606b4e482f6242abc2500029134e2f1 Mon Sep 17 00:00:00 2001 From: Vogel612 Date: Sat, 7 Jan 2017 23:11:09 +0100 Subject: [PATCH 2/9] aggregate results by type before returning from Inspector --- .../Inspections/Concrete/Inspector.cs | 10 +++- .../Results/AggregateInspectionResult.cs | 50 +++++++++++++++++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/RetailCoder.VBE/Inspections/Concrete/Inspector.cs b/RetailCoder.VBE/Inspections/Concrete/Inspector.cs index 90f3dae403..1b65210720 100644 --- a/RetailCoder.VBE/Inspections/Concrete/Inspector.cs +++ b/RetailCoder.VBE/Inspections/Concrete/Inspector.cs @@ -13,6 +13,7 @@ using Rubberduck.Parsing.VBA; using Rubberduck.Settings; using Rubberduck.UI; +using Rubberduck.Inspections.Results; namespace Rubberduck.Inspections.Concrete { @@ -22,6 +23,7 @@ public class Inspector : IInspector { private readonly IGeneralConfigService _configService; private readonly List _inspections; + private readonly int AGGREGATION_THRESHOLD = 128; public Inspector(IGeneralConfigService configService, IEnumerable inspections) { @@ -94,7 +96,13 @@ public async Task> FindIssuesAsync(RubberduckPars LogManager.GetCurrentClassLogger().Error(e); } state.OnStatusMessageUpdate(RubberduckUI.ResourceManager.GetString("ParserState_" + state.Status, UI.Settings.Settings.Culture)); // should be "Ready" - return allIssues; + + var issuesByType = allIssues.GroupBy(issue => issue.GetType()).ToDictionary(grouping => grouping.Key, grouping => grouping.ToList()); + return issuesByType.Where(kv => kv.Value.Count <= AGGREGATION_THRESHOLD) + .SelectMany(kv => kv.Value) + .Union(issuesByType.Where(kv => kv.Value.Count > AGGREGATION_THRESHOLD) + .Select(kv => new AggregateInspectionResult(kv.Value))); + //return allIssues; } private IReadOnlyList GetParseTreeResults(Configuration config, RubberduckParserState state) diff --git a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs index 5db676b82b..7f800cd45d 100644 --- a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs @@ -6,13 +6,12 @@ namespace Rubberduck.Inspections.Results { - class AggregateInspectionResult: InspectionResultBase + class AggregateInspectionResult: IInspectionResult { private readonly IEnumerable _quickFixes; private readonly List _results; - public AggregateInspectionResult(List encapsulatedResults) - : base(null, new QualifiedModuleName(), null) + public AggregateInspectionResult(List encapsulatedResults) { if (encapsulatedResults.Count == 0) { @@ -23,7 +22,9 @@ public AggregateInspectionResult(List encapsulatedResults) // If there were any reasonable way to provide this mess with a fix, we'd put it here } - public override string Description + //public override IEnumerable QuickFixes { get { return _quickFixes; } } + + public string Description { get { @@ -31,6 +32,45 @@ public override string Description } } - public override IEnumerable QuickFixes { get { return _quickFixes; } } + public IInspection Inspection + { + get + { + return _results[0].Inspection; + } + } + + public QualifiedSelection QualifiedSelection + { + get + { + return _results[0].QualifiedSelection; + } + } + + public IEnumerable QuickFixes + { + get + { + throw new NotImplementedException(); + } + } + + public int CompareTo(object obj) + { + throw new NotImplementedException(); + } + + public int CompareTo(IInspectionResult other) + { + throw new NotImplementedException(); + } + + public object[] ToArray() + { + throw new NotImplementedException(); + } + + } } From 9113a06812a5cd8d3b7c66106c853746e7008b84 Mon Sep 17 00:00:00 2001 From: Vogel612 Date: Sat, 7 Jan 2017 23:32:17 +0100 Subject: [PATCH 3/9] secure _projects and fix ghost component detection _projects is non-secured Dictionary. We need to secure it against race-conditions. All access should be locked or explain why it's not. Also fixed a bug with ghost component detection that appeared when an uneven number of items were pretending to be one project --- .../VBA/RubberduckParserState.cs | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/Rubberduck.Parsing/VBA/RubberduckParserState.cs b/Rubberduck.Parsing/VBA/RubberduckParserState.cs index 3fa5c7daf6..c70012eab0 100644 --- a/Rubberduck.Parsing/VBA/RubberduckParserState.cs +++ b/Rubberduck.Parsing/VBA/RubberduckParserState.cs @@ -239,9 +239,12 @@ public void RefreshProjects(IVBE vbe) private void RemoveProject(string projectId, bool notifyStateChanged = false) { - if (_projects.ContainsKey(projectId)) + lock (_projects) { - _projects.Remove(projectId); + if (_projects.ContainsKey(projectId)) + { + _projects.Remove(projectId); + } } ClearStateCache(projectId, notifyStateChanged); @@ -251,7 +254,10 @@ public List Projects { get { - return new List(_projects.Values); + lock(_projects) + { + return new List(_projects.Values); + } } } @@ -303,11 +309,20 @@ public void SetModuleState(IVBComponent component, ParserState state, SyntaxErro var projectId = component.Collection.Parent.HelpFile; IVBProject project = null; - foreach (var item in _projects) + lock (_projects) { - if (item.Value.HelpFile == projectId) + foreach (var item in _projects) { - project = project != null ? null : item.Value; + if (item.Value.HelpFile == projectId) + { + if (project != null) + { + // ghost component detected, abort project iteration + project = null; + break; + } + project = item.Value; + } } } @@ -1077,6 +1092,7 @@ public void Dispose() _moduleStates.Clear(); _declarationSelections.Clear(); + // no lock because nobody should try to update anything here _projects.Clear(); _isDisposed = true; From 66fb27d1783cb60f611b928225904659664305b2 Mon Sep 17 00:00:00 2001 From: Vogel612 Date: Sun, 8 Jan 2017 02:25:31 +0100 Subject: [PATCH 4/9] clear development debris --- .../Results/AggregateInspectionResult.cs | 44 ++++++++++++++----- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs index 7f800cd45d..8bd138de38 100644 --- a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs @@ -3,12 +3,12 @@ using Rubberduck.VBEditor; using System; using System.Collections.Generic; +using System.Linq; namespace Rubberduck.Inspections.Results { class AggregateInspectionResult: IInspectionResult { - private readonly IEnumerable _quickFixes; private readonly List _results; public AggregateInspectionResult(List encapsulatedResults) @@ -18,17 +18,14 @@ public AggregateInspectionResult(List encapsulatedResults) throw new InvalidOperationException("Cannot aggregate results without results, wtf?"); } _results = encapsulatedResults; - _quickFixes = new QuickFixBase[]{}; - // If there were any reasonable way to provide this mess with a fix, we'd put it here } - - //public override IEnumerable QuickFixes { get { return _quickFixes; } } + public string Description { get { - return string.Format(InspectionsUI.AggregateInspectionResultFormat, _results[0].Inspection.Meta, _results.Count); + return string.Format(InspectionsUI.AggregateInspectionResultFormat, _results[0].Inspection.Description, _results.Count); } } @@ -52,25 +49,48 @@ public IEnumerable QuickFixes { get { - throw new NotImplementedException(); + return Enumerable.Empty(); } } public int CompareTo(object obj) { - throw new NotImplementedException(); + if (obj == this) + { + return 0; + } + IInspectionResult result = obj as IInspectionResult; + return result == null ? -1 : CompareTo(result); } public int CompareTo(IInspectionResult other) { - throw new NotImplementedException(); + if (other == this) + { + return 0; + } + AggregateInspectionResult result = other as AggregateInspectionResult; + if (result == null) + { + return -1; + } + if (_results.Count != result._results.Count) { + return _results.Count - result._results.Count; + } + for (int i = 0; i < _results.Count; i++) + { + if (_results[i].CompareTo(result._results[i]) != 0) + { + return _results[i].CompareTo(result._results[i]); + } + } + return 0; } public object[] ToArray() { - throw new NotImplementedException(); + var module = QualifiedSelection.QualifiedName; + return new object[] { Inspection.Severity.ToString(), module.ProjectName, module.ComponentName, Description, QualifiedSelection.Selection.StartLine, QualifiedSelection.Selection.StartColumn }; } - - } } From b3e33aa4cef3f7ecddcac5821f592dad186726dd Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Mon, 9 Jan 2017 23:27:40 -0500 Subject: [PATCH 5/9] made AggregateInspectionResult inherit InspectionResultBase --- .../Abstract/InspectionResultBase.cs | 6 +- .../Results/AggregateInspectionResult.cs | 64 ++++++------------- 2 files changed, 21 insertions(+), 49 deletions(-) diff --git a/RetailCoder.VBE/Inspections/Abstract/InspectionResultBase.cs b/RetailCoder.VBE/Inspections/Abstract/InspectionResultBase.cs index e3f9e16394..24772a4fe7 100644 --- a/RetailCoder.VBE/Inspections/Abstract/InspectionResultBase.cs +++ b/RetailCoder.VBE/Inspections/Abstract/InspectionResultBase.cs @@ -68,7 +68,7 @@ protected InspectionResultBase(IInspection inspection, QualifiedModuleName quali /// /// Gets the information needed to select the target instruction in the VBE. /// - public QualifiedSelection QualifiedSelection + public virtual QualifiedSelection QualifiedSelection { get { @@ -85,13 +85,13 @@ public QualifiedSelection QualifiedSelection /// /// Gets all available "quick fixes" for a code inspection result. /// - public virtual IEnumerable QuickFixes { get { return new QuickFixBase[] {}; } } + public virtual IEnumerable QuickFixes { get { return Enumerable.Empty(); } } public bool HasQuickFixes { get { return QuickFixes.Any(); } } public virtual QuickFixBase DefaultQuickFix { get { return QuickFixes.FirstOrDefault(); } } - public int CompareTo(IInspectionResult other) + public virtual int CompareTo(IInspectionResult other) { return Inspection.CompareTo(other.Inspection); } diff --git a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs index 8bd138de38..1604acbc3b 100644 --- a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs @@ -4,72 +4,50 @@ using System; using System.Collections.Generic; using System.Linq; +using Antlr4.Runtime; +using Rubberduck.Parsing.Symbols; namespace Rubberduck.Inspections.Results { - class AggregateInspectionResult: IInspectionResult + class AggregateInspectionResult: InspectionResultBase { private readonly List _results; + private readonly IEnumerable _quickFixes; + private readonly IInspectionResult _result; - public AggregateInspectionResult(List encapsulatedResults) + public AggregateInspectionResult(List results, IEnumerable quickFixes = null) + : base(results[0].Inspection, results[0].QualifiedSelection.QualifiedName, ParserRuleContext.EmptyContext) { - if (encapsulatedResults.Count == 0) - { - throw new InvalidOperationException("Cannot aggregate results without results, wtf?"); - } - _results = encapsulatedResults; + _results = results; + _result = results[0]; + _quickFixes = quickFixes ?? Enumerable.Empty(); } - - public string Description + public override string Description { get { - return string.Format(InspectionsUI.AggregateInspectionResultFormat, _results[0].Inspection.Description, _results.Count); + return string.Format(InspectionsUI.AggregateInspectionResultFormat, _result.Inspection.Description, _results.Count); } } - public IInspection Inspection + public override QualifiedSelection QualifiedSelection { get { - return _results[0].Inspection; + return _result.QualifiedSelection; } } - public QualifiedSelection QualifiedSelection - { - get - { - return _results[0].QualifiedSelection; - } - } + public override IEnumerable QuickFixes { get { return _quickFixes; } } - public IEnumerable QuickFixes - { - get - { - return Enumerable.Empty(); - } - } - - public int CompareTo(object obj) - { - if (obj == this) - { - return 0; - } - IInspectionResult result = obj as IInspectionResult; - return result == null ? -1 : CompareTo(result); - } - - public int CompareTo(IInspectionResult other) + public override int CompareTo(IInspectionResult other) { if (other == this) { return 0; } - AggregateInspectionResult result = other as AggregateInspectionResult; + var result = other as AggregateInspectionResult; if (result == null) { return -1; @@ -77,7 +55,7 @@ public int CompareTo(IInspectionResult other) if (_results.Count != result._results.Count) { return _results.Count - result._results.Count; } - for (int i = 0; i < _results.Count; i++) + for (var i = 0; i < _results.Count; i++) { if (_results[i].CompareTo(result._results[i]) != 0) { @@ -86,11 +64,5 @@ public int CompareTo(IInspectionResult other) } return 0; } - - public object[] ToArray() - { - var module = QualifiedSelection.QualifiedName; - return new object[] { Inspection.Severity.ToString(), module.ProjectName, module.ComponentName, Description, QualifiedSelection.Selection.StartLine, QualifiedSelection.Selection.StartColumn }; - } } } From 959d637f78bb6dcf4be07a14c1047bed9cc7266e Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Mon, 9 Jan 2017 23:28:04 -0500 Subject: [PATCH 6/9] added AggregateInspectionResult --- .../Inspections/Resources/InspectionsUI.Designer.cs | 2 +- .../Inspections/Resources/InspectionsUI.de.resx | 9 ++++++--- .../Inspections/Resources/InspectionsUI.fr.resx | 3 +++ RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx | 4 ++-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs index 21a3c5bca8..5ba2fee303 100644 --- a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs +++ b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.Designer.cs @@ -61,7 +61,7 @@ internal InspectionsUI() { } /// - /// Looks up a localized string similar to Warning {0}: {1} results. + /// Looks up a localized string similar to {0} ({1} results). /// public static string AggregateInspectionResultFormat { get { diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.de.resx b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.de.resx index 45e714c169..2e169d28a1 100644 --- a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.de.resx +++ b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.de.resx @@ -568,7 +568,7 @@ Falls der Parameter 'null' sein kann, bitte dieses Auftreten ignorieren. 'null' Code, der undeklarierte Variablen verwendet, kompiliert nicht wenn 'Option Explicit' spezifiziert wird. Undeklarierte Variablen sind immer vom Typ 'Variant', was unnötige Zusatzkosten in Ausführungszeit und Speicherverbauch verursacht. - Add property get + Addieren 'Property Get' Das Schlüsselwort 'Public' kann nur auf Modulebene verwendet werden; Sein Konterpart 'Private' kann auch nur auf Modulebene verwendet werden. 'Dim' jedoch kann verwendet werden, um sowohl modulweite als auch prozedurweite Variablen zu deklarieren. Um der Konsistenz Willen ist es besser, 'Dim' nur für lokale Variablen zu verwenden, also 'Private' statt 'Dim' auf Modulebene zu verwenden. @@ -588,7 +588,10 @@ Falls der Parameter 'null' sein kann, bitte dieses Auftreten ignorieren. 'null' Die Modulvariable '{0}' ist mit dem 'Dim'-Schlüsselwort deklariert. - + Ein Member ist als Funktion geschrieben, aber wird wie eine Prozedur verwendet. Falls die Funktion nicht rekursiv ist, sollten Sie in Erwägung ziehen, die 'Function' in ein 'Sub' zu konvertieren. Falls die Funktion rekursiv ist, verwendet keiner der externen Aufrufer den Rückgabewert. - + + {0} ({1} Ergebnisse) + + \ No newline at end of file diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.fr.resx b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.fr.resx index 8ff6827d1a..14de829bee 100644 --- a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.fr.resx +++ b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.fr.resx @@ -601,4 +601,7 @@ Si le paramètre peut être nul, ignorer ce résultat; passer une valeur nulle Assignation de '{0}' réfère implicitement au membre par défaut de la classe '{1}' + + {0} ({1} résultats) + \ No newline at end of file diff --git a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx index 442838eabe..7de01c9287 100644 --- a/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx +++ b/RetailCoder.VBE/Inspections/Resources/InspectionsUI.resx @@ -603,7 +603,7 @@ If the parameter can be null, ignore this inspection result; passing a null valu Assignment to '{0}' implicitly assigns default member of class '{1}' - Warning {0}: {1} results - {0} inpection meta, {1} result count + {0} ({1} results) + {0} inpection description, {1} result count \ No newline at end of file From 4e8b7cf06bab1fbd64f6073ac3abb9f7093fb4a3 Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Mon, 9 Jan 2017 23:46:07 -0500 Subject: [PATCH 7/9] removed aggregate quickfixes --- .../Inspections/Concrete/Inspector.cs | 3 ++- .../Results/AggregateInspectionResult.cs | 21 +++++++------------ 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/RetailCoder.VBE/Inspections/Concrete/Inspector.cs b/RetailCoder.VBE/Inspections/Concrete/Inspector.cs index 1b65210720..174582b42f 100644 --- a/RetailCoder.VBE/Inspections/Concrete/Inspector.cs +++ b/RetailCoder.VBE/Inspections/Concrete/Inspector.cs @@ -97,7 +97,8 @@ public async Task> FindIssuesAsync(RubberduckPars } state.OnStatusMessageUpdate(RubberduckUI.ResourceManager.GetString("ParserState_" + state.Status, UI.Settings.Settings.Culture)); // should be "Ready" - var issuesByType = allIssues.GroupBy(issue => issue.GetType()).ToDictionary(grouping => grouping.Key, grouping => grouping.ToList()); + var issuesByType = allIssues.GroupBy(issue => issue.GetType()) + .ToDictionary(grouping => grouping.Key, grouping => grouping.ToList()); return issuesByType.Where(kv => kv.Value.Count <= AGGREGATION_THRESHOLD) .SelectMany(kv => kv.Value) .Union(issuesByType.Where(kv => kv.Value.Count > AGGREGATION_THRESHOLD) diff --git a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs index 1604acbc3b..50339a35dc 100644 --- a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs @@ -1,26 +1,21 @@ using Rubberduck.Inspections.Abstract; using Rubberduck.Inspections.Resources; using Rubberduck.VBEditor; -using System; using System.Collections.Generic; -using System.Linq; using Antlr4.Runtime; -using Rubberduck.Parsing.Symbols; namespace Rubberduck.Inspections.Results { class AggregateInspectionResult: InspectionResultBase { private readonly List _results; - private readonly IEnumerable _quickFixes; private readonly IInspectionResult _result; - public AggregateInspectionResult(List results, IEnumerable quickFixes = null) + public AggregateInspectionResult(List results) : base(results[0].Inspection, results[0].QualifiedSelection.QualifiedName, ParserRuleContext.EmptyContext) { _results = results; _result = results[0]; - _quickFixes = quickFixes ?? Enumerable.Empty(); } public override string Description @@ -39,27 +34,25 @@ public override QualifiedSelection QualifiedSelection } } - public override IEnumerable QuickFixes { get { return _quickFixes; } } - public override int CompareTo(IInspectionResult other) { if (other == this) { return 0; } - var result = other as AggregateInspectionResult; - if (result == null) + var aggregated = other as AggregateInspectionResult; + if (aggregated == null) { return -1; } - if (_results.Count != result._results.Count) { - return _results.Count - result._results.Count; + if (_results.Count != aggregated._results.Count) { + return _results.Count - aggregated._results.Count; } for (var i = 0; i < _results.Count; i++) { - if (_results[i].CompareTo(result._results[i]) != 0) + if (_results[i].CompareTo(aggregated._results[i]) != 0) { - return _results[i].CompareTo(result._results[i]); + return _results[i].CompareTo(aggregated._results[i]); } } return 0; From 0e6b56a12c18223de4acbae406a060238db32913 Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Mon, 9 Jan 2017 23:50:16 -0500 Subject: [PATCH 8/9] added default quickfix --- .../Inspections/Results/AggregateInspectionResult.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs index 50339a35dc..3ed9e82e6b 100644 --- a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs @@ -2,6 +2,7 @@ using Rubberduck.Inspections.Resources; using Rubberduck.VBEditor; using System.Collections.Generic; +using System.Linq; using Antlr4.Runtime; namespace Rubberduck.Inspections.Results @@ -34,6 +35,8 @@ public override QualifiedSelection QualifiedSelection } } + public override QuickFixBase DefaultQuickFix { get { return _result.QuickFixes == null ? null : _result.QuickFixes.FirstOrDefault(); } } + public override int CompareTo(IInspectionResult other) { if (other == this) From c9e5128a42ac5992b6611c0719d3959111dee2b1 Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Tue, 10 Jan 2017 01:00:54 -0500 Subject: [PATCH 9/9] made fix work for same-line declarations --- .../Inspections/Concrete/Inspector.cs | 2 +- .../DeclareAsExplicitVariantQuickFix.cs | 23 +++++++++++-------- .../Results/AggregateInspectionResult.cs | 7 ++++++ .../Inspections/InspectionResultsViewModel.cs | 15 ++++++++---- Rubberduck.VBEEditor/QualifiedSelection.cs | 16 +++++++++++-- Rubberduck.VBEEditor/Selection.cs | 16 ++++++++++++- 6 files changed, 61 insertions(+), 18 deletions(-) diff --git a/RetailCoder.VBE/Inspections/Concrete/Inspector.cs b/RetailCoder.VBE/Inspections/Concrete/Inspector.cs index 174582b42f..2945a5191b 100644 --- a/RetailCoder.VBE/Inspections/Concrete/Inspector.cs +++ b/RetailCoder.VBE/Inspections/Concrete/Inspector.cs @@ -102,7 +102,7 @@ public async Task> FindIssuesAsync(RubberduckPars return issuesByType.Where(kv => kv.Value.Count <= AGGREGATION_THRESHOLD) .SelectMany(kv => kv.Value) .Union(issuesByType.Where(kv => kv.Value.Count > AGGREGATION_THRESHOLD) - .Select(kv => new AggregateInspectionResult(kv.Value))); + .Select(kv => new AggregateInspectionResult(kv.Value.OrderBy(i => i.QualifiedSelection).ToList()))); //return allIssues; } diff --git a/RetailCoder.VBE/Inspections/QuickFixes/DeclareAsExplicitVariantQuickFix.cs b/RetailCoder.VBE/Inspections/QuickFixes/DeclareAsExplicitVariantQuickFix.cs index 2e4b60325b..eeb40f057c 100644 --- a/RetailCoder.VBE/Inspections/QuickFixes/DeclareAsExplicitVariantQuickFix.cs +++ b/RetailCoder.VBE/Inspections/QuickFixes/DeclareAsExplicitVariantQuickFix.cs @@ -1,3 +1,4 @@ +using System; using System.Linq; using Antlr4.Runtime; using Rubberduck.Inspections.Abstract; @@ -25,7 +26,7 @@ public override void Fix() // DeclareExplicitVariant() overloads return empty string if context is null Selection selection; - var fix = DeclareExplicitVariant(Context as VBAParser.VariableSubStmtContext, out originalInstruction, out selection); + var fix = DeclareExplicitVariant(Context as VBAParser.VariableSubStmtContext, contextLines, out originalInstruction, out selection); if (!string.IsNullOrEmpty(fix)) { // maintain original indentation for a variable declaration @@ -34,7 +35,7 @@ public override void Fix() if (string.IsNullOrEmpty(originalInstruction)) { - fix = DeclareExplicitVariant(Context as VBAParser.ConstSubStmtContext, out originalInstruction, out selection); + fix = DeclareExplicitVariant(Context as VBAParser.ConstSubStmtContext, contextLines, out originalInstruction, out selection); } if (string.IsNullOrEmpty(originalInstruction)) @@ -100,7 +101,7 @@ private string DeclareExplicitVariant(VBAParser.ArgContext context, out string i return fix; } - private string DeclareExplicitVariant(VBAParser.VariableSubStmtContext context, out string instruction, out Selection selection) + private string DeclareExplicitVariant(VBAParser.VariableSubStmtContext context, string contextLines, out string instruction, out Selection selection) { if (context == null) { @@ -110,17 +111,19 @@ private string DeclareExplicitVariant(VBAParser.VariableSubStmtContext context, } var parent = (ParserRuleContext)context.Parent.Parent; - instruction = parent.GetText(); selection = parent.GetSelection(); + instruction = contextLines.Substring(selection.StartColumn - 1); var variable = context.GetText(); var replacement = context.identifier().GetText() + ' ' + Tokens.As + ' ' + Tokens.Variant; - var result = instruction.Replace(variable, replacement); + var insertIndex = instruction.IndexOf(variable, StringComparison.Ordinal); + var result = instruction.Substring(0, insertIndex) + + replacement + instruction.Substring(insertIndex + variable.Length); return result; } - private string DeclareExplicitVariant(VBAParser.ConstSubStmtContext context, out string instruction, out Selection selection) + private string DeclareExplicitVariant(VBAParser.ConstSubStmtContext context, string contextLines, out string instruction, out Selection selection) { if (context == null) { @@ -130,8 +133,8 @@ private string DeclareExplicitVariant(VBAParser.ConstSubStmtContext context, out } var parent = (ParserRuleContext)context.Parent; - instruction = parent.GetText(); - selection = parent.GetSelection(); + selection = parent.GetSelection(); + instruction = contextLines.Substring(selection.StartColumn - 1); var constant = context.GetText(); var replacement = context.identifier().GetText() + ' ' @@ -139,7 +142,9 @@ private string DeclareExplicitVariant(VBAParser.ConstSubStmtContext context, out + context.EQ().GetText() + ' ' + context.expression().GetText(); - var result = instruction.Replace(constant, replacement); + var insertIndex = instruction.IndexOf(constant, StringComparison.Ordinal); + var result = instruction.Substring(0, insertIndex) + + replacement + instruction.Substring(insertIndex + constant.Length); return result; } } diff --git a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs index 3ed9e82e6b..308ec87317 100644 --- a/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs +++ b/RetailCoder.VBE/Inspections/Results/AggregateInspectionResult.cs @@ -18,6 +18,8 @@ public AggregateInspectionResult(List results) _results = results; _result = results[0]; } + + public IReadOnlyList IndividualResults { get { return _results; } } public override string Description { @@ -35,6 +37,11 @@ public override QualifiedSelection QualifiedSelection } } + public override IEnumerable QuickFixes + { + get { return _result.QuickFixes == null ? base.QuickFixes : new[] { _result.QuickFixes.FirstOrDefault() }; } + } + public override QuickFixBase DefaultQuickFix { get { return _result.QuickFixes == null ? null : _result.QuickFixes.FirstOrDefault(); } } public override int CompareTo(IInspectionResult other) diff --git a/RetailCoder.VBE/UI/Inspections/InspectionResultsViewModel.cs b/RetailCoder.VBE/UI/Inspections/InspectionResultsViewModel.cs index 085c2cf8f1..01209364bb 100644 --- a/RetailCoder.VBE/UI/Inspections/InspectionResultsViewModel.cs +++ b/RetailCoder.VBE/UI/Inspections/InspectionResultsViewModel.cs @@ -12,6 +12,7 @@ using Rubberduck.Common; using Rubberduck.Inspections.Abstract; using Rubberduck.Inspections.Resources; +using Rubberduck.Inspections.Results; using Rubberduck.Parsing.VBA; using Rubberduck.Settings; using Rubberduck.UI.Command; @@ -353,12 +354,16 @@ private void ExecuteQuickFixInModuleCommand(object parameter) return; } - var items = _results.Where(result => result.Inspection == SelectedInspection - && result.QualifiedSelection.QualifiedName == selectedResult.QualifiedSelection.QualifiedName) - .Select(item => item.QuickFixes.Single(fix => fix.GetType() == _defaultFix.GetType())) - .OrderByDescending(item => item.Selection.Selection.EndLine) - .ThenByDescending(item => item.Selection.Selection.EndColumn); + var filteredResults = _results + .Where(result => result.Inspection == SelectedInspection + && result.QualifiedSelection.QualifiedName == selectedResult.QualifiedSelection.QualifiedName) + .ToList(); + var items = filteredResults.Where(result => !(result is AggregateInspectionResult)) + .Select(item => item.QuickFixes.Single(fix => fix.GetType() == _defaultFix.GetType())) + .Union(filteredResults.OfType() + .SelectMany(aggregate => aggregate.IndividualResults.Select(result => result.QuickFixes.Single(fix => fix.GetType() == _defaultFix.GetType())))) + .OrderByDescending(fix => fix.Selection); ExecuteQuickFixes(items); } diff --git a/Rubberduck.VBEEditor/QualifiedSelection.cs b/Rubberduck.VBEEditor/QualifiedSelection.cs index a25fb7218b..fab6782687 100644 --- a/Rubberduck.VBEEditor/QualifiedSelection.cs +++ b/Rubberduck.VBEEditor/QualifiedSelection.cs @@ -1,6 +1,8 @@ -namespace Rubberduck.VBEditor +using System; + +namespace Rubberduck.VBEditor { - public struct QualifiedSelection + public struct QualifiedSelection : IComparable { public QualifiedSelection(QualifiedModuleName qualifiedName, Selection selection) { @@ -14,6 +16,16 @@ public QualifiedSelection(QualifiedModuleName qualifiedName, Selection selection private readonly Selection _selection; public Selection Selection { get { return _selection; } } + public int CompareTo(QualifiedSelection other) + { + if (other.QualifiedName != QualifiedName) + { + return string.Compare(QualifiedName.ToString(), other.QualifiedName.ToString(), StringComparison.Ordinal); + } + + return Selection.CompareTo(other.Selection); + } + public override string ToString() { return string.Concat(QualifiedName, " ", Selection); diff --git a/Rubberduck.VBEEditor/Selection.cs b/Rubberduck.VBEEditor/Selection.cs index f761ca0235..188112c2ca 100644 --- a/Rubberduck.VBEEditor/Selection.cs +++ b/Rubberduck.VBEEditor/Selection.cs @@ -2,7 +2,7 @@ namespace Rubberduck.VBEditor { - public struct Selection : IEquatable + public struct Selection : IEquatable, IComparable { public Selection(int startLine, int startColumn, int endLine, int endColumn) { @@ -88,6 +88,20 @@ public bool Equals(Selection other) && other.EndColumn == EndColumn; } + public int CompareTo(Selection other) + { + if (this > other) + { + return 1; + } + if (this < other) + { + return -1; + } + + return 0; + } + public override string ToString() { return (_startLine == _endLine && _startColumn == _endColumn)