Replace AutoConstructor_DisableNullChecking with AutoConstructor_Gene…

…rateArgumentNullExceptionChecks

Fixes #109

README.md

@@ -174,26 +174,33 @@ partial class Test
 
 Get-only properties (`public int Property { get; }`) are injected by the generator by default.
 Non get-only properties (`public int Property { get; set;}`) are injected only if marked with (`[field: AutoConstructorInject]`) attributte.
-The behavior of the injection can be modified using [auto-implemented property field-targeted attributes](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-7.3/auto-prop-field-attrs) on its backing field. The following code show an injected get-only property with a custom injecter:
+The behavior of the injection can be modified using auto-implemented property field-targeted attributes on its backing field. The following code show an injected get-only property with a custom injecter:
 
 ```csharp
 [field: AutoConstructorInject(initializer: "injected.ToString()", injectedType: typeof(int), parameterName: "injected")]
 public int Property { get; }
 ```
 
-⚠️ The compiler support for auto-implemented property field-targeted attributes is not perfect (as described in the link above), and Roslyn analyzers are not running on backings fields so some warnings may not be reported.
+⚠️ The compiler support for auto-implemented property field-targeted attributes is not perfect, and Roslyn analyzers are not running on backings fields so some warnings may not be reported.
 
 ## Configuration
 
 ### Generating `ArgumentNullException`
 
 By default, null checks with `ArgumentNullException` are not generated when needed.
-To enable this behavior, set `AutoConstructor_DisableNullChecking` to `false` in the project file:
+
+To enable this behavior, set `AutoConstructor_GenerateArgumentNullExceptionChecks` to `true` in the project file:
 
 ``` xml
-<AutoConstructor_DisableNullChecking>false</AutoConstructor_DisableNullChecking>
+<AutoConstructor_GenerateArgumentNullExceptionChecks>true</AutoConstructor_GenerateArgumentNullExceptionChecks>
 ```
 
+<details>
+  <summary>5.2.X and previous versions</summary>
+
+  To enable this behavior, set `AutoConstructor_DisableNullChecking` to `false` in the project file.
+</details>
+
 ### Generating `this()` calls
 
 By default, if a parameterless constructor is available on the class (other than the implicit one), a call

src/AutoConstructor.Generator/AnalyzerReleases.Unshipped.md

@@ -6,3 +6,4 @@
 Rule ID | Category | Severity | Notes
 --------|----------|----------|-------
 ACONS11 | Usage | Error | DefaultBaseAttributeAnalyzer, [Documentation](https://github.com/k94ll13nn3/AutoConstructor#ACONS11)
+ACONS99 | Usage | Warning | AutoConstructorGenerator, [Documentation](https://github.com/k94ll13nn3/AutoConstructor#ACONS99)

src/AutoConstructor.Generator/AutoConstructor.props

@@ -5,6 +5,7 @@
     <CompilerVisibleProperty Include="AutoConstructor_GenerateConstructorDocumentation" />
     <CompilerVisibleProperty Include="AutoConstructor_ConstructorDocumentationComment" />
     <CompilerVisibleProperty Include="AutoConstructor_GenerateThisCalls" />
+    <CompilerVisibleProperty Include="AutoConstructor_GenerateArgumentNullExceptionChecks" />
   </ItemGroup>
 
 </Project>

src/AutoConstructor.Generator/AutoConstructorGenerator.cs

@@ -31,7 +31,17 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
             i.AddSource(Source.DefaultBaseAttributeFullName, SourceText.From(Source.DefaultBaseAttributeText, Encoding.UTF8));
         });
 
-        IncrementalValuesProvider<(GeneratorExectutionResult? result, Options options)> valueProvider = context.SyntaxProvider
+        // TODO: remove in v6
+        IncrementalValueProvider<bool> obsoleteOptionDiagnosticProvider = context.AnalyzerConfigOptionsProvider
+            .Select((c, _) =>
+            {
+                return
+                    c.GlobalOptions.TryGetValue($"build_property.{BuildProperties.AutoConstructor_DisableNullChecking}", out string? disableNullCheckingSwitch) &&
+                    !string.IsNullOrWhiteSpace(disableNullCheckingSwitch);
+            })
+            .WithTrackingName("obsoleteOptionDiagnosticProvider");
+
+        IncrementalValuesProvider<(GeneratorExectutionResult? result, Options options)> valuesProvider = context.SyntaxProvider
             .ForAttributeWithMetadataName(
                 Source.AttributeFullName,
                 static (node, _) => IsSyntaxTargetForGeneration(node),
@@ -41,7 +51,15 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
             .Combine(context.AnalyzerConfigOptionsProvider.Select((c, _) => ParseOptions(c.GlobalOptions)))
             .WithTrackingName("Combine");
 
-        context.RegisterSourceOutput(valueProvider, static (context, item) =>
+        context.RegisterSourceOutput(obsoleteOptionDiagnosticProvider, static (context, item) =>
+        {
+            if (item)
+            {
+                context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.DisableNullCheckingIsObsoleteRule, null));
+            }
+        });
+
+        context.RegisterSourceOutput(valuesProvider, static (context, item) =>
         {
             if (item.result is not null)
             {
@@ -67,21 +85,27 @@ private static bool IsSyntaxTargetForGeneration(SyntaxNode node)
     private static Options ParseOptions(AnalyzerConfigOptions analyzerOptions)
     {
         bool generateConstructorDocumentation = false;
-        if (analyzerOptions.TryGetValue("build_property.AutoConstructor_GenerateConstructorDocumentation", out string? generateConstructorDocumentationSwitch))
+        if (analyzerOptions.TryGetValue($"build_property.{BuildProperties.AutoConstructor_GenerateConstructorDocumentation}", out string? generateConstructorDocumentationSwitch))
         {
             generateConstructorDocumentation = generateConstructorDocumentationSwitch.Equals("true", StringComparison.OrdinalIgnoreCase);
         }
 
-        analyzerOptions.TryGetValue("build_property.AutoConstructor_ConstructorDocumentationComment", out string? constructorDocumentationComment);
+        analyzerOptions.TryGetValue($"build_property.{BuildProperties.AutoConstructor_ConstructorDocumentationComment}", out string? constructorDocumentationComment);
 
+        // TODO: remove in v6
         bool emitNullChecks = false;
-        if (analyzerOptions.TryGetValue("build_property.AutoConstructor_DisableNullChecking", out string? disableNullCheckingSwitch))
+        if (analyzerOptions.TryGetValue($"build_property.{BuildProperties.AutoConstructor_DisableNullChecking}", out string? disableNullCheckingSwitch))
         {
             emitNullChecks = disableNullCheckingSwitch.Equals("false", StringComparison.OrdinalIgnoreCase);
         }
 
+        if (analyzerOptions.TryGetValue($"build_property.{BuildProperties.AutoConstructor_GenerateArgumentNullExceptionChecks}", out string? generateArgumentNullExceptionChecksSwitch))
+        {
+            emitNullChecks = generateArgumentNullExceptionChecksSwitch.Equals("true", StringComparison.OrdinalIgnoreCase);
+        }
+
         bool emitThisCalls = true;
-        if (analyzerOptions.TryGetValue("build_property.AutoConstructor_GenerateThisCalls", out string? enableThisCallsSwitch))
+        if (analyzerOptions.TryGetValue($"build_property.{BuildProperties.AutoConstructor_GenerateThisCalls}", out string? enableThisCallsSwitch))
         {
             emitThisCalls = !enableThisCallsSwitch.Equals("false", StringComparison.OrdinalIgnoreCase);
         }

src/AutoConstructor.Generator/BuildProperties.cs

@@ -0,0 +1,10 @@
+namespace AutoConstructor.Generator;
+
+internal static class BuildProperties
+{
+    public const string AutoConstructor_GenerateConstructorDocumentation = nameof(AutoConstructor_GenerateConstructorDocumentation);
+    public const string AutoConstructor_ConstructorDocumentationComment = nameof(AutoConstructor_ConstructorDocumentationComment);
+    public const string AutoConstructor_DisableNullChecking = nameof(AutoConstructor_DisableNullChecking);
+    public const string AutoConstructor_GenerateThisCalls = nameof(AutoConstructor_GenerateThisCalls);
+    public const string AutoConstructor_GenerateArgumentNullExceptionChecks = nameof(AutoConstructor_GenerateArgumentNullExceptionChecks);
+}

src/AutoConstructor.Generator/DiagnosticDescriptors.cs

@@ -26,6 +26,9 @@ public static class DiagnosticDescriptors
 
     public const string MultipleDefaultBaseDiagnosticId = "ACONS11";
 
+    // TODO: remove in v6
+    public const string DisableNullCheckingIsObsoleteDiagnosticId = "ACONS99";
+
     public static readonly DiagnosticDescriptor TypeWithoutPartialRule = new(
         TypeWithoutPartialDiagnosticId,
         "Couldn't generate constructor",
@@ -146,4 +149,15 @@ public static class DiagnosticDescriptors
         null,
         $"https://github.com/k94ll13nn3/AutoConstructor#{MultipleDefaultBaseDiagnosticId}",
         WellKnownDiagnosticTags.Build);
+
+    public static readonly DiagnosticDescriptor DisableNullCheckingIsObsoleteRule = new(
+        DisableNullCheckingIsObsoleteDiagnosticId,
+        "Replace AutoConstructor_DisableNullChecking with AutoConstructor_GenerateArgumentNullExceptionChecks",
+        "AutoConstructor_DisableNullChecking is obsolete and will be removed in a future version, use AutoConstructor_GenerateArgumentNullExceptionChecks instead",
+        "Usage",
+        DiagnosticSeverity.Warning,
+        true,
+        null,
+        $"https://github.com/k94ll13nn3/AutoConstructor#{DisableNullCheckingIsObsoleteDiagnosticId}",
+        WellKnownDiagnosticTags.Unnecessary);
 }

tests/AutoConstructor.Tests/GeneratorTests.cs

@@ -379,9 +379,41 @@ public Test(int t)
     }
 
     [Theory]
+    [InlineData(null)]
+    [InlineData(true)]
+    [InlineData(false)]
+    public async Task Run_WithMsbuildConfigNullChecks_ShouldGenerateClassAndReportDiagnostic(bool? disableNullChecks)
+    {
+        const string code = @"
+namespace Test
+{
+    [AutoConstructor]
+    internal partial class Test
+    {
+        private readonly string _t;
+    }
+}";
+        string generated = $@"namespace Test
+{{
+    partial class Test
+    {{
+        public Test(string t)
+        {{
+            this._t = t{(disableNullChecks is false ? " ?? throw new System.ArgumentNullException(nameof(t))" : "")};
+        }}
+    }}
+}}
+";
+        DiagnosticResult diagnosticResult = new(DiagnosticDescriptors.DisableNullCheckingIsObsoleteDiagnosticId, DiagnosticSeverity.Warning);
+        string? configFileContent = disableNullChecks is null ? null : $"build_property.AutoConstructor_DisableNullChecking = {disableNullChecks}";
+        await VerifySourceGenerator.RunAsync(code, generated, diagnostics: disableNullChecks is null ? null : [diagnosticResult], configFileContent: configFileContent, runWithNullChecks: false);
+    }
+
+    [Theory]
+    [InlineData(null)]
     [InlineData(true)]
     [InlineData(false)]
-    public async Task Run_WithMsbuildConfigNullChecks_ShouldGenerateClass(bool disableNullChecks)
+    public async Task Run_WithMsbuildConfigGenerateArgumentNullExceptionChecks_ShouldGenerateClass(bool? generateNullChecks)
     {
         const string code = @"
 namespace Test
@@ -398,13 +430,47 @@ partial class Test
     {{
         public Test(string t)
         {{
-            this._t = t{(!disableNullChecks ? " ?? throw new System.ArgumentNullException(nameof(t))" : "")};
+            this._t = t{(generateNullChecks is true ? " ?? throw new System.ArgumentNullException(nameof(t))" : "")};
         }}
     }}
 }}
 ";
 
-        await VerifySourceGenerator.RunAsync(code, generated, configFileContent: $"build_property.AutoConstructor_DisableNullChecking = {disableNullChecks}");
+        string? configFileContent = generateNullChecks is null ? null : $"build_property.AutoConstructor_GenerateArgumentNullExceptionChecks = {generateNullChecks}";
+        await VerifySourceGenerator.RunAsync(code, generated, configFileContent: configFileContent, runWithNullChecks: false);
+    }
+
+    [Fact]
+    public async Task Run_WithBothMsbuildConfigForNullChecks_ShouldGenerateClassWithNewOneTakingPrecedenceAndReportDiagnostic()
+    {
+        const string code = @"
+namespace Test
+{
+    [AutoConstructor]
+    internal partial class Test
+    {
+        private readonly string _t;
+    }
+}";
+        const string generated = @"namespace Test
+{
+    partial class Test
+    {
+        public Test(string t)
+        {
+            this._t = t;
+        }
+    }
+}
+";
+
+        const string configFileContent = """
+build_property.AutoConstructor_DisableNullChecking = false
+build_property.AutoConstructor_GenerateArgumentNullExceptionChecks = false
+""";
+
+        DiagnosticResult diagnosticResult = new(DiagnosticDescriptors.DisableNullCheckingIsObsoleteDiagnosticId, DiagnosticSeverity.Warning);
+        await VerifySourceGenerator.RunAsync(code, generated, diagnostics: [diagnosticResult], configFileContent: configFileContent, runWithNullChecks: false);
     }
 
     [Theory]

tests/AutoConstructor.Tests/IncrementalGeneratorTests.cs

@@ -128,7 +128,8 @@ public void CheckGeneratorIsIncremental(
         IEnumerable<(object Value, IncrementalStepRunReason Reason)> sourceOuputs =
             result.TrackedOutputSteps.SelectMany(outputStep => outputStep.Value).SelectMany(output => output.Outputs);
 
-        (_, IncrementalStepRunReason Reason) = Assert.Single(sourceOuputs);
+        // TODO: remove skip in v6
+        (_, IncrementalStepRunReason Reason) = Assert.Single(sourceOuputs.Skip(1));
         Assert.Equal(sourceStepReason, Reason);
         Assert.Equal(executeStepReason, result.TrackedSteps["Execute"].Single().Outputs[0].Reason);
         Assert.Equal(combineStepReason, result.TrackedSteps["Combine"].Single().Outputs[0].Reason);

tests/AutoConstructor.Tests/Verifiers/CSharpSourceGeneratorVerifier`1.cs

@@ -19,9 +19,10 @@ public static Task RunAsync(
         bool nullable = false,
         IEnumerable<DiagnosticResult>? diagnostics = null,
         string? configFileContent = null,
-        string? additionalProjectsSource = null)
+        string? additionalProjectsSource = null,
+        bool runWithNullChecks = true)
     {
-        return RunAsync(code, new[] { (generated, generatedName) }, nullable, diagnostics, configFileContent, additionalProjectsSource);
+        return RunAsync(code, new[] { (generated, generatedName) }, nullable, diagnostics, configFileContent, additionalProjectsSource, runWithNullChecks);
     }
 
     public static async Task RunAsync(
@@ -30,7 +31,8 @@ public static async Task RunAsync(
         bool nullable = false,
         IEnumerable<DiagnosticResult>? diagnostics = null,
         string? configFileContent = null,
-        string? additionalProjectsSource = null)
+        string? additionalProjectsSource = null,
+        bool runWithNullChecks = true)
     {
         var test = new CSharpSourceGeneratorVerifier<TSourceGenerator>.Test()
         {
@@ -88,9 +90,16 @@ public static async Task RunAsync(
         }
 
         // Enable null checks for the tests.
-        test.TestState.AnalyzerConfigFiles.Add(("/.editorconfig", SourceText.From($@"
+        if (runWithNullChecks)
+        {
+            configFileContent = $@"
+build_property.{BuildProperties.AutoConstructor_GenerateArgumentNullExceptionChecks} = true
+{configFileContent}
+";
+        }
+
+        test.TestState.AnalyzerConfigFiles.Add(("/.globalconfig", SourceText.From($@"
 is_global=true
-build_property.AutoConstructor_DisableNullChecking = false
 {configFileContent}")));
 
         await test.RunAsync();