Skip to content

Commit

Permalink
Enable TreatWarningsAsErrors (#165)
Browse files Browse the repository at this point in the history
#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
dotnet/reproducible-builds#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
  • Loading branch information
MattKotsenas authored Jul 29, 2024
1 parent 914926a commit a883a67
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 38 deletions.
2 changes: 0 additions & 2 deletions build/targets/codeanalysis/CodeAnalysis.props
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
<AnalysisMode>preview</AnalysisMode>
<WarningLevel>9999</WarningLevel>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<TreatWarningsAsErrors Condition=" '$(ContinuousIntegrationBuild)' == 'true' ">true</TreatWarningsAsErrors>
<MSBuildTreatWarningsAsErrors Condition=" '$(ContinuousIntegrationBuild)' == 'true' ">true</MSBuildTreatWarningsAsErrors>
</PropertyGroup>

<ItemGroup>
Expand Down
5 changes: 5 additions & 0 deletions build/targets/codeanalysis/CodeAnalysis.targets
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
<Project>
<PropertyGroup Label="Computed properties">
<PedanticMode Condition=" '$(PedanticMode)' == '' ">$([MSBuild]::ValueOrDefault('$(ContinuousIntegrationBuild)', 'false'))</PedanticMode>
<TreatWarningsAsErrors>$(PedanticMode)</TreatWarningsAsErrors>
<MSBuildTreatWarningsAsErrors>$(PedanticMode)</MSBuildTreatWarningsAsErrors>
</PropertyGroup>
</Project>
75 changes: 39 additions & 36 deletions src/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Diagnostics.CodeAnalysis;

namespace Moq.Analyzers;

/// <summary>
Expand Down Expand Up @@ -127,6 +129,41 @@ private static bool IsFirstArgumentMockBehavior(SyntaxNodeAnalysisContext contex
return IsExpressionMockBehavior(context, expression);
}

private static void VerifyDelegateMockAttempt(
SyntaxNodeAnalysisContext context,
ArgumentListSyntax? argumentList,
ImmutableArray<ArgumentSyntax> 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<ArgumentSyntax> 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();
Expand All @@ -140,7 +177,7 @@ private void AnalyzeCompilation(CompilationStartAnalysisContext context)
// We're interested in the few ways to create mocks:
// - new Mock<T>()
// - Mock.Of<T>()
// - MockRepository.Create<T>();
// - MockRepository.Create<T>()
//
// Ensure Moq is referenced in the compilation
ImmutableArray<INamedTypeSymbol> mockTypes = context.Compilation.GetMoqMock();
Expand Down Expand Up @@ -283,6 +320,7 @@ private void AnalyzeNewObject(SyntaxNodeAnalysisContext context)
/// <param name="context">The context.</param>
/// <returns><c>true</c> if a suitable constructor was found; otherwise <c>false</c>. </returns>
/// <remarks>Handles <see langword="params" /> and optional parameters.</remarks>
[SuppressMessage("Design", "MA0051:Method is too long", Justification = "This should be refactored; suppressing for now to enable TreatWarningsAsErrors in CI.")]
private bool AnyConstructorsFound(
ImmutableArray<IMethodSymbol> constructors,
ImmutableArray<ArgumentSyntax> arguments,
Expand Down Expand Up @@ -449,39 +487,4 @@ private void VerifyClassMockAttempt(
context.ReportDiagnostic(diagnostic);
}
}

private static void VerifyDelegateMockAttempt(
SyntaxNodeAnalysisContext context,
ArgumentListSyntax? argumentList,
ImmutableArray<ArgumentSyntax> 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<ArgumentSyntax> 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);
}
}
}
1 change: 1 addition & 0 deletions src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public bool IsEnabled(LogLevel logLevel)
}

public IDisposable BeginScope<TState>(TState state)
where TState : notnull
{
return NullScope.Instance;
}
Expand Down
10 changes: 10 additions & 0 deletions tests/.editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.ConstructorArgumentsShouldMatchAnalyzer>;

namespace Moq.Analyzers.Test;

public partial class ConstructorArgumentsShouldMatchAnalyzerTests
{
public static IEnumerable<object[]> ClassWithDefaultParamCtorTestData()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.ConstructorArgumentsShouldMatchAnalyzer>;

namespace Moq.Analyzers.Test;

public partial class ConstructorArgumentsShouldMatchAnalyzerTests
{
public static IEnumerable<object[]> DelegateTestData()
Expand Down

0 comments on commit a883a67

Please sign in to comment.