From 58197da5bb36b94fcbd2ec3ef5865b1e50b3f1e7 Mon Sep 17 00:00:00 2001 From: Vogel612 Date: Thu, 29 Dec 2016 02:16:26 +0100 Subject: [PATCH 01/14] 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 b2f1679a0961f696aa72dd35aee25f3e443d9a80 Mon Sep 17 00:00:00 2001 From: comintern Date: Sat, 7 Jan 2017 10:35:11 -0600 Subject: [PATCH 02/14] Allow Excel tests in project with names containing spaces and single quotes. --- Rubberduck.VBEEditor/Application/ExcelApp.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/Rubberduck.VBEEditor/Application/ExcelApp.cs b/Rubberduck.VBEEditor/Application/ExcelApp.cs index 376d10ccde..b6e8ae7411 100644 --- a/Rubberduck.VBEEditor/Application/ExcelApp.cs +++ b/Rubberduck.VBEEditor/Application/ExcelApp.cs @@ -1,4 +1,7 @@ -using Rubberduck.VBEditor.SafeComWrappers.Abstract; +using Path = System.IO.Path; +using System.Runtime.InteropServices; +using Microsoft.Office.Interop.Visio; +using Rubberduck.VBEditor.SafeComWrappers.Abstract; namespace Rubberduck.VBEditor.Application { @@ -9,14 +12,19 @@ public ExcelApp(IVBE vbe) : base(vbe, "Excel") { } public override void Run(QualifiedMemberName qualifiedMemberName) { - string call = GenerateMethodCall(qualifiedMemberName); + var call = GenerateMethodCall(qualifiedMemberName); Application.Run(call); } - protected virtual string GenerateMethodCall(QualifiedMemberName qualifiedMemberName) + protected virtual string GenerateMethodCall(QualifiedMemberName qualifiedMemberName) { - var documentName = qualifiedMemberName.QualifiedModuleName.ProjectDisplayName; - return string.Concat(documentName, "!", qualifiedMemberName.ToString()); + var module = qualifiedMemberName.QualifiedModuleName; + + var documentName = string.IsNullOrEmpty(module.Project.FileName) + ? module.ProjectDisplayName + : Path.GetFileName(module.Project.FileName); + + return string.Format("'{0}'!{1}", documentName.Replace("'", "''"), qualifiedMemberName); } } } From 65f7bf6add6249443eca76416ab5cd53d174e458 Mon Sep 17 00:00:00 2001 From: comintern Date: Sat, 7 Jan 2017 11:47:29 -0600 Subject: [PATCH 03/14] Fixed project name to support project names with spaces or dashes. --- Rubberduck.VBEEditor/QualifiedModuleName.cs | 25 ++++++++++++--------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/Rubberduck.VBEEditor/QualifiedModuleName.cs b/Rubberduck.VBEEditor/QualifiedModuleName.cs index df9ba85fa7..7b0fa5b3da 100644 --- a/Rubberduck.VBEEditor/QualifiedModuleName.cs +++ b/Rubberduck.VBEEditor/QualifiedModuleName.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Text.RegularExpressions; using Rubberduck.VBEditor.SafeComWrappers.Abstract; namespace Rubberduck.VBEditor @@ -122,6 +123,9 @@ public QualifiedMemberName QualifyMemberName(string member) private readonly string _projectPath; public string ProjectPath { get { return _projectPath; } } + private static readonly Regex CaptionProjectRegex = new Regex(@"^(?:[^-]+)(?:\s-\s)(?.+)(?:\s-\s.*)?$"); + private static readonly Regex OpenModuleRegex = new Regex(@"^(?.+)(?\s-\s\[.*\((Code|UserForm)\)\])$"); + // because this causes a flicker in the VBE, we only want to do it once. // we also want to defer it as long as possible because it is only // needed in a couple places, and QualifiedModuleName is used in many places. @@ -146,17 +150,18 @@ public string ProjectDisplayName vbe.ActiveVBProject = _project; } - var windowCaptionElements = mainWindow.Caption.Split(' ').ToList(); - // without an active code pane: {"Microsoft", "Visual", "Basic", "for", "Applications", "-", "Book2"} - // with an active code pane: {"Microsoft", "Visual", "Basic", "for", "Applications", "-", "Book2", "-", "[Thisworkbook", "(Code)]"} - // so we need to index of the first "-" element; the display name is the element next to that. - _projectDisplayName = windowCaptionElements[windowCaptionElements.IndexOf("-") + 1]; - return _projectDisplayName; - } - catch - { - return string.Empty; + var caption = mainWindow.Caption; + if (CaptionProjectRegex.IsMatch(caption)) + { + caption = CaptionProjectRegex.Matches(caption)[0].Groups["project"].Value; + _projectDisplayName = OpenModuleRegex.IsMatch(caption) + ? OpenModuleRegex.Matches(caption)[0].Groups["project"].Value + : caption; + } } + // ReSharper disable once EmptyGeneralCatchClause + catch { } + return _projectDisplayName; } } } From 93969fecc606b4e482f6242abc2500029134e2f1 Mon Sep 17 00:00:00 2001 From: Vogel612 Date: Sat, 7 Jan 2017 23:11:09 +0100 Subject: [PATCH 04/14] 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 05/14] 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 06/14] 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 0e0f2b1b78d11564f96dff01b9c5711826403ea4 Mon Sep 17 00:00:00 2001 From: comintern Date: Sat, 7 Jan 2017 21:47:41 -0600 Subject: [PATCH 07/14] Validate log level in general settings. --- RetailCoder.VBE/Common/LogLevelHelper.cs | 13 ++++++++++- RetailCoder.VBE/Settings/GeneralSettings.cs | 26 +++++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/RetailCoder.VBE/Common/LogLevelHelper.cs b/RetailCoder.VBE/Common/LogLevelHelper.cs index d60497aaa7..6636318814 100644 --- a/RetailCoder.VBE/Common/LogLevelHelper.cs +++ b/RetailCoder.VBE/Common/LogLevelHelper.cs @@ -1,4 +1,5 @@ -using NLog; +using System.Linq; +using NLog; using NLog.Config; using System; using System.Collections.Generic; @@ -28,6 +29,16 @@ private static IEnumerable GetLogLevels() return logLevels; } + public static int MinLogLevel() + { + return GetLogLevels().Min(lvl => lvl.Ordinal); + } + + public static int MaxLogLevel() + { + return GetLogLevels().Max(lvl => lvl.Ordinal); + } + public static void SetMinimumLogLevel(LogLevel minimumLogLevel) { var loggingRules = LogManager.Configuration.LoggingRules; diff --git a/RetailCoder.VBE/Settings/GeneralSettings.cs b/RetailCoder.VBE/Settings/GeneralSettings.cs index 463db58024..74032f9c55 100644 --- a/RetailCoder.VBE/Settings/GeneralSettings.cs +++ b/RetailCoder.VBE/Settings/GeneralSettings.cs @@ -1,5 +1,7 @@ -using NLog; +using System; +using NLog; using System.Xml.Serialization; +using Rubberduck.Common; namespace Rubberduck.Settings { @@ -23,7 +25,27 @@ public class GeneralSettings : IGeneralSettings public bool AutoSaveEnabled { get; set; } public int AutoSavePeriod { get; set; } public char Delimiter { get; set; } - public int MinimumLogLevel { get; set; } + + private int _logLevel; + public int MinimumLogLevel + { + get { return _logLevel; } + set + { + if (value < LogLevelHelper.MinLogLevel()) + { + _logLevel = LogLevelHelper.MinLogLevel(); + } + else if (value > LogLevelHelper.MaxLogLevel()) + { + _logLevel = LogLevelHelper.MaxLogLevel(); + } + else + { + _logLevel = value; + } + } + } public GeneralSettings() { From b3e33aa4cef3f7ecddcac5821f592dad186726dd Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Mon, 9 Jan 2017 23:27:40 -0500 Subject: [PATCH 08/14] 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 09/14] 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 10/14] 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 11/14] 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 12/14] 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) From c6a8e68d07827ee58170181ebebb1a2e832f5a28 Mon Sep 17 00:00:00 2001 From: ThunderFrame Date: Tue, 10 Jan 2017 23:05:24 +1100 Subject: [PATCH 13/14] Removes Illegal Characters when exporting components In theory there could be file-name collisions using this approach. Might be better to save with a unique hash for the name? --- Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponent.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponent.cs b/Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponent.cs index 6b4957ba73..04a73d38ff 100644 --- a/Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponent.cs +++ b/Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponent.cs @@ -56,6 +56,10 @@ public string Name set { Target.Name = value; } } + private string SafeName + { + } + public IControls Controls { get @@ -107,7 +111,7 @@ public void Export(string path) /// Destination folder for the resulting source file. public string ExportAsSourceFile(string folder) { - var fullPath = Path.Combine(folder, Name + Type.FileExtension()); + var fullPath = Path.Combine(folder, SafeName + Type.FileExtension()); switch (Type) { case ComponentType.UserForm: @@ -165,7 +169,7 @@ private void ExportDocumentModule(string path) private string ExportToTempFile() { - var path = Path.Combine(Path.GetTempPath(), Name + Type.FileExtension()); + var path = Path.Combine(Path.GetTempPath(), SafeName + Type.FileExtension()); Export(path); return path; } From a8e68f0375c9682a6a99fd3add8959f0b7d8df13 Mon Sep 17 00:00:00 2001 From: ThunderFrame Date: Tue, 10 Jan 2017 23:06:46 +1100 Subject: [PATCH 14/14] Safe File Name substition to underscore --- Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponent.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponent.cs b/Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponent.cs index 04a73d38ff..18b98bb0d9 100644 --- a/Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponent.cs +++ b/Rubberduck.VBEEditor/SafeComWrappers/VBA/VBComponent.cs @@ -58,6 +58,7 @@ public string Name private string SafeName { + get { return Path.GetInvalidFileNameChars().Aggregate(Name, (current, c) => current.Replace(c.ToString(), "_")); } } public IControls Controls