Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for destructured records #7

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add support for destructured records #7

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
99bdc8d
add support for destructured records
rfvgyhn Dec 7, 2023
ba051a6
add unit tests
rfvgyhn Dec 7, 2023
82b637b
make new constructors internal
rfvgyhn Jun 6, 2024
902d824
add negative test for matching against base type
rfvgyhn Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchExpressionSuppressorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of the tests are the wrong way around. The test should define a hierarchy where Leaf1 has a object v and then test that a match agains Leaf1(string v) is not exhaustive.

The fact that the current wrong tests succeed is due to a missing feature implementation in the IsDeclarationPatternNonRestrictive method. The method currently expects containingSubpatternSyntax.NameColon to be present, whereas with deconstruction it is not.

Please fix the tests and add the implementation accordingly.

The positive tests (that assert suppression) happen to succeed correctly because they use var. Had you written them with object v, they would have failed as well and revealed the missing implementation.

{
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()
Expand Down
26 changes: 24 additions & 2 deletions ClosedTypeHierarchyDiagnosticSuppressor.Tests/SwitchStatementSuppressorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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()
{
Expand Down
2 changes: 1 addition & 1 deletion ClosedTypeHierarchyDiagnosticSuppressor.Tests/TypeHierarchies.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
}
";
Expand Down