diff --git a/.claude/scratchpad/Common.props b/.claude/scratchpad/Common.props index 7e24bef9..1c08ac47 100644 --- a/.claude/scratchpad/Common.props +++ b/.claude/scratchpad/Common.props @@ -31,6 +31,12 @@ false false + + false + false + + + false diff --git a/docs/sdk-modules/standard-analyzers.md b/docs/sdk-modules/standard-analyzers.md index d163996f..8647871b 100644 --- a/docs/sdk-modules/standard-analyzers.md +++ b/docs/sdk-modules/standard-analyzers.md @@ -1,6 +1,6 @@ # `StandardAnalyzers` module -This module turns on the code quality and code style analyzers of the .NET SDK, StyleCop, and the public API analyzers. +This module turns on the code quality and code style analyzers of the .NET SDK, StyleCop, the public API analyzers, and Nullable.Extended.Analyzer. It sets stricter defaults than the .NET SDK, and adds the `stylecop.json` file and the public and internal API files to the compilation. --- @@ -13,6 +13,7 @@ It sets stricter defaults than the .NET SDK, and adds the `stylecop.json` file a - [Code analysis properties](#code-analysis-properties) - [`UseStyleCopAnalyzers` property](#usestylecopanalyzers-property) - [`UsePublicApiAnalyzers` property](#usepublicapianalyzers-property) + - [`UseNullableExtendedAnalyzer` property](#usenullableextendedanalyzer-property) - [`UseTfmSpecificPublicApiFiles` property](#usetfmspecificpublicapifiles-property) - [`UseTfmSpecificInternalApiFiles` property](#usetfmspecificinternalapifiles-property) - [Usage](#usage) @@ -20,13 +21,14 @@ It sets stricter defaults than the .NET SDK, and adds the `stylecop.json` file a - [`stylecop.json` file](#stylecopjson-file) - [Public API files](#public-api-files) - [Internal API files](#internal-api-files) + - [Null-forgiving operators](#null-forgiving-operators) - [Diagnostics](#diagnostics) --- ## Configuration -The module is active in every project, and each of the three analyzer sets has a switch of its own. +The module is active in every project, and each of the four analyzer sets has a switch of its own. Configure the rules of every set in an `.editorconfig` file or a `.globalconfig` file, as the [.NET documentation](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files) describes. A severity in an `.editorconfig` file reaches only a diagnostic located in a source file. So configure RS0052, RS0053, RS0054, and RS0058 in a `.globalconfig` file, because they report a line of an internal API file, or no location. @@ -62,6 +64,12 @@ Set it to `true` to run the public API analyzers in a project, or to `false` to The default is `true` for a [library project](../internal-use-properties.md#project-type), and `false` for every other project. A value MSBuild reads as true, such as `yes`, counts as `true`, and any other value counts as `false`. +### `UseNullableExtendedAnalyzer` property + +Set it to `false` to leave Nullable.Extended.Analyzer out of a project. +The default is `true`, except for a project that uses the `Microsoft.Build.NoTargets` SDK. +A value MSBuild reads as true, such as `yes`, counts as `true`, and any other value counts as `false`. + ### `UseTfmSpecificPublicApiFiles` property Set it to `true` to keep one pair of [public API files](#public-api-files) per target framework, or to `false` to keep one pair for the project. @@ -80,10 +88,17 @@ A value MSBuild reads as true, such as `yes`, counts as `true`, and any other va ### Package references -The module adds a `PackageReference` to `StyleCop.Analyzers` when `UseStyleCopAnalyzers` is `true`, and one to `Microsoft.CodeAnalysis.PublicApiAnalyzers` when `UsePublicApiAnalyzers` is `true`. +The module adds a `PackageReference` to each analyzer package whose switch is `true`: + +| Package | Switch | +| ------------------------------------------- | ----------------------------- | +| `StyleCop.Analyzers` | `UseStyleCopAnalyzers` | +| `Microsoft.CodeAnalysis.PublicApiAnalyzers` | `UsePublicApiAnalyzers` | +| `Nullable.Extended.Analyzer` | `UseNullableExtendedAnalyzer` | + Buildvana SDK pins the version of each package, and `dotnet list package` shows the reference as auto-referenced, with the `(A)` mark. Each reference has `PrivateAssets="All"`, so a project that references this one, and a NuGet package this one produces, get no dependency on the analyzers. -A `PackageReference` of yours to either package replaces the module's reference, and under central package management a `PackageVersion` item sets its version. +A `PackageReference` of yours to one of the packages replaces the module's reference, and under central package management a `PackageVersion` item sets its version. The code quality and code style analyzers ship with the .NET SDK, and need no package. @@ -149,6 +164,29 @@ Without `require_api_files`, RS0051 reports every internal type and member of a With `require_api_files` set to `true`, a project with neither file of a pair gets no diagnostic for that kind of API. A project with one file of the internal pair still gets RS0058. +### Null-forgiving operators + +Nullable.Extended.Analyzer reports every null-forgiving operator (`!`) as a warning. +The operator silences a nullable warning on the strength of an invariant the compiler cannot check, and the analyzer makes each such invariant visible. +Four rules report the operator, one per context: + +| Rule | Context | +| ------ | ---------------------------------------------------------------------------------------------- | +| NX0001 | Any context the other three rules do not cover | +| NX0002 | On `null`, `default`, or `default(T)` | +| NX0003 | Inside a lambda expression, on any other operand | +| NX0004 | On `null`, `default`, or `default(T)` in the initializer of a property with an `init` accessor | + +A comment on the line above the statement or declaration clears the warning, when its text starts with `!` and a space: + +```csharp +// ! The dictionary was filled from the same keys a moment ago. +var value = lookup.GetValueOrDefault(key)!; +``` + +The [Nullable.Extended documentation](https://github.com/tom-englert/Nullable.Extended) describes the analyzer and its code fixes. +Set the severity of each rule in a `.editorconfig` file or a `.globalconfig` file, as [Configuration](#configuration) says. + --- ## Diagnostics diff --git a/dotnet-tools.json b/dotnet-tools.json index a029fe81..aaaa9323 100644 --- a/dotnet-tools.json +++ b/dotnet-tools.json @@ -7,6 +7,12 @@ "commands": [ "bv" ] + }, + "nullable.extended.mcp": { + "version": "1.18.2", + "commands": [ + "nullable-extended-mcp" + ] } } } diff --git a/src/Buildvana.Sdk/Modules/StandardAnalyzers/Module.UseNullableExtendedAnalyzer.targets b/src/Buildvana.Sdk/Modules/StandardAnalyzers/Module.UseNullableExtendedAnalyzer.targets new file mode 100644 index 00000000..0c48c6d0 --- /dev/null +++ b/src/Buildvana.Sdk/Modules/StandardAnalyzers/Module.UseNullableExtendedAnalyzer.targets @@ -0,0 +1,10 @@ + + + + + + + + diff --git a/src/Buildvana.Sdk/Modules/StandardAnalyzers/Module.targets b/src/Buildvana.Sdk/Modules/StandardAnalyzers/Module.targets index ac1a3f0b..76f22479 100644 --- a/src/Buildvana.Sdk/Modules/StandardAnalyzers/Module.targets +++ b/src/Buildvana.Sdk/Modules/StandardAnalyzers/Module.targets @@ -54,4 +54,20 @@ + + + true + false + + + + true + + + false + + + + diff --git a/src/Buildvana.Sdk/Sdk/PackageVersions.props b/src/Buildvana.Sdk/Sdk/PackageVersions.props index fba6a863..5cca52ce 100644 --- a/src/Buildvana.Sdk/Sdk/PackageVersions.props +++ b/src/Buildvana.Sdk/Sdk/PackageVersions.props @@ -8,6 +8,7 @@ --> + diff --git a/tests/Buildvana.Sdk.Tests/Buildvana.Sdk.Tests.csproj b/tests/Buildvana.Sdk.Tests/Buildvana.Sdk.Tests.csproj index 91f8b35e..e08b7799 100644 --- a/tests/Buildvana.Sdk.Tests/Buildvana.Sdk.Tests.csproj +++ b/tests/Buildvana.Sdk.Tests/Buildvana.Sdk.Tests.csproj @@ -39,6 +39,7 @@ + diff --git a/tests/Buildvana.Sdk.Tests/StandardAnalyzersModuleTargetsTests.cs b/tests/Buildvana.Sdk.Tests/StandardAnalyzersModuleTargetsTests.cs new file mode 100644 index 00000000..d318a0a0 --- /dev/null +++ b/tests/Buildvana.Sdk.Tests/StandardAnalyzersModuleTargetsTests.cs @@ -0,0 +1,283 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Reflection; +using Buildvana.Core.Testing; +using Microsoft.Build.Definition; +using Microsoft.Build.Evaluation; +using Microsoft.Build.Execution; + +// Evaluates the real Module.targets of the StandardAnalyzers module, imported by a project of a temporary home +// directory that states the project type properties Buildvana SDK computes before the module runs. +// Evaluation-only: no targets are executed, so assertions are limited to properties and items. +internal sealed class StandardAnalyzersModuleTargetsTests +{ + private const string ProjectDirectory = "src/Test"; + private const string EnableNETAnalyzers = "EnableNETAnalyzers"; + private const string UseStyleCopAnalyzers = "UseStyleCopAnalyzers"; + private const string UsePublicApiAnalyzers = "UsePublicApiAnalyzers"; + private const string UseNullableExtendedAnalyzer = "UseNullableExtendedAnalyzer"; + private const string StyleCopAnalyzers = "StyleCop.Analyzers"; + private const string PublicApiAnalyzers = "Microsoft.CodeAnalysis.PublicApiAnalyzers"; + private const string NullableExtendedAnalyzer = "Nullable.Extended.Analyzer"; + + // A project that is neither a no-target project nor a library project. + private const string OrdinaryProject = """ + false + false + """; + + private const string NoTargetsProject = """ + true + false + """; + + private const string LibraryProject = """ + false + true + """; + + [Test] + public async Task Evaluate_ByDefault_SetsTheCodeAnalysisDefaults() + { + using var home = new TempHome(); + var project = Evaluate(home, OrdinaryProject); + await Assert.That(project.GetPropertyValue(EnableNETAnalyzers)).IsEqualTo("true"); + await Assert.That(project.GetPropertyValue("AnalysisLevel")).IsEqualTo("latest"); + await Assert.That(project.GetPropertyValue("AnalysisMode")).IsEqualTo("All"); + await Assert.That(project.GetPropertyValue("EnforceCodeStyleInBuild")).IsEqualTo("true"); + } + + [Test] + public async Task Evaluate_AnalysisLevelAndAnalysisModeSet_KeepsBoth() + { + using var home = new TempHome(); + const string properties = OrdinaryProject + "9.0Recommended"; + var project = Evaluate(home, properties); + await Assert.That(project.GetPropertyValue("AnalysisLevel")).IsEqualTo("9.0"); + await Assert.That(project.GetPropertyValue("AnalysisMode")).IsEqualTo("Recommended"); + } + + [Test] + [Arguments("false")] + [Arguments("no")] + [Arguments("Off")] + [Arguments("!true")] + public async Task Evaluate_EnforceCodeStyleInBuildMSBuildReadsAsFalse_StoresFalse(string value) + { + using var home = new TempHome(); + var project = Evaluate(home, OrdinaryProject + $"{value}"); + await Assert.That(project.GetPropertyValue("EnforceCodeStyleInBuild")).IsEqualTo("false"); + } + + [Test] + [Arguments("true")] + [Arguments("maybe")] + public async Task Evaluate_EnforceCodeStyleInBuildOtherThanFalse_StoresTrue(string value) + { + using var home = new TempHome(); + var project = Evaluate(home, OrdinaryProject + $"{value}"); + await Assert.That(project.GetPropertyValue("EnforceCodeStyleInBuild")).IsEqualTo("true"); + } + + [Test] + public async Task Evaluate_OrdinaryProject_ReferencesStyleCopAndNullableExtendedAnalyzer() + { + using var home = new TempHome(); + var project = Evaluate(home, OrdinaryProject); + await Assert.That(project.GetPropertyValue(UseStyleCopAnalyzers)).IsEqualTo("true"); + await Assert.That(project.GetPropertyValue(UsePublicApiAnalyzers)).IsEqualTo("false"); + await Assert.That(project.GetPropertyValue(UseNullableExtendedAnalyzer)).IsEqualTo("true"); + await Assert.That(GetReferences(project)).IsEquivalentTo([StyleCopAnalyzers, NullableExtendedAnalyzer]); + } + + [Test] + public async Task Evaluate_NoTargetsProject_TurnsEveryAnalyzerOff() + { + using var home = new TempHome(); + var project = Evaluate(home, NoTargetsProject); + await Assert.That(project.GetPropertyValue(EnableNETAnalyzers)).IsEqualTo("false"); + await Assert.That(project.GetPropertyValue(UseStyleCopAnalyzers)).IsEqualTo("false"); + await Assert.That(project.GetPropertyValue(UsePublicApiAnalyzers)).IsEqualTo("false"); + await Assert.That(project.GetPropertyValue(UseNullableExtendedAnalyzer)).IsEqualTo("false"); + await Assert.That(GetReferences(project)).IsEmpty(); + } + + [Test] + public async Task Evaluate_LibraryProject_ReferencesEveryAnalyzerPackage() + { + using var home = new TempHome(); + var project = Evaluate(home, LibraryProject); + await Assert.That(project.GetPropertyValue(UsePublicApiAnalyzers)).IsEqualTo("true"); + string[] expected = [StyleCopAnalyzers, PublicApiAnalyzers, NullableExtendedAnalyzer]; + await Assert.That(GetReferences(project)).IsEquivalentTo(expected); + } + + // A no-target project that is not a library project defaults every switch to false, so a value that stores 'true' + // is the one set. + [Test] + [MatrixDataSource] + public async Task Evaluate_SwitchMSBuildReadsAsTrue_StoresTrue( + [Matrix(EnableNETAnalyzers, UseStyleCopAnalyzers, UsePublicApiAnalyzers, UseNullableExtendedAnalyzer)] + string name, + [Matrix("true", "yes", "On", "!false")] string value) + { + using var home = new TempHome(); + var project = Evaluate(home, NoTargetsProject + $"<{name}>{value}"); + await Assert.That(project.GetPropertyValue(name)).IsEqualTo("true"); + } + + // A library project defaults every switch to true, so a value that stores 'false' is the one set. + [Test] + [MatrixDataSource] + public async Task Evaluate_SwitchOtherThanTrue_StoresFalse( + [Matrix(EnableNETAnalyzers, UseStyleCopAnalyzers, UsePublicApiAnalyzers, UseNullableExtendedAnalyzer)] + string name, + [Matrix("false", "no", "maybe")] string value) + { + using var home = new TempHome(); + var project = Evaluate(home, LibraryProject + $"<{name}>{value}"); + await Assert.That(project.GetPropertyValue(name)).IsEqualTo("false"); + } + + [Test] + [Arguments(UseStyleCopAnalyzers, StyleCopAnalyzers)] + [Arguments(UsePublicApiAnalyzers, PublicApiAnalyzers)] + [Arguments(UseNullableExtendedAnalyzer, NullableExtendedAnalyzer)] + public async Task Evaluate_SwitchTrue_ReferencesThePackageWithPrivateAssets(string name, string package) + { + using var home = new TempHome(); + var project = Evaluate(home, NoTargetsProject + $"<{name}>true"); + await Assert.That(GetReferences(project)).IsEquivalentTo([package]); + var reference = project.GetItems("BV_PackageReference").Single(); + await Assert.That(reference.GetMetadataValue("PrivateAssets")).IsEqualTo("All"); + } + + [Test] + [Arguments(UseStyleCopAnalyzers, StyleCopAnalyzers)] + [Arguments(UsePublicApiAnalyzers, PublicApiAnalyzers)] + [Arguments(UseNullableExtendedAnalyzer, NullableExtendedAnalyzer)] + public async Task Evaluate_SwitchFalse_LeavesThePackageOut(string name, string package) + { + using var home = new TempHome(); + var project = Evaluate(home, LibraryProject + $"<{name}>false"); + await Assert.That(GetReferences(project)).DoesNotContain(package); + } + + [Test] + public async Task Evaluate_StyleCopJsonInTheHomeDirectory_AddsItAsAdditionalFile() + { + using var home = new TempHome(); + home.WriteFile("stylecop.json", "{}"); + var project = Evaluate(home, OrdinaryProject); + await Assert.That(GetAdditionalFiles(project)).IsEquivalentTo([(home.GetFullPath("stylecop.json"), "stylecop.json")]); + } + + [Test] + public async Task Evaluate_StyleCopJsonNearerTheProject_WinsOverTheOneInTheHomeDirectory() + { + using var home = new TempHome(); + home.WriteFile("stylecop.json", "{}"); + home.WriteFile($"{ProjectDirectory}/stylecop.json", "{}"); + var project = Evaluate(home, OrdinaryProject); + var expected = home.GetFullPath($"{ProjectDirectory}/stylecop.json"); + await Assert.That(GetAdditionalFiles(project)).IsEquivalentTo([(expected, "stylecop.json")]); + } + + [Test] + public async Task Evaluate_DotStyleCopJsonAlone_AddsItAsStyleCopJson() + { + using var home = new TempHome(); + home.WriteFile("src/.stylecop.json", "{}"); + var project = Evaluate(home, OrdinaryProject); + await Assert.That(GetAdditionalFiles(project)).IsEquivalentTo([(home.GetFullPath("src/.stylecop.json"), "stylecop.json")]); + } + + // The search for .stylecop.json starts only when no stylecop.json exists, however far up. + [Test] + public async Task Evaluate_StyleCopJsonAndNearerDotStyleCopJson_AddsStyleCopJson() + { + using var home = new TempHome(); + home.WriteFile("stylecop.json", "{}"); + home.WriteFile($"{ProjectDirectory}/.stylecop.json", "{}"); + var project = Evaluate(home, OrdinaryProject); + await Assert.That(GetAdditionalFiles(project)).IsEquivalentTo([(home.GetFullPath("stylecop.json"), "stylecop.json")]); + } + + // The home directory is the src subdirectory of the temporary directory, and the file is above it. + [Test] + [Arguments("stylecop.json")] + [Arguments(".stylecop.json")] + public async Task Evaluate_FileAboveTheHomeDirectory_AddsNothing(string fileName) + { + using var home = new TempHome(); + home.WriteFile(fileName, "{}"); + var properties = OrdinaryProject + $"{home.GetFullPath("src")}{Path.DirectorySeparatorChar}"; + var project = Evaluate(home, properties); + await Assert.That(project.GetPropertyValue("StyleCopJsonPath")).IsEmpty(); + await Assert.That(GetAdditionalFiles(project)).IsEmpty(); + } + + [Test] + public async Task Evaluate_StyleCopJsonListedByTheProject_AddsItOnceAsAdditionalFile() + { + using var home = new TempHome(); + home.WriteFile($"{ProjectDirectory}/stylecop.json", "{}"); + const string items = """ + + + """; + var project = Evaluate(home, OrdinaryProject, items); + var expected = home.GetFullPath($"{ProjectDirectory}/stylecop.json"); + await Assert.That(GetAdditionalFiles(project)).IsEquivalentTo([(expected, "stylecop.json")]); + await Assert.That(project.GetItems("None")).IsEmpty(); + } + + [Test] + public async Task Evaluate_UseStyleCopAnalyzersFalse_AddsNoStyleCopJson() + { + using var home = new TempHome(); + home.WriteFile("stylecop.json", "{}"); + var project = Evaluate(home, OrdinaryProject + "false"); + await Assert.That(GetAdditionalFiles(project)).IsEmpty(); + } + + // The project is in src/Test, and the home directory is the temporary directory, unless the properties set + // HomeDirectory. + private static ProjectInstance Evaluate(TempHome home, string properties, string items = "") + { + var projectPath = home.GetFullPath($"{ProjectDirectory}/Test.proj"); + var projectText = $""" + + + {home.RootPath}{Path.DirectorySeparatorChar} + {properties} + + + {items} + + + + """; + home.WriteFile($"{ProjectDirectory}/Test.proj", projectText); + + using var collection = new ProjectCollection(); + return ProjectInstance.FromFile(projectPath, new ProjectOptions { ProjectCollection = collection }); + } + + private static string[] GetReferences(ProjectInstance project) + => [.. project.GetItems("BV_PackageReference").Select(static item => item.EvaluatedInclude)]; + + // Each additional file as its full path and its Link metadata. + private static (string Path, string Link)[] GetAdditionalFiles(ProjectInstance project) + => [ + .. project.GetItems("AdditionalFiles") + .Select(static item => (item.GetMetadataValue("FullPath"), item.GetMetadataValue("Link"))), + ]; + + private static string GetRealTargetsPath() + => typeof(StandardAnalyzersModuleTargetsTests).Assembly + .GetCustomAttributes() + .First(static attribute => attribute.Key == "RealStandardAnalyzersModuleTargetsPath") + .Value!; +}