From a883a67cb52389ee6b27f532ef24f74d68625514 Mon Sep 17 00:00:00 2001 From: Matt Kotsenas Date: Mon, 29 Jul 2024 14:57:17 -0700 Subject: [PATCH] Enable TreatWarningsAsErrors (#165) #91 intended to enable warnings as errors in CI. However, `$(ContinuousIntegrationBuild)` is set by `DotNet.ReproducibleBuilds`, and that package's .props file is imported _after_ our own Directory.Build.props. As a result, the properties were evaluated before they were set. This change moves them into the .targets file as "computed properties" in order for them to pick up the configuration from .props files. This aligns with the [documented guidance](https://learn.microsoft.com/en-us/visualstudio/msbuild/customize-your-build?view=vs-2022#choose-between-adding-properties-to-a-props-or-targets-file) "Set dependent properties in .targets files, because they pick up customizations from individual projects.". I've also filed https://github.com/dotnet/reproducible-builds/issues/45 to see if we can make the reproducible-builds package more intuitive. Lastly, this cleans up existing warnings: - Fixes whitespace issues - Suppresses StyleCop warnings about documenting public members in tests, which already had the built-in rules suppressed - Reorders static members to be above instance members - Adds a single suppression for a long method to keep the change small --- build/targets/codeanalysis/CodeAnalysis.props | 2 - .../targets/codeanalysis/CodeAnalysis.targets | 5 ++ ...ConstructorArgumentsShouldMatchAnalyzer.cs | 75 ++++++++++--------- .../PerfDiff/Logging/SimpleConsoleLogger.cs | 1 + tests/.editorconfig | 10 +++ ...tsShouldMatchAnalyzerTests.DefaultParam.cs | 1 + ...mentsShouldMatchAnalyzerTests.Delegates.cs | 1 + 7 files changed, 57 insertions(+), 38 deletions(-) diff --git a/build/targets/codeanalysis/CodeAnalysis.props b/build/targets/codeanalysis/CodeAnalysis.props index cc3e67ab..550f9e6b 100644 --- a/build/targets/codeanalysis/CodeAnalysis.props +++ b/build/targets/codeanalysis/CodeAnalysis.props @@ -5,8 +5,6 @@ preview 9999 true - true - true diff --git a/build/targets/codeanalysis/CodeAnalysis.targets b/build/targets/codeanalysis/CodeAnalysis.targets index 8c119d54..55b528a4 100644 --- a/build/targets/codeanalysis/CodeAnalysis.targets +++ b/build/targets/codeanalysis/CodeAnalysis.targets @@ -1,2 +1,7 @@ + + $([MSBuild]::ValueOrDefault('$(ContinuousIntegrationBuild)', 'false')) + $(PedanticMode) + $(PedanticMode) + diff --git a/src/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs b/src/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs index 3a2f9344..f2a1e51c 100644 --- a/src/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs +++ b/src/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace Moq.Analyzers; /// @@ -127,6 +129,41 @@ private static bool IsFirstArgumentMockBehavior(SyntaxNodeAnalysisContext contex return IsExpressionMockBehavior(context, expression); } + private static void VerifyDelegateMockAttempt( + SyntaxNodeAnalysisContext context, + ArgumentListSyntax? argumentList, + ImmutableArray arguments) + { + if (arguments.Length == 0) + { + return; + } + + Diagnostic? diagnostic = argumentList?.GetLocation().CreateDiagnostic(ClassMustHaveMatchingConstructor, argumentList); + if (diagnostic != null) + { + context.ReportDiagnostic(diagnostic); + } + } + + private static void VerifyInterfaceMockAttempt( + SyntaxNodeAnalysisContext context, + ArgumentListSyntax? argumentList, + ImmutableArray arguments) + { + // Interfaces and delegates don't have ctors, so bail out early + if (arguments.Length == 0) + { + return; + } + + Diagnostic? diagnostic = argumentList?.GetLocation().CreateDiagnostic(InterfaceMustNotHaveConstructorParameters, argumentList); + if (diagnostic != null) + { + context.ReportDiagnostic(diagnostic); + } + } + private void AnalyzeCompilation(CompilationStartAnalysisContext context) { context.CancellationToken.ThrowIfCancellationRequested(); @@ -140,7 +177,7 @@ private void AnalyzeCompilation(CompilationStartAnalysisContext context) // We're interested in the few ways to create mocks: // - new Mock() // - Mock.Of() - // - MockRepository.Create(); + // - MockRepository.Create() // // Ensure Moq is referenced in the compilation ImmutableArray mockTypes = context.Compilation.GetMoqMock(); @@ -283,6 +320,7 @@ private void AnalyzeNewObject(SyntaxNodeAnalysisContext context) /// The context. /// true if a suitable constructor was found; otherwise false. /// Handles and optional parameters. + [SuppressMessage("Design", "MA0051:Method is too long", Justification = "This should be refactored; suppressing for now to enable TreatWarningsAsErrors in CI.")] private bool AnyConstructorsFound( ImmutableArray constructors, ImmutableArray arguments, @@ -449,39 +487,4 @@ private void VerifyClassMockAttempt( context.ReportDiagnostic(diagnostic); } } - - private static void VerifyDelegateMockAttempt( - SyntaxNodeAnalysisContext context, - ArgumentListSyntax? argumentList, - ImmutableArray arguments) - { - if (arguments.Length == 0) - { - return; - } - - Diagnostic? diagnostic = argumentList?.GetLocation().CreateDiagnostic(ClassMustHaveMatchingConstructor, argumentList); - if (diagnostic != null) - { - context.ReportDiagnostic(diagnostic); - } - } - - private static void VerifyInterfaceMockAttempt( - SyntaxNodeAnalysisContext context, - ArgumentListSyntax? argumentList, - ImmutableArray arguments) - { - // Interfaces and delegates don't have ctors, so bail out early - if (arguments.Length == 0) - { - return; - } - - Diagnostic? diagnostic = argumentList?.GetLocation().CreateDiagnostic(InterfaceMustNotHaveConstructorParameters, argumentList); - if (diagnostic != null) - { - context.ReportDiagnostic(diagnostic); - } - } } diff --git a/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs b/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs index 918daf4f..e6d3d867 100644 --- a/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs +++ b/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs @@ -66,6 +66,7 @@ public bool IsEnabled(LogLevel logLevel) } public IDisposable BeginScope(TState state) + where TState : notnull { return NullScope.Instance; } diff --git a/tests/.editorconfig b/tests/.editorconfig index 576433c2..7fb596fa 100644 --- a/tests/.editorconfig +++ b/tests/.editorconfig @@ -13,6 +13,16 @@ dotnet_diagnostic.CS1591.severity = suggestion # CS1712: Type parameter 'type parameter' has no matching typeparam tag in the XML comment on 'type' (but other type parameters do) dotnet_diagnostic.CS1712.severity = suggestion +# SA1601: Partial elements should be documented +dotnet_diagnostic.SA1601.severity = suggestion + +#### Code quality #### + +# MA0051: Method is too long +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 diff --git a/tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.DefaultParam.cs b/tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.DefaultParam.cs index b136201e..fd90b868 100644 --- a/tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.DefaultParam.cs +++ b/tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.DefaultParam.cs @@ -1,6 +1,7 @@ using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; namespace Moq.Analyzers.Test; + public partial class ConstructorArgumentsShouldMatchAnalyzerTests { public static IEnumerable ClassWithDefaultParamCtorTestData() diff --git a/tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.Delegates.cs b/tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.Delegates.cs index 27af26e4..e701b8e2 100644 --- a/tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.Delegates.cs +++ b/tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.Delegates.cs @@ -1,6 +1,7 @@ using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; namespace Moq.Analyzers.Test; + public partial class ConstructorArgumentsShouldMatchAnalyzerTests { public static IEnumerable DelegateTestData()