From 99bdc8d76ada2d8dcb973ce04b301e625989f789 Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 6 Dec 2023 20:49:50 -0700 Subject: [PATCH 1/4] add support for destructured records --- ClosedTypeHierarchyDiagnosticSuppressor/PatternHelper.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ClosedTypeHierarchyDiagnosticSuppressor/PatternHelper.cs b/ClosedTypeHierarchyDiagnosticSuppressor/PatternHelper.cs index 16dd2b6..0c53079 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor/PatternHelper.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor/PatternHelper.cs @@ -51,7 +51,8 @@ bool IsSubpatternNonRestrictive(SubpatternSyntax subpatternSyntax) => }; bool IsRecursivePatternNonRestrictive(RecursivePatternSyntax recursivePatternSyntax) => - recursivePatternSyntax.PropertyPatternClause?.Subpatterns.All(IsSubpatternNonRestrictive) ?? false; + (recursivePatternSyntax.PropertyPatternClause?.Subpatterns ?? recursivePatternSyntax.PositionalPatternClause?.Subpatterns) + ?.All(IsSubpatternNonRestrictive) ?? false; bool IsDeclarationPatternNonRestrictive(DeclarationPatternSyntax declarationPatternSyntax, SubpatternSyntax containingSubpatternSyntax) { From ba051a6db6eeda1bd3562d2bb8512188bb078dcd Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 6 Dec 2023 20:52:19 -0700 Subject: [PATCH 2/4] add unit tests --- .../SwitchExpressionSuppressorTests.cs | 24 +++++++++++++++-- .../SwitchStatementSuppressorTests.cs | 26 +++++++++++++++++-- .../TypeHierarchies.cs | 8 ++++++ .../SwitchExpressionSuppressor.cs | 11 +++++++- .../SwitchStatementSuppressor.cs | 11 +++++++- 5 files changed, 74 insertions(+), 6 deletions(-) diff --git a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchExpressionSuppressorTests.cs b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchExpressionSuppressorTests.cs index 9ad05db..330d3f5 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchExpressionSuppressorTests.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchExpressionSuppressorTests.cs @@ -20,9 +20,9 @@ Task EnsureNotSuppressed(string code, NullableContextOptions nullableContextOpti nullableContextOptions, ("IDE0072", IDE0072Analyzer)); - Task EnsureSuppressed(string code, NullableContextOptions nullableContextOptions) => + Task EnsureSuppressed(string code, NullableContextOptions nullableContextOptions, bool allowRecords = false) => DiagnosticSuppressorAnalyer.EnsureSuppressed( - new SwitchExpressionSuppressor(), + new SwitchExpressionSuppressor(allowRecords), SwitchExpressionSuppressor.SuppressionDescriptorByDiagnosticId.Values, code, nullableContextOptions, @@ -87,6 +87,26 @@ public static int DoSwitch(Root root) return EnsureNotSuppressed(code, NullableContextOptions.Disable); } + + [Test] + public Task When_record_with_destructure_And_all_subtypes_match_Then_suppress() + { + var code = CodeHelper.WrapInNamespace(TypeHierarchies.ProtectedCopyConstructorOnly.PositionalParameter + @" +static class SwitchTest +{ + public static int DoSwitch(Root root) + { + return root switch + { + Root.Leaf1(var v) => 0, + Root.Leaf2 => 1, + }; + } +} +"); + + return EnsureSuppressed(code, NullableContextOptions.Enable, allowRecords: true); + } [Test] public Task When_nullable_is_disabled_And_null_is_matched_on_its_own_Then_suppress() diff --git a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchStatementSuppressorTests.cs b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchStatementSuppressorTests.cs index 6a21293..bcf6b24 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchStatementSuppressorTests.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchStatementSuppressorTests.cs @@ -20,9 +20,9 @@ Task EnsureNotSuppressed(string code, NullableContextOptions nullableContextOpti nullableContextOptions, ("IDE0010", IDE0010Analyzer)); - Task EnsureSuppressed(string code, NullableContextOptions nullableContextOptions) => + Task EnsureSuppressed(string code, NullableContextOptions nullableContextOptions, bool allowRecords = false) => DiagnosticSuppressorAnalyer.EnsureSuppressed( - new SwitchStatementSuppressor(), + new SwitchStatementSuppressor(allowRecords), SwitchStatementSuppressor.SuppressionDescriptorByDiagnosticId.Values, code, nullableContextOptions, @@ -94,6 +94,28 @@ public static void DoSwitch(Root root) return EnsureNotSuppressed(code, NullableContextOptions.Disable); } + [Test] + public Task When_record_with_destructure_And_all_subtypes_match_Then_suppress() + { + var code = CodeHelper.WrapInNamespace(TypeHierarchies.ProtectedCopyConstructorOnly.PositionalParameter + @" +static class SwitchTest +{ + public static void DoSwitch(Root root) + { + switch(root) + { + case Root.Leaf1(var v): + break; + case Root.Leaf2: + break; + } + } +} +"); + + return EnsureSuppressed(code, NullableContextOptions.Enable, allowRecords: true); + } + [Test] public Task When_nullable_is_disabled_And_null_is_matched_on_its_own_Then_suppress() { diff --git a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/TypeHierarchies.cs b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/TypeHierarchies.cs index 371b9bb..17a4f17 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/TypeHierarchies.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/TypeHierarchies.cs @@ -49,6 +49,14 @@ abstract record Root public sealed record Leaf1 : Root {} public sealed record Leaf2 : Root {} } +"; + public const string PositionalParameter = @" +abstract record Root +{ + Root() {} + public sealed record Leaf1(int Value) : Root {} + public sealed record Leaf2 : Root {} +} "; } diff --git a/ClosedTypeHierarchyDiagnosticSuppressor/SwitchExpressionSuppressor.cs b/ClosedTypeHierarchyDiagnosticSuppressor/SwitchExpressionSuppressor.cs index 8cc8ed7..bdba488 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor/SwitchExpressionSuppressor.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor/SwitchExpressionSuppressor.cs @@ -13,6 +13,15 @@ namespace SvSoft.Analyzers.ClosedTypeHerarchyDiagnosticSuppression; [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class SwitchExpressionSuppressor : DiagnosticSuppressor { + readonly bool _forceAllowRecords; + + public SwitchExpressionSuppressor() : this(false) { } + + public SwitchExpressionSuppressor(bool forceAllowRecords) + { + _forceAllowRecords = forceAllowRecords; + } + static readonly string[] SuppressedDiagnosticIds = { "CS8509", "IDE0072" }; public static readonly IReadOnlyDictionary SuppressionDescriptorByDiagnosticId = SuppressedDiagnosticIds.ToDictionary( @@ -42,7 +51,7 @@ void HandleDiagnostic(Diagnostic diagnostic) return; } - bool allowRecords = context.AreRecordHierarchiesAllowed(node.SyntaxTree); + bool allowRecords = _forceAllowRecords || context.AreRecordHierarchiesAllowed(node.SyntaxTree); var switchExpression = node.DescendantNodesAndSelf().OfType().FirstOrDefault(); diff --git a/ClosedTypeHierarchyDiagnosticSuppressor/SwitchStatementSuppressor.cs b/ClosedTypeHierarchyDiagnosticSuppressor/SwitchStatementSuppressor.cs index 6a02d24..c091196 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor/SwitchStatementSuppressor.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor/SwitchStatementSuppressor.cs @@ -12,6 +12,15 @@ namespace SvSoft.Analyzers.ClosedTypeHerarchyDiagnosticSuppression; [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class SwitchStatementSuppressor : DiagnosticSuppressor { + readonly bool _forceAllowRecords; + + public SwitchStatementSuppressor() : this(false) { } + + public SwitchStatementSuppressor(bool forceAllowRecords) + { + _forceAllowRecords = forceAllowRecords; + } + static readonly string[] SuppressedDiagnosticIds = { "IDE0010" }; public static readonly IReadOnlyDictionary SuppressionDescriptorByDiagnosticId = SuppressedDiagnosticIds.ToDictionary( @@ -40,7 +49,7 @@ void HandleDiagnostic(Diagnostic diagnostic) return; } - bool allowRecords = context.AreRecordHierarchiesAllowed(node.SyntaxTree); + bool allowRecords = _forceAllowRecords || context.AreRecordHierarchiesAllowed(node.SyntaxTree); ExpressionSyntax switchee = switchStatement.Expression; var switcheeModel = context.GetSemanticModel(switchee.SyntaxTree); From 82b637b9888ac7a1c7c264f0be58e0c0c3b04020 Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 6 Jun 2024 13:54:05 -0600 Subject: [PATCH 3/4] make new constructors internal --- .../ClosedTypeHierarchyDiagnosticSuppressor.csproj | 4 ++++ .../SwitchExpressionSuppressor.cs | 3 ++- .../SwitchStatementSuppressor.cs | 3 ++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ClosedTypeHierarchyDiagnosticSuppressor/ClosedTypeHierarchyDiagnosticSuppressor.csproj b/ClosedTypeHierarchyDiagnosticSuppressor/ClosedTypeHierarchyDiagnosticSuppressor.csproj index d73e933..8a53af3 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor/ClosedTypeHierarchyDiagnosticSuppressor.csproj +++ b/ClosedTypeHierarchyDiagnosticSuppressor/ClosedTypeHierarchyDiagnosticSuppressor.csproj @@ -18,6 +18,10 @@ Suppresses exhaustiveness warnings of switch statement and switch expressions when all cases of a "closed type hierarchy" are covered. See project URL for details. It suppresses IDE0010, IDE0072 and CS8509. It is NRT-aware. + + + + diff --git a/ClosedTypeHierarchyDiagnosticSuppressor/SwitchExpressionSuppressor.cs b/ClosedTypeHierarchyDiagnosticSuppressor/SwitchExpressionSuppressor.cs index bdba488..43cffcf 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor/SwitchExpressionSuppressor.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor/SwitchExpressionSuppressor.cs @@ -17,7 +17,8 @@ public sealed class SwitchExpressionSuppressor : DiagnosticSuppressor public SwitchExpressionSuppressor() : this(false) { } - public SwitchExpressionSuppressor(bool forceAllowRecords) + /// Constructor to facilitate unit testing + internal SwitchExpressionSuppressor(bool forceAllowRecords) { _forceAllowRecords = forceAllowRecords; } diff --git a/ClosedTypeHierarchyDiagnosticSuppressor/SwitchStatementSuppressor.cs b/ClosedTypeHierarchyDiagnosticSuppressor/SwitchStatementSuppressor.cs index c091196..eba05a7 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor/SwitchStatementSuppressor.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor/SwitchStatementSuppressor.cs @@ -16,7 +16,8 @@ public sealed class SwitchStatementSuppressor : DiagnosticSuppressor public SwitchStatementSuppressor() : this(false) { } - public SwitchStatementSuppressor(bool forceAllowRecords) + /// Constructor to facilitate unit testing + internal SwitchStatementSuppressor(bool forceAllowRecords) { _forceAllowRecords = forceAllowRecords; } From 902d8240de9211c34ee3f2e50d6102b79b17a3b1 Mon Sep 17 00:00:00 2001 From: Chris Date: Mon, 17 Jun 2024 15:02:06 -0600 Subject: [PATCH 4/4] add negative test for matching against base type --- .../SwitchExpressionSuppressorTests.cs | 24 +++++++++++++++-- .../SwitchStatementSuppressorTests.cs | 26 +++++++++++++++++-- .../TypeHierarchies.cs | 2 +- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchExpressionSuppressorTests.cs b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchExpressionSuppressorTests.cs index 330d3f5..c1c5711 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchExpressionSuppressorTests.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchExpressionSuppressorTests.cs @@ -13,9 +13,9 @@ class SwitchExpressionSuppressorTests "Microsoft.CodeAnalysis.CSharp.PopulateSwitch.CSharpPopulateSwitchExpressionDiagnosticAnalyzer")?.Unwrap() ?? throw new InvalidOperationException("could not instantiate populate switch expression analyzer for IDE0072")); - Task EnsureNotSuppressed(string code, NullableContextOptions nullableContextOptions) => + Task EnsureNotSuppressed(string code, NullableContextOptions nullableContextOptions, bool allowRecords = false) => DiagnosticSuppressorAnalyer.EnsureNotSuppressed( - new SwitchExpressionSuppressor(), + new SwitchExpressionSuppressor(allowRecords), code, nullableContextOptions, ("IDE0072", IDE0072Analyzer)); @@ -107,6 +107,26 @@ public static int DoSwitch(Root root) return EnsureSuppressed(code, NullableContextOptions.Enable, allowRecords: true); } + + [Test] + public Task When_record_with_destructure_And_only_base_type_is_matched_Then_do_not_suppress() + { + var code = CodeHelper.WrapInNamespace(TypeHierarchies.ProtectedCopyConstructorOnly.PositionalParameter + @" +static class SwitchTest +{ + public static int DoSwitch(Root root) + { + return root switch + { + Root.Leaf1(object v) => 0, + Root.Leaf2 => 1, + }; + } +} +"); + + return EnsureNotSuppressed(code, NullableContextOptions.Enable, allowRecords: true); + } [Test] public Task When_nullable_is_disabled_And_null_is_matched_on_its_own_Then_suppress() diff --git a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchStatementSuppressorTests.cs b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchStatementSuppressorTests.cs index bcf6b24..1f40dae 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchStatementSuppressorTests.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchStatementSuppressorTests.cs @@ -13,9 +13,9 @@ class SwitchStatementSuppressorTests "Microsoft.CodeAnalysis.CSharp.PopulateSwitch.CSharpPopulateSwitchStatementDiagnosticAnalyzer")?.Unwrap() ?? throw new InvalidOperationException("could not instantiate populate switch statement analyzer for IDE0072")); - Task EnsureNotSuppressed(string code, NullableContextOptions nullableContextOptions) => + Task EnsureNotSuppressed(string code, NullableContextOptions nullableContextOptions, bool allowRecords = false) => DiagnosticSuppressorAnalyer.EnsureNotSuppressed( - new SwitchStatementSuppressor(), + new SwitchStatementSuppressor(allowRecords), code, nullableContextOptions, ("IDE0010", IDE0010Analyzer)); @@ -116,6 +116,28 @@ public static void DoSwitch(Root root) return EnsureSuppressed(code, NullableContextOptions.Enable, allowRecords: true); } + [Test] + public Task When_record_with_destructure_And_only_base_type_is_matched_Then_do_not_suppress() + { + var code = CodeHelper.WrapInNamespace(TypeHierarchies.ProtectedCopyConstructorOnly.PositionalParameter + @" +static class SwitchTest +{ + public static void DoSwitch(Root root) + { + switch(root) + { + case Root.Leaf1(object v): + break; + case Root.Leaf2: + break; + }; + } +} +"); + + return EnsureNotSuppressed(code, NullableContextOptions.Enable, allowRecords: true); + } + [Test] public Task When_nullable_is_disabled_And_null_is_matched_on_its_own_Then_suppress() { diff --git a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/TypeHierarchies.cs b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/TypeHierarchies.cs index 17a4f17..07b9246 100644 --- a/ClosedTypeHierarchyDiagnosticSuppressor.Tests/TypeHierarchies.cs +++ b/ClosedTypeHierarchyDiagnosticSuppressor.Tests/TypeHierarchies.cs @@ -54,7 +54,7 @@ public sealed record Leaf2 : Root {} abstract record Root { Root() {} - public sealed record Leaf1(int Value) : Root {} + public sealed record Leaf1(string Value) : Root {} public sealed record Leaf2 : Root {} } ";