Page MenuHomePhabricator

[clang-format] Improve handling of C# attributes
ClosedPublic

Authored by jbcoe on Feb 7 2020, 4:12 PM.

Details

Summary

C# attributes can appear on classes and methods, in which case they should go on their own line, or on method parameters in which case
they should be left inline.

Diff Detail

Event Timeline

jbcoe created this revision.Feb 7 2020, 4:12 PM
jbcoe marked an inline comment as done.
jbcoe added inline comments.
clang/lib/Format/TokenAnnotator.cpp
3283

I removed this as it's too aggressive and introduces line breaks when attributes appear on method parameters.

MyDeveloperDay accepted this revision.Feb 9 2020, 8:47 AM

LGTM, thanks

This revision is now accepted and ready to land.Feb 9 2020, 8:47 AM
krasimir accepted this revision.Feb 11 2020, 3:32 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2020, 4:03 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

This issue seems to have caused a regression https://bugs.llvm.org/show_bug.cgi?id=50515, I understand we removed the rule because it was too aggressive, now its not aggressive enough.

I'm going to try and find some sort of compromise so we can gravitate to a solution that can work for both.

The second property in a class is putting the property on the same line

[XmlIgnore] 
public string property1 { get; set; }

[XmlIgnore] public string property2{ get; set; }

as are all successive properties

Multiple properites are also not getting broken unless they are the first property

[XmlIgnore] 
[ScriptIgnore] 
public string property1 { get; set; }

[XmlIgnore] [ScriptIgnore] public string property2{ get; set; }

Ideally we want this all the time.

[XmlIgnore] 
[ScriptIgnore] 
public string Url { get; set; }

In the original change @jbcoe I think you were trying to fix the case where void MethodA([In][Out] ref... got broken, I presume into

void MethodA([In]
   [Out] ref

Part of the problem although not a solution was that the [ of the [Out] isn't even actually seen as an TT_AttributeSquare but instead as a TT_ArraySubscriptLSquare

AnnotatedTokens(L=1):
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=void L=4 PPK=2 FakeLParens= FakeRParens=0 II=0x1c0f418 Text='void'
 M=0 C=1 T=StartOfName S=1 F=0 B=0 BK=0 P=1020 Name=identifier L=15 PPK=2 FakeLParens= FakeRParens=0 II=0x1c38cb0 Text='myFunction'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=l_paren L=16 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=1 T=AttributeSquare S=0 F=0 B=0 BK=0 P=140 Name=l_square L=17 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='['
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=identifier L=19 PPK=2 FakeLParens= FakeRParens=0 II=0x1c38cd0 Text='In'
 M=0 C=0 T=AttributeSquare S=0 F=0 B=0 BK=0 P=63 Name=r_square L=20 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=']'
 M=0 C=0 T=ArraySubscriptLSquare S=0 F=0 B=0 BK=0 P=240 Name=l_square L=21 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='['
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=259 Name=identifier L=24 PPK=2 FakeLParens= FakeRParens=0 II=0x1c38cf0 Text='Out'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=243 Name=r_square L=25 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=']'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=43 Name=identifier L=27 PPK=2 FakeLParens= FakeRParens=0 II=0x1c38d10 Text='a'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=28 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'

That I think you move the handling of the newlines to the parsing rather than in the canBreakBefore/mustBreakBefore (i.e. you wrap the lines when you see it on the function)

But of course a property isn't a function and so this is why its not being handled correctly. Going from the overly aggressive to the the parsing model regressed the behaviour for properties with attributes (which is very common if you use Newtonsoft.Json)

See D103307: [clang-format] successive C# attributes cause line breaking issues