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.

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

SA1316 should not be reported for tuple deconstruction #3878

Open
Arthri opened this issue Jul 23, 2024 · 7 comments
Open

SA1316 should not be reported for tuple deconstruction #3878

Arthri opened this issue Jul 23, 2024 · 7 comments

Comments

@Arthri
Copy link

Arthri commented Jul 23, 2024

Tuples can follow two different casing conventions depending on how it's used: tuple types may follow pascal casing, while deconstruction may follow camel casing

The rule does not differentiate between the two uses and indiscriminately flags both, so I propose having a separate rule for tuple deconstruction

// no diagnostic
(string K, string V) a = ("", "");
// SA1316
(string k, string v) = a;

// SA1316
foreach ((string k, string v) in dict)
{
}
@bjornhellander
Copy link
Contributor

I am not seeing this behavior. Which version are you using?

@bjornhellander
Copy link
Contributor

Have you set includeInferredTupleElementNames to true in your stylecop.json configuration file? I see the behavior that you describe when I do that.

@Arthri
Copy link
Author

Arthri commented Jul 24, 2024

Have you set includeInferredTupleElementNames to true in your stylecop.json configuration file? I see the behavior that you describe when I do that.

I have. It seems I'm misunderstanding what includeInferredTupleElementNames means. I thought by "inferred tuple names" it means x and y in var tuple = (x, y) as opposed to explicit tuple names var tuple = (X: x, Y: y)?

@bjornhellander
Copy link
Contributor

var tuple = (x, y) is equivalent to var tuple = (x: x, y: y),. Setting includeInferredTupleElementNames to true means that SA1316 will also check the implicit/inferred element names if they are omitted, like they are in var tuple = (x, y). So with the default PascalCase configuration, that code would also trigger a SA1316 warning.

@bjornhellander
Copy link
Contributor

It looks to me like you have found a bug. In my opinion, tuple expressions used in the "target" of a deconstruction should not be checked by this rule. @sharwell, do you agree? I am a bit surprised that this hasn't been found before, but includeInferredTupleElementNames is unfortunately false by default, so maybe it isn't used much.

@Arthri
Copy link
Author

Arthri commented Jul 24, 2024

I see, thanks for the clarification

@bjornhellander
Copy link
Contributor

It looks like setting includeInferredTupleElementNames to false doesn't work well either:
This rule does not even trigger on var tuple = (x: 1, Y: 2), which I assume it should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants