Page MenuHomePhabricator

[clang-format] Handle quotes and escaped braces in C# interpolated strings
ClosedPublic

Authored by jbcoe on Jan 27 2020, 9:35 AM.

Details

Summary

This addresses issues raised in https://bugs.llvm.org/show_bug.cgi?id=44454.

There are outstanding issues with multi-line verbatim strings in C# that will be addressed in a follow-up PR.

Diff Detail

Event Timeline

jbcoe created this revision.Jan 27 2020, 9:35 AM
jbcoe added a subscriber: MyDeveloperDay.
MyDeveloperDay accepted this revision.Jan 27 2020, 9:52 AM

This looks good, I had lots of problems when I first started the C# support around this which the initial C# support tried to address by adding support in the lexer, but we pulled back from that implementation. So its no surprise to me we are hitting a few errors in this area, one issue I've had problems with is a string ending with \ e.g.

@"ABC\" + x + "ABC"

this can confuse the tokenizer into thinking this is one string 'ABC" + x + '

But I think adding more support here to handle these cases is good. I'd like to see more test examples if possible

This revision is now accepted and ready to land.Jan 27 2020, 9:52 AM

Just for reference, take a look at D58404: [clang-format] Add basic support for formatting C# files especially the historical diff

https://reviews.llvm.org/D58404?vs=on&id=188239&whitespace=ignore-most#toc

in the end, we didn't land that Lex changes but incase we feel it needs to come back to provide full support for C# strings it might help a little

krasimir added inline comments.Jan 28 2020, 2:57 AM
clang/lib/Format/FormatTokenLexer.cpp
190

nit: start with an upper case: // Interpolated ...

229

Curious why do we need this check?

235

(Not needed to be addressed in this patch):
What if there was a newline between CSharpInterpolatedString and NextToken, like in:

string s = $"hello {@"la
     la" + name} world!";

I'm not sure if this is valid C#, and I'm not sure if verbatim C# strings with newlines are formatted well currently, however worth trying several such examples and seeing how they get laid out. The ColumnWidth computation might be incorrect and you might need to set FormatToken::IsMultiline.

clang/unittests/Format/FormatTestCSharp.cpp
424

Please also add a test where }} is skipped.

jbcoe updated this revision to Diff 240849.Jan 28 2020, 5:59 AM

Address review comments.

jbcoe marked 4 inline comments as done.Jan 28 2020, 6:00 AM
jbcoe added inline comments.
clang/lib/Format/FormatTokenLexer.cpp
229

Check for empty TokenText removed.

jbcoe marked an inline comment as done.Jan 28 2020, 6:01 AM
jbcoe marked an inline comment as done.
jbcoe added inline comments.
clang/lib/Format/FormatTokenLexer.cpp
235

Newlines in verbatim strings still cause problems. I'll look into fixing this in a follow-up PR.

krasimir accepted this revision.Jan 28 2020, 6:17 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJan 28 2020, 7:08 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript