Remove dead suppressions and dead code (#242)

Fixes #90 - The AVXXXX rules were removed as part of #145, but some of the suppressions were left behind. Remove them. - The only instance of S2583 is dead code (guards above make it impossible). Remove it. - S2589 is still unclear to me, but it seems silly to keep a bug open for one suppression that we can revisit whenever we touch the code next.
rjmurillo · Oct 25, 2024 · ae15a28 · ae15a28
1 parent f2f9790
commit ae15a28
Show file tree

Hide file tree

Showing 19 changed files with 3 additions and 62 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -82,17 +82,13 @@ dotnet_style_readonly_field = true:warning
 
 # Parameter preferences
 dotnet_code_quality_unused_parameters = all:suggestion
-# AV1561: Signature contains too many parameters
-dotnet_diagnostic.AV1561.severity = suggestion
 
 # Suppression preferences
 dotnet_remove_unnecessary_suppression_exclusions = none
 
 # XMLDocs preferences
 # SA1600: Elements should be documented. We disable this it requires xmldocs for _all_ members. CS1591 already covers documenting public members.
 dotnet_diagnostic.SA1600.severity = silent
-# AV2305: Missing XML comment for internally visible type, member or parameter
-dotnet_diagnostic.AV2305.severity = silent
 
 #### C# Coding Conventions ####
 [*.cs]

diff --git a/build/targets/codeanalysis/.globalconfig b/build/targets/codeanalysis/.globalconfig
@@ -4,10 +4,6 @@ is_global=true
 # Only use this file for configuring diagnostics that aren't tied to a source file, and thus can't be placed under
 # any .editorconfig section.
 
-# AV2210 : Pass -warnaserror to the compiler or add <TreatWarningsAsErrors>True</TreatWarningsAsErrors> to your project file
-# This is set as part of the CI build. It is intentionally not set locally to allow for a fast inner dev loop.
-dotnet_diagnostic.AV2210.severity = none
-
 # Enable Effective C# Analyzers
 dotnet_diagnostic.ECS0100.severity = warning
 dotnet_diagnostic.ECS0200.severity = warning

diff --git a/src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs b/src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs
@@ -29,7 +29,6 @@ public override void Initialize(AnalysisContext context)
         context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
     }
 
-    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     private static void Analyze(SyntaxNodeAnalysisContext context)
     {
         InvocationExpressionSyntax callbackOrReturnsInvocation = (InvocationExpressionSyntax)context.Node;

diff --git a/src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs b/src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs
@@ -53,7 +53,7 @@ public override void Initialize(AnalysisContext context)
     /// <see langword="null" />.</returns>
     private static GenericNameSyntax? GetGenericNameSyntax(TypeSyntax typeSyntax)
     {
-        // REVIEW: Switch and ifs are equal in this case, but switch causes AV1535 to trigger
+        // REVIEW: Switch and ifs are equal in this case?
         // The switch expression adds more instructions to do the same, so stick with ifs
         if (typeSyntax is GenericNameSyntax genericNameSyntax)
         {

diff --git a/src/Analyzers/NoMethodsInPropertySetupAnalyzer.cs b/src/Analyzers/NoMethodsInPropertySetupAnalyzer.cs
@@ -29,7 +29,6 @@ public override void Initialize(AnalysisContext context)
         context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
     }
 
-    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     private static void Analyze(SyntaxNodeAnalysisContext context)
     {
         InvocationExpressionSyntax setupGetOrSetInvocation = (InvocationExpressionSyntax)context.Node;

diff --git a/src/Analyzers/NoSealedClassMocksAnalyzer.cs b/src/Analyzers/NoSealedClassMocksAnalyzer.cs
@@ -29,7 +29,6 @@ public override void Initialize(AnalysisContext context)
         context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.ObjectCreationExpression);
     }
 
-    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     private static void Analyze(SyntaxNodeAnalysisContext context)
     {
         ObjectCreationExpressionSyntax objectCreation = (ObjectCreationExpressionSyntax)context.Node;

diff --git a/src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs b/src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs
@@ -32,7 +32,6 @@ public override void Initialize(AnalysisContext context)
         context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
     }
 
-    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     private static void Analyze(SyntaxNodeAnalysisContext context)
     {
         InvocationExpressionSyntax setupInvocation = (InvocationExpressionSyntax)context.Node;

diff --git a/src/Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs b/src/Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs
@@ -29,7 +29,6 @@ public override void Initialize(AnalysisContext context)
         context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
     }
 
-    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     private static void Analyze(SyntaxNodeAnalysisContext context)
     {
         // Check Moq version and skip analysis if the version is 4.16.0 or later

diff --git a/src/Analyzers/SquiggleCop.Baseline.yaml b/src/Analyzers/SquiggleCop.Baseline.yaml
@@ -1053,7 +1053,7 @@
 - {Id: S2479, Title: Whitespace and control characters in string literals should be explicit, Category: Critical Code Smell, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: S2486, Title: Generic exceptions should not be ignored, Category: Minor Code Smell, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: S2551, Title: Shared resources should not be used for locking, Category: Critical Bug, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
-- {Id: S2583, Title: Conditionally executed code should be reachable, Category: Major Bug, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: true}
+- {Id: S2583, Title: Conditionally executed code should be reachable, Category: Major Bug, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: S2589, Title: Boolean expressions should not be gratuitous, Category: Major Code Smell, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: true}
 - {Id: S2612, Title: Setting loose file permissions is security-sensitive, Category: Major Security Hotspot, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: S2629, Title: Logging templates should be constant, Category: Major Code Smell, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}

diff --git a/src/CodeFixes/CallbackSignatureShouldMatchMockedMethodCodeFix.cs b/src/CodeFixes/CallbackSignatureShouldMatchMockedMethodCodeFix.cs
@@ -48,7 +48,6 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
             diagnostic);
     }
 
-    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     private static async Task<Document> FixCallbackSignatureAsync(SyntaxNode root, Document document, ParameterListSyntax? oldParameters, CancellationToken cancellationToken)
     {
         SemanticModel? semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);

diff --git a/src/CodeFixes/SquiggleCop.Baseline.yaml b/src/CodeFixes/SquiggleCop.Baseline.yaml
@@ -1053,7 +1053,7 @@
 - {Id: S2479, Title: Whitespace and control characters in string literals should be explicit, Category: Critical Code Smell, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: S2486, Title: Generic exceptions should not be ignored, Category: Minor Code Smell, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: S2551, Title: Shared resources should not be used for locking, Category: Critical Bug, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
-- {Id: S2583, Title: Conditionally executed code should be reachable, Category: Major Bug, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: true}
+- {Id: S2583, Title: Conditionally executed code should be reachable, Category: Major Bug, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: S2589, Title: Boolean expressions should not be gratuitous, Category: Major Code Smell, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: true}
 - {Id: S2612, Title: Setting loose file permissions is security-sensitive, Category: Major Security Hotspot, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: S2629, Title: Logging templates should be constant, Category: Major Code Smell, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}

diff --git a/src/Common/ArrayExtensions.cs b/src/Common/ArrayExtensions.cs
@@ -37,13 +37,6 @@ private static T[] RemoveRange<T>(this T[] array, int index, int length)
             throw new ArgumentOutOfRangeException(nameof(length));
         }
 
-#pragma warning disable S2583 // Change condition so it doesn't always evaluate to false
-        if (array.Length == 0)
-#pragma warning restore S2583
-        {
-            return array;
-        }
-
         T[] tmp = new T[array.Length - length];
         Array.Copy(array, tmp, index);
         Array.Copy(array, index + length, tmp, index, array.Length - index - length);

diff --git a/src/Common/MoqSetupMethodDescriptor.cs b/src/Common/MoqSetupMethodDescriptor.cs
@@ -8,7 +8,6 @@ internal class MoqSetupMethodDescriptor : MoqMethodDescriptorBase
 {
     private static readonly string MethodName = "Setup";
 
-    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     public override bool IsMatch(SemanticModel semanticModel, MemberAccessExpressionSyntax memberAccessSyntax, CancellationToken cancellationToken)
     {
         if (!IsFastMatch(memberAccessSyntax, MethodName.AsSpan()))

diff --git a/src/Common/SemanticModelExtensions.cs b/src/Common/SemanticModelExtensions.cs
@@ -34,7 +34,6 @@ internal static IEnumerable<IMethodSymbol> GetAllMatchingMockedMethodSymbolsFrom
             : semanticModel.GetAllMatchingSymbols<IMethodSymbol>(mockedMethodInvocation);
     }
 
-    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     internal static bool IsCallbackOrReturnInvocation(this SemanticModel semanticModel, InvocationExpressionSyntax callbackOrReturnsInvocation)
     {
         MemberAccessExpressionSyntax? callbackOrReturnsMethod = callbackOrReturnsInvocation.Expression as MemberAccessExpressionSyntax;
@@ -67,7 +66,6 @@ internal static bool IsMoqSetupMethod(this SemanticModel semanticModel, MemberAc
         return MoqSetupMethodDescriptor.IsMatch(semanticModel, method, cancellationToken);
     }
 
-    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     private static List<T> GetAllMatchingSymbols<T>(this SemanticModel semanticModel, ExpressionSyntax expression)
         where T : class
     {

diff --git a/src/tools/PerfDiff/.editorconfig b/src/tools/PerfDiff/.editorconfig
@@ -10,20 +10,6 @@ dotnet_diagnostic.CA1819.severity = none
 ### Suppress standards from our repo to this project since it came from dotnet team and we want to minimize changes ###
 # SA1600: Elements should be documented
 dotnet_diagnostic.SA1600.severity = none
-# AV1130: Return type in method signature should be an interface to an unchangeable collection
-dotnet_diagnostic.AV1130.severity = none
-# AV1135: Do not return null for strings, collections or tasks
-dotnet_diagnostic.AV1135.severity = none
-# AV1500: Member or local function contains too many statements
-dotnet_diagnostic.AV1500.severity = none
-# AV1537: If-else-if construct should end with an unconditional else clause
-dotnet_diagnostic.AV1537.severity = none
-# AV1562: Do not declare a parameter as ref or out
-dotnet_diagnostic.AV1562.severity = none
-# AV1564: Parameter in public or internal member is of type bool or bool?
-dotnet_diagnostic.AV1564.severity = none
-# AV1706: Identifier contains an abbreviation or is too short
-dotnet_diagnostic.AV1706.severity = none
 # CS1591: Missing XML comment for publicly visible type or member
 dotnet_diagnostic.CS1591.severity = none
 # IDE0008: Use explicit type
@@ -34,8 +20,6 @@ dotnet_diagnostic.IDE0161.severity = none
 dotnet_diagnostic.MA0002.severity = none
 # MA0049: Type name should not match containing namespace
 dotnet_diagnostic.MA0049.severity = none
-# AV1130: Return type in method signature should be an interface to an unchangeable collection
-dotnet_diagnostic.AV1130.severity = none
 # MA0051: Method is too long
 dotnet_diagnostic.MA0051.severity = none
 # S1118: Utility classes should not have public constructors
@@ -54,8 +38,6 @@ dotnet_diagnostic.S2930.severity = none
 dotnet_diagnostic.S6605.severity = none
 # S6667: Logging in a catch clause should pass the caught exception as a parameter.
 dotnet_diagnostic.S6667.severity = none
-# AV1580: Method argument calls a nested method
-dotnet_diagnostic.AV1580.severity = none
 # SA1009: Closing parenthesis should be spaced correctly
 dotnet_diagnostic.SA1009.severity = none
 # SA1208: System using directives should be placed before other using directives
@@ -78,14 +60,8 @@ dotnet_diagnostic.SA1204.severity = none
 dotnet_diagnostic.SA1601.severity = none
 # SA1308: Variable names should not be prefixed
 dotnet_diagnostic.SA1308.severity = none
-# AV1008: Class should not be static
-dotnet_diagnostic.AV1008.severity = none
 # IDE0022: Use block body for method
 dotnet_diagnostic.IDE0022.severity = none
-# AV1561: Signature contains too many parameters
-dotnet_diagnostic.AV1561.severity = none
-# AV1739: Unused lambda parameter should be renamed to underscore(s)
-dotnet_diagnostic.AV1739.severity = none
 # RCS1163: Unused parameter
 dotnet_diagnostic.RCS1163.severity = none
 # CA1860: Avoid using 'Enumerable.Any()' extension method

diff --git a/tests/.editorconfig b/tests/.editorconfig
@@ -24,7 +24,5 @@ dotnet_diagnostic.MA0051.severity = suggestion
 #### Naming conventions ####
 
 # VSTHRD200: Use "Async" suffix for async methods
-# AV1755: Postfix asynchronous methods with Async or TaskAsync
 # Just about every test method is async, doesn't provide any real value and clustters up test window
 dotnet_diagnostic.VSTHRD200.severity = none
-dotnet_diagnostic.AV1755.severity = none
diff --git a/tests/Moq.Analyzers.Benchmarks/Helpers/CSharpCompilationCreator.cs b/tests/Moq.Analyzers.Benchmarks/Helpers/CSharpCompilationCreator.cs
@@ -21,7 +21,6 @@ internal static class CSharpCompilationCreator
         return (await project.GetCompilationAsync().ConfigureAwait(false), options);
     }
 
-    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1553:Do not use optional parameters with default value null for strings, collections or tasks", Justification = "Minimizing divergence from upstream code")]
     private static Task<(Project Project, AnalyzerOptions Options)> CreateProjectAsync((string, string)[] sourceFiles, (string, string)[]? globalOptions = null)
         => CompilationCreator.CreateProjectAsync(
             sourceFiles,

diff --git a/tests/Moq.Analyzers.Benchmarks/Helpers/CompilationCreator.cs b/tests/Moq.Analyzers.Benchmarks/Helpers/CompilationCreator.cs
@@ -15,7 +15,6 @@ internal static class CompilationCreator
 {
     private static readonly ReferenceAssemblies ReferenceAssemblies = ReferenceAssemblies.Net.Net80.AddPackages([new PackageIdentity("Moq", "4.18.4")]);
 
-    [SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Minimizing divergence from upstream code.")]
     public static async Task<(Project Project, AnalyzerOptions Options)> CreateProjectAsync(
         (string, string)[] sourceFiles,
         (string, string)[]? globalOptions,
@@ -47,9 +46,6 @@ internal static class CompilationCreator
         return (project, project.AnalyzerOptions);
     }
 
-    [SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Minimizing divergence with upstream code")]
-    [SuppressMessage("Maintainability", "AV1551:Method overload should call another overload", Justification = "Minimizing divergence with upstream code")]
-    [SuppressMessage("Maintainability", "AV1555:Avoid using non-(nullable-)boolean named arguments", Justification = "Minimizing divergence with upstream code")]
     private static async Task<Project> CreateProjectAsync(
         EvaluatedProjectState primaryProject,
         CompilationOptions compilationOptions,
@@ -79,8 +75,6 @@ private static async Task<Project> CreateProjectAsync(
         return solution.GetProject(projectId)!;
     }
 
-    [SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Minimizing divergence from upstream")]
-    [SuppressMessage("Maintainability", "AV1561:Signature contains too many parameters", Justification = "Minimizing divergence from upstream")]
     private static async Task<Solution> CreateSolutionAsync(
         ProjectId projectId,
         EvaluatedProjectState projectState,

diff --git a/tests/Moq.Analyzers.Benchmarks/Helpers/ExportProviderExtensions.cs b/tests/Moq.Analyzers.Benchmarks/Helpers/ExportProviderExtensions.cs
@@ -24,7 +24,6 @@ public CompositionContextShim(ExportProvider exportProvider)
             _exportProvider = exportProvider;
         }
 
-        [SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Minimizing divergence from upstream")]
         public override bool TryGetExport(CompositionContract contract, [NotNullWhen(true)] out object? export)
         {
 #pragma warning disable ECS0900 // Minimize boxing and unboxing
@@ -58,7 +57,6 @@ where method.GetParameters().Length == 1 && method.GetParameters()[0].ParameterT
 #pragma warning restore CS8762 // Parameter must have a non-null value when exiting in some condition.
         }
 
-        [SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Minimizing divergence from upstream")]
         private static (Type ExportType, Type? MetadataType) GetContractType(Type contractType, bool importMany)
         {
             if (importMany && contractType.IsConstructedGenericType &&