Code metrics moving and redesign #3934

Vogel612 · 2018-04-14T21:10:51Z

The following things are done:

Move Rubberduck.Inspections to Rubberduck.CodeAnalysis
Move CodeMetrics from Rubberduck.Core into Rubberduck.CodeAnalysis
Overhaul the design for CodeMetrics' internal workings
Reimplement the existing CodeMetrics with the new design (CC, LineCount and Nesting done)

TBD:

UI. A metric ton of UI work is necessary here. As of now the UI is broken, help wanted.

See #3862 for a bit of background, as well as #403 and #3522's review comments for why this is done

bclothier

LGTM; two questions.

bclothier · 2018-04-16T03:09:02Z

Rubberduck.CodeAnalysis/CodeMetrics/CodeMetricBase.cs

+{
+    internal abstract class CodeMetricResultBase : ICodeMetricResult
+    {
+        public CodeMetricResultBase(Declaration declaration, CodeMetric metric)


isn't it weird to have a internal class that contains a public ctor?

bclothier · 2018-04-16T03:13:16Z

Rubberduck.CodeAnalysis/CodeMetrics/LineCountMetric.cs

+
+        public override IEnumerable<ICodeMetricResult> Results() => new[] { new LineCountModuleMetricResult(_finder.ModuleDeclaration(_qmn), ownerReference, workingValue) };
+
+        public override void EnterEndOfLine([NotNull]  VBAParser.EndOfLineContext _) => workingValue++;


Do we wanna to count blank and comment lines? Or better, enhance to return a struct with counts of each types Declarations, Blank, Comment line, and Module body line with a total?

hmm ... the current design doesn't really support anything that closely coupled and rich as result type.
Most notably it can only really represent strings properly.

Each of these can be spawning their own result type though ...

Vogel612 · 2018-06-04T20:19:51Z

Rubberduck.Main/Root/RubberduckIoCInstaller.cs

+                    {
+                        try
+                        {
+                            return (CodeMetric)Activator.CreateInstance(t, new object[] { });


I kinda want a cleaner way than instantiating these at registration time

Why do you not just register all the code metrics with CodeMetric? Then the collection of all of them will be registered with IEnumerable<CodeMetric>.

I tried that before, it didn't work :/

Vogel612 · 2018-06-04T20:20:10Z

Rubberduck.Main/Root/RubberduckIoCInstaller.cs

+            }
+            container.Register(Component.For<ICodeMetricsAnalyst>()
+                .ImplementedBy<CodeMetricsAnalyst>()
+                .DependsOn(Dependency.OnValue<IEnumerable<CodeMetric>>(metricImplementations))


a rare case of explicit dependency configuration. Is that okay?

Also move a ton of other stuff around and cowboy a design for the CodeMetrics overhaul

This implements each CodeMetric in it's own separate class-collection and ParseTreeListener For now only Cyclomatic Complexity is implemented as a Proof Of Concept

…ies into baseclass

…ser with using-block

this should allow implementation of a proper UI. Do note that the interface of the VM is basically guesswork to how this works best.

…d ClassId on IDockableUserControl

…ovide results

codecov · 2018-06-05T20:52:48Z

Codecov Report

Merging #3934 into next will decrease coverage by 0.16%.
The diff coverage is 38.64%.

@@            Coverage Diff            @@
##             next   #3934      +/-   ##
=========================================
- Coverage   52.96%   52.8%   -0.16%     
=========================================
  Files         955     960       +5     
  Lines       32838   32960     +122     
=========================================
+ Hits        17390   17403      +13     
- Misses      15448   15557     +109

Impacted Files	Coverage Δ
...UnreachableCaseInspection/ParseTreeValueFactory.cs	`100% <ø> (ø)`
...ons/Results/IdentifierReferenceInspectionResult.cs	`100% <ø> (ø)`
...crete/ImplicitDefaultMemberAssignmentInspection.cs	`5.56% <ø> (ø)`
...is/Inspections/Abstract/ParseTreeInspectionBase.cs	`100% <ø> (ø)`
...Concrete/ApplicationWorksheetFunctionInspection.cs	`100% <ø> (ø)`
...QuickFixes/SpecifyExplicitByRefModifierQuickFix.cs	`87.5% <ø> (ø)`
...ections/Concrete/ObjectVariableNotSetInspection.cs	`100% <ø> (ø)`
...lysis/QuickFixes/IntroduceLocalVariableQuickFix.cs	`0% <ø> (ø)`
...ections/Concrete/ImplicitPublicMemberInspection.cs	`100% <ø> (ø)`
...ableCaseInspection/ParseTreeExpressionEvaluator.cs	`92.08% <ø> (ø)`
... and 158 more

retailcoder · 2018-06-05T21:06:14Z

Rubberduck.CodeAnalysis/Inspections/Inspector.cs

@@ -53,7 +55,6 @@ private void UpdateInspectionSeverity(Configuration config)
                        if (inspection.Name == setting.Name)
                        {
                            inspection.Severity = setting.Severity;
-                            continue;


Shouldn't this be a break?

I don't even know where this is coming from ... let me investigate that bit of change

seems to have been added on next with commit 459d322 ... :cough:

retailcoder · 2018-06-05T21:09:11Z

Rubberduck.Core/CodeAnalysis/CodeMetrics/CodeMetricsViewModel.cs

+            get => _selectedItem;
+            set
+            {
+                _selectedItem = value;


if value == _selectedItem, we're firing extraneous PropertyChanged events here.

retailcoder · 2018-06-05T21:13:30Z

Rubberduck.Core/UI/CodeExplorer/CodeExplorerWindow.cs

@@ -8,8 +9,8 @@ namespace Rubberduck.UI.CodeExplorer
    [ExcludeFromCodeCoverage]
    public partial class CodeExplorerWindow : UserControl, IDockableUserControl
    {
-        private const string ClassId = "C5318B59-172F-417C-88E3-B377CDA2D809";
-        string IDockableUserControl.ClassId { get { return ClassId; } }
+        private readonly string RandomGuid = Guid.NewGuid().ToString();


☠️ makes me wonder what the purpose of IDockableUserControl.GuidIdentifier really is... do we really need this?

retailcoder · 2018-06-05T21:14:13Z

Rubberduck.Core/UI/Controls/SearchResultWindow.cs

-        {
-            get { return "BFD04A86-CACA-4F95-9656-A0BF7D3AE254"; }
-        }
+        private readonly string RandomGuid = Guid.NewGuid().ToString();


If we do need a GUID, then why can't it be a RubberduckGuid constant?

This particular GUID is just used as an unique identifier for an instance created by the CreateToolWindow, as per documentation for GuidPosition parameter.

retailcoder · 2018-06-05T21:15:39Z

Rubberduck.Core/UI/Converters/DeclarationToMemberSignatureConverter.cs

+        {
+            var signature = $"{GetMethodType(member)} {member.IdentifierName}({string.Join(", ", GetMemberParameters(member))})";
+
+            return member.AsTypeName == null ? signature : $"{signature} As {member.AsTypeName}";


tempted to swap As for {Tokens.As} here 😉

retailcoder · 2018-06-05T21:19:10Z

Rubberduck.Core/UI/Converters/DeclarationToMemberSignatureConverter.cs

+                return new List<Parameter>();
+            }
+
+            if (GetMethodType(member) == "Property Get")


Feels wrong when the method returns $"{Tokens.Property} {Tokens.Get}", would prefer to have string interpolation here too... but why can we assume parameters.Count is >= 1 here? Most Property Get members are parameterless... 😕

bclothier reviewed Apr 16, 2018

View reviewed changes

Vogel612 force-pushed the codeMetrics-fixes branch 2 times, most recently from bd90b4a to 32ddfda Compare April 18, 2018 21:25

Vogel612 added help wanted PR-Status: Conflicting PR can't be merged as it stands, conflicts must be resolved by the author. and removed PR-Status: Review Requested No more commits, PR is ready for the eyes that need to see it. labels Apr 24, 2018

Vogel612 force-pushed the codeMetrics-fixes branch from 15197ef to f0b7c98 Compare June 4, 2018 18:37

Vogel612 commented Jun 4, 2018

View reviewed changes

Vogel612 added 14 commits June 5, 2018 22:09

Rename Rubberduck.Inspections to Rubberduck.StaticCodeAnalysis

d59c0f4

Move CodeMetrics into Rubberduck.CodeAnalysis

000d023

Also move a ton of other stuff around and cowboy a design for the CodeMetrics overhaul

Move to Rubberduck.CodeAnalysis

8cf6ad6

Cowboy the Redesign of CodeMetrics

102340d

This implements each CodeMetric in it's own separate class-collection and ParseTreeListener For now only Cyclomatic Complexity is implemented as a Proof Of Concept

one of these days I'll rewrite the projects to use globbing, FFS

0c5dd21

Implement LineCount metric on module level and extract common propert…

76631f6

…ies into baseclass

Add LineCount tests, refactor all CodeAnalysis tests to scope MockPar…

2d0af09

…ser with using-block

Implement NestingLevel Metric

82969b6

Clean up implementation of Cyclomatic Complexity

9346309

fix expected vs. actual in UciUnit_VariableMath

ae08bfc

more rebase-fixes

237a4dd

Remove old CodeMetrics Implementation

97fc5cd

Move CodeMetrics Core classes into separate files

ba6a6a1

Start work on CodeMetricsViewModel,

5d94a26

this should allow implementation of a proper UI. Do note that the interface of the VM is basically guesswork to how this works best.

Hosch250 and others added 11 commits June 5, 2018 22:34

Major fixes

e5976f2

Auto-select first and track selection across parses

e8149f8

Fix merge a bit

5967c5b

Move my VM changes over

3b0917d

UI might work

8f438ed

Fix remaining Rubberduck.Inspections references, Fix incorrectly name…

6d0bc73

…d ClassId on IDockableUserControl

Fix remaining Rubberduck.Inspections references, Fix incorrectly name…

213b716

…d ClassId on IDockableUserControl

Fix displaying signatures twice

6342cfd

Register CodeMetric implementations with MetricAnalyst to actually pr…

9d809c8

…ovide results

Use a sensible registration strategy

2dc99a3

Couple tweaks to the UI

3aac9ae

Vogel612 force-pushed the codeMetrics-fixes branch from 1e2f71f to 3aac9ae Compare June 5, 2018 20:52

retailcoder changed the title ~~Code metrics moving and redesign (WIP)~~ Code metrics moving and redesign Jun 5, 2018

retailcoder reviewed Jun 5, 2018

View reviewed changes

Vogel612 added 3 commits June 6, 2018 21:36

Adress Review Concerns

fac0f14

Replace skipped As with {Tokens.As}

7274423

Merge branch 'next' into codeMetrics-fixes

7a2eaaf

Vogel612 merged commit 7a2eaaf into rubberduck-vba:next Jun 7, 2018

Vogel612 mentioned this pull request Jun 7, 2018

Major fixes #4054

Closed

Vogel612 deleted the codeMetrics-fixes branch June 12, 2018 16:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code metrics moving and redesign #3934

Code metrics moving and redesign #3934

Vogel612 commented Apr 14, 2018 •

edited

Loading

bclothier left a comment

bclothier Apr 16, 2018

bclothier Apr 16, 2018

Vogel612 Apr 16, 2018

Vogel612 Jun 4, 2018

MDoerner Jun 4, 2018

Vogel612 Jun 4, 2018

Vogel612 Jun 4, 2018

codecov bot commented Jun 5, 2018 •

edited

Loading

retailcoder Jun 5, 2018

Vogel612 Jun 5, 2018

Vogel612 Jun 5, 2018

retailcoder Jun 5, 2018

retailcoder Jun 5, 2018

retailcoder Jun 5, 2018

bclothier Jun 5, 2018

retailcoder Jun 5, 2018

retailcoder Jun 5, 2018


		public override IEnumerable<ICodeMetricResult> Results() => new[] { new LineCountModuleMetricResult(_finder.ModuleDeclaration(_qmn), ownerReference, workingValue) };

		public override void EnterEndOfLine([NotNull] VBAParser.EndOfLineContext _) => workingValue++;

Code metrics moving and redesign #3934

Code metrics moving and redesign #3934

Conversation

Vogel612 commented Apr 14, 2018 • edited Loading

bclothier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 5, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vogel612 commented Apr 14, 2018 •

edited

Loading

codecov bot commented Jun 5, 2018 •

edited

Loading