Port FxCop Code Metrics rules #1740

mavasani · 2018-07-19T20:49:02Z

CA1501: Avoid excessive inheritance
CA1502: Avoid excessive complexity
CA1505: Avoid unmaintainable code
CA1506: Avoid excessive class coupling

Fixes #433
Fixes #434
Fixes #436
Fixes #437

Customer requests:

Future work items (~~I will file issues for these soon~~#1741):

Refactor the core code metrics computation into a separate library.
A command line tool, such as Metrics.exe, that uses the above library and dumps out the metrics tree onto console or a log file (Metrics.exe shipped prior to VS2017).
Share code with VS implementation of Code Metrics - currently we have a clone of the code that computes the metrics. Once we start inserting packages from this repo to VS repo, we can delete the clone and reference that package (which will need to happen when we move CodeStyle rules to NuGet packages).

1. [CA1501](https://docs.microsoft.com/en-us/visualstudio/code-quality/ca1501-avoid-excessive-inheritance): Avoid excessive inheritance 2. [CA1502](https://docs.microsoft.com/en-us/visualstudio/code-quality/ca1502-avoid-excessive-complexity): Avoid excessive complexity 3. [CA1505](https://docs.microsoft.com/en-us/visualstudio/code-quality/ca1505-avoid-unmaintainable-code): Avoid unmaintainable code 4. [CA1506](https://docs.microsoft.com/en-us/visualstudio/code-quality/ca1506-avoid-excessive-class-coupling): Avoid excessive class coupling Fixes dotnet#433 Fixes dotnet#434 Fixes dotnet#436 Fixes dotnet#437 Future work items (I will file issues for these soon): 1. Refactor the core code metrics computation into a separate library. 2 A command line tool, such as Metrics.exe, that uses the above library and dumps out the metrics tree onto console or a log file (Metrics.exe shipped prior to VS2017). 3. Share code with VS implementation of Code Metrics - currently we have a clone of the code that computes the metrics. Once we start inserting packages from this repo to VS repo, we can delete the clone and reference that package (which will need to happen when we move CodeStyle rules to NuGet packages).

mavasani · 2018-07-19T21:00:53Z

Tagging @heejaechang as the core code metrics computation code/tests are a clone from the VS side pull request. #1741 tracks refactoring/packaging this further.

heejaechang · 2018-07-19T21:36:18Z

is analyzer only new code and all others are just fork from vs side?

mavasani · 2018-07-19T21:44:52Z

is analyzer only new code and all others are just fork from vs side?

Yes, analyzer and analyzer tests are the only new piece of code.

heejaechang · 2018-07-19T21:46:18Z

src/Microsoft.CodeQuality.Analyzers/Core/Maintainability/CodeMetricsAnalyzer.cs

+        ///     CA1501: 10
+        /// See CA1508 unit tests for more examples.
+        /// </summary>
+        private const string CodeMetricsConfigurationFile = "CodeMetricsConfig.txt";


is this existing format? or new? if it is new, can we use new editorconfig support?

Talked offline, the editorconfig options passed down to the compiler/analyzers is still not checked in, and will not work against 2.6.1 (which the current branch targets)

heejaechang · 2018-07-19T21:48:18Z

src/Microsoft.CodeQuality.Analyzers/Core/Maintainability/CodeMetricsAnalyzer.cs

+                    {
+                        compilationContext.ReportDiagnostic(diagnostic);
+                    }
+                }


is it intentionally keep going? or forgot to return?>

Good idea, let me change analyzer to bail out on configuration errors.

heejaechang · 2018-07-19T21:52:38Z

src/Microsoft.CodeQuality.Analyzers/Core/Maintainability/CodeMetricsAnalyzer.cs

+                }
+
+                // Compute code metrics.
+                var computeTask = Task.Run(() => CodeAnalysisMetricData.ComputeAsync(compilationContext.Compilation, compilationContext.CancellationToken));


is there a reason who run it on Task.Run and then wait on that? rather than directly wait on task returned by ComputAsync?

heejaechang · 2018-07-19T21:57:54Z

src/Microsoft.CodeQuality.Analyzers/Core/Maintainability/CodeMetricsAnalyzer.cs

+        private static bool TryParseCodeMetricsConfigurationFile(
+            AdditionalText additionalText,
+            CancellationToken cancellationToken,
+            out ImmutableDictionary<string, List<(SymbolKind?, uint)>> ruleIdToThresholdMap,


reason it is List rather than dictionary? I guess it is expected to be small most of time?

IReadOnlyList?

Yes, only one rule realistically needs separate values for Types and Methods. I can change it to IReadOnlyList.

mavasani · 2018-07-19T23:24:46Z

@dotnet-bot retest prtest/Windows/NT/Debug please

mavasani · 2018-07-20T00:10:11Z

@dotnet-bot retest prtest/Windows/NT/Debug please

heejaechang · 2018-07-20T00:10:14Z

src/Microsoft.CodeQuality.Analyzers/UnitTests/Maintainability/CodeMetricsComputationTests.cs

+
+";
+
+            var expectedMetricsText = @"


are these new test or ported from VS side?

Ported from VS side. Hopefully we will delete from VS side soon.

heejaechang · 2018-07-20T00:11:14Z

src/Utilities/CodeMetrics/CodeAnalysisMetricData.AssemblyMetricData.cs

@@ -0,0 +1,103 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.


is this right copyright header?

I used the same header as other files in the repo, for example https://github.com/dotnet/roslyn-analyzers/blob/master/src/Utilities/DiagnosticHelpers.cs#L1

mavasani requested a review from a team July 19, 2018 20:49

mavasani mentioned this pull request Jul 19, 2018

Future work items for Code Metrics #1741

Closed

3 tasks

Associate tracking issue

7577dcd

heejaechang reviewed Jul 19, 2018

View reviewed changes

Address review feedback

f5b4c5d

heejaechang reviewed Jul 20, 2018

View reviewed changes

heejaechang approved these changes Jul 20, 2018

View reviewed changes

mavasani merged commit 6875477 into dotnet:master Jul 20, 2018

mavasani deleted the Complexity branch July 20, 2018 12:49

mavasani mentioned this pull request Jul 24, 2018

Running Code Metrics on CPS project targeting .NET Core/.NET Standard displays an error dotnet/roslyn#27001

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port FxCop Code Metrics rules #1740

Port FxCop Code Metrics rules #1740

mavasani commented Jul 19, 2018 •

edited

Loading

mavasani commented Jul 19, 2018

heejaechang commented Jul 19, 2018

mavasani commented Jul 19, 2018

heejaechang Jul 19, 2018

mavasani Jul 19, 2018

heejaechang Jul 19, 2018

mavasani Jul 19, 2018

heejaechang Jul 19, 2018

heejaechang Jul 19, 2018

heejaechang Jul 19, 2018

mavasani Jul 19, 2018

mavasani commented Jul 19, 2018

mavasani commented Jul 20, 2018

heejaechang Jul 20, 2018

mavasani Jul 20, 2018

heejaechang Jul 20, 2018

mavasani Jul 20, 2018

		@@ -0,0 +1,103 @@
		// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

Port FxCop Code Metrics rules #1740

Port FxCop Code Metrics rules #1740

Conversation

mavasani commented Jul 19, 2018 • edited Loading

mavasani commented Jul 19, 2018

heejaechang commented Jul 19, 2018

mavasani commented Jul 19, 2018

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

mavasani commented Jul 19, 2018

mavasani commented Jul 20, 2018

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

mavasani commented Jul 19, 2018 •

edited

Loading