Fix formatting of attributes on lambdas#56639
Conversation
|
|
||
| // * [ | ||
| if (currentToken.IsKind(SyntaxKind.OpenBracketToken) && | ||
| !currentToken.Parent.IsKind(SyntaxKind.AttributeList) && |
There was a problem hiding this comment.
what happens if i have[Foo] [Bar] now? how did it format before, how does it format now? #Resolved
There was a problem hiding this comment.
Good question. Before and after behavior is that we leave whatever spaces you put there.
Should we make that 1 space?
There was a problem hiding this comment.
I went with 0 space as at least one existing test had that expectation.
There was a problem hiding this comment.
Yes. 0 spaces would be my expectation.
|
@CyrusNajmabadi @sharwell for another look. Thanks |
|
@CyrusNajmabadi for another look. Thanks |
|
|
||
| // [Attribute1]$$ [Attribute2] | ||
| if (currentToken.IsKind(SyntaxKind.OpenBracketToken) && | ||
| currentToken.Parent.IsKind(SyntaxKind.AttributeList)) |
There was a problem hiding this comment.
this case seems weird given the if check above this. we already checked if the owner was a member, and if so we force a space. it seems like we'd want to do the multiple-attribute check prior to doing the member check, otherwise we would not hit this for [Foo] [Bar] void SomeMember(), right?
There was a problem hiding this comment.
For methods, we introduce newlines between attributes.
But for fields, the order here matters. If I swap the order, then FormatMultipleAttributeOnSameLineAsField1 is affected (it expects [Attr3] [Attr4] int i; with 1 space.
So I think we should keep the order as-is.
There was a problem hiding this comment.
can you comment teh code explicitly to indicate this is intentional. note if we want fields to keep thigns on a single line, i'd far prefer that we do that explicitly. Note: for fields, i would expect no space between attributes as well :(
tagging @sharwell for thoughts here.
|
@CyrusNajmabadi @sharwell I modified existing behavior so we no longer have a space between attributes per suggestion above. |
| { | ||
| await AssertFormatAsync( | ||
| expected: @" | ||
| [Attribute][Attribute2] |
There was a problem hiding this comment.
❔ What did this do before your change? Just wondering if the output used to split them to their own lines (like SpacingAfterAttribute_MultipleOnDeclaration shows). #Resolved
There was a problem hiding this comment.
Prior to this PR, this would leave the formatting as-is (ie. [Attribute] [Attribute2] with 2 spaces)
Four tests were affected by this file by the product code change:
|
From discussion with Jinu, I'm pushing the bug and the fix from 17.0 to 17.1, so we don't have to rush to resolve this today. Thanks |
|
This sounds good to me:
As part of (3), I would prefer to see the formatter apply to attributes on generic type parameters (previously this case was ignored by the formatter), which gives consistency to parameters and type parameters. |
|
@CyrusNajmabadi for another look. Thanks |
Fixes #56543
Relates to test plan #52192