[PATCH] D148473: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG799b794d7699: [clang-format] C# short ternary operator
misinterpreted as a CSharpNullable (authored by MyDeveloperDay).
Changed prior to commit:
https://reviews.llvm.org/D148473?vs=514343=514573#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148473/new/
https://reviews.llvm.org/D148473
Files:
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp
Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1724,6 +1724,34 @@
EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_ConditionLParen);
}
+TEST_F(TokenAnnotatorTest, CSharpNullableTypes) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+
+ auto Tokens = annotate("int? a;", Style);
+ EXPECT_EQ(Tokens.size(), 5u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int? a = 1;", Style);
+ EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?)", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?>", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("cond? id : id2", Style);
+ EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_ConditionalExpr);
+
+ Tokens = annotate("cond ? cond2 ? : id1 : id2", Style);
+ EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_ConditionalExpr);
+}
+
} // namespace
} // namespace format
} // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1312,11 +1312,34 @@
if (Style.isCSharp()) {
// `Type?)`, `Type?>`, `Type? name;` and `Type? name =` can only be
// nullable types.
+
+// `Type?)`, `Type?>`, `Type? name;`
+if (Tok->Next &&
+(Tok->Next->startsSequence(tok::question, tok::r_paren) ||
+ Tok->Next->startsSequence(tok::question, tok::greater) ||
+ Tok->Next->startsSequence(tok::question, tok::identifier,
+ tok::semi))) {
+ Tok->setType(TT_CSharpNullable);
+ break;
+}
+
+// `Type? name =`
+if (Tok->Next && Tok->Next->is(tok::identifier) && Tok->Next->Next &&
+Tok->Next->Next->is(tok::equal)) {
+ Tok->setType(TT_CSharpNullable);
+ break;
+}
+
// Line.MustBeDeclaration will be true for `Type? name;`.
-if ((!Contexts.back().IsExpression && Line.MustBeDeclaration) ||
-(Tok->Next && Tok->Next->isOneOf(tok::r_paren, tok::greater)) ||
-(Tok->Next && Tok->Next->is(tok::identifier) && Tok->Next->Next &&
- Tok->Next->Next->is(tok::equal))) {
+// But not
+// cond ? "A" : "B";
+// cond ? id : "B";
+// cond ? cond2 ? "A" : "B" : "C";
+if (!Contexts.back().IsExpression && Line.MustBeDeclaration &&
+(!Tok->Next ||
+ !Tok->Next->isOneOf(tok::identifier, tok::string_literal) ||
+ !Tok->Next->Next ||
+ !Tok->Next->Next->isOneOf(tok::colon, tok::question))) {
Tok->setType(TT_CSharpNullable);
break;
}
Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1724,6 +1724,34 @@
EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_ConditionLParen);
}
+TEST_F(TokenAnnotatorTest, CSharpNullableTypes) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+
+ auto Tokens = annotate("int? a;", Style);
+ EXPECT_EQ(Tokens.size(), 5u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int? a = 1;", Style);
+ EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?)", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?>", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("cond? id : id2", Style);
+ EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_ConditionalExpr);
+
+
[PATCH] D148473: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable
owenpan accepted this revision.
owenpan added inline comments.
Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:1730
+
+ auto Tokens = annotate("int? a;\n", Style);
+ EXPECT_EQ(Tokens.size(), 5u) << Tokens;
Nit: no trailing newlines here and below.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148473/new/
https://reviews.llvm.org/D148473
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148473: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable
MyDeveloperDay updated this revision to Diff 514343.
MyDeveloperDay added a comment.
simplify negative if
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148473/new/
https://reviews.llvm.org/D148473
Files:
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp
Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1724,6 +1724,34 @@
EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_ConditionLParen);
}
+TEST_F(TokenAnnotatorTest, CSharpNullableTypes) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+
+ auto Tokens = annotate("int? a;\n", Style);
+ EXPECT_EQ(Tokens.size(), 5u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int? a = 1;\n", Style);
+ EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?)\n", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?>\n", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("cond? id : id2\n", Style);
+ EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_ConditionalExpr);
+
+ Tokens = annotate("cond ? cond2 ? : id1 : id2\n", Style);
+ EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_ConditionalExpr);
+}
+
} // namespace
} // namespace format
} // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1312,11 +1312,34 @@
if (Style.isCSharp()) {
// `Type?)`, `Type?>`, `Type? name;` and `Type? name =` can only be
// nullable types.
+
+// `Type?)`, `Type?>`, `Type? name;`
+if (Tok->Next &&
+(Tok->Next->startsSequence(tok::question, tok::r_paren) ||
+ Tok->Next->startsSequence(tok::question, tok::greater) ||
+ Tok->Next->startsSequence(tok::question, tok::identifier,
+ tok::semi))) {
+ Tok->setType(TT_CSharpNullable);
+ break;
+}
+
+// `Type? name =`
+if (Tok->Next && Tok->Next->is(tok::identifier) && Tok->Next->Next &&
+Tok->Next->Next->is(tok::equal)) {
+ Tok->setType(TT_CSharpNullable);
+ break;
+}
+
// Line.MustBeDeclaration will be true for `Type? name;`.
-if ((!Contexts.back().IsExpression && Line.MustBeDeclaration) ||
-(Tok->Next && Tok->Next->isOneOf(tok::r_paren, tok::greater)) ||
-(Tok->Next && Tok->Next->is(tok::identifier) && Tok->Next->Next &&
- Tok->Next->Next->is(tok::equal))) {
+// But not
+// cond ? "A" : "B";
+// cond ? id : "B";
+// cond ? cond2 ? "A" : "B" : "C";
+if (!Contexts.back().IsExpression && Line.MustBeDeclaration &&
+(!Tok->Next ||
+ !Tok->Next->isOneOf(tok::identifier, tok::string_literal) ||
+ !Tok->Next->Next ||
+ !Tok->Next->Next->isOneOf(tok::colon, tok::question))) {
Tok->setType(TT_CSharpNullable);
break;
}
Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1724,6 +1724,34 @@
EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_ConditionLParen);
}
+TEST_F(TokenAnnotatorTest, CSharpNullableTypes) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+
+ auto Tokens = annotate("int? a;\n", Style);
+ EXPECT_EQ(Tokens.size(), 5u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int? a = 1;\n", Style);
+ EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?)\n", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?>\n", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("cond? id : id2\n", Style);
+ EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_ConditionalExpr);
+
+ Tokens = annotate("cond ? cond2 ? : id1 : id2\n", Style);
+ EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_ConditionalExpr);
+}
+
} // namespace
} // namespace format
} // namespace clang
Index:
[PATCH] D148473: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable
HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added inline comments.
This revision is now accepted and ready to land.
Comment at: clang/lib/Format/TokenAnnotator.cpp:1338-1342
+if (!Contexts.back().IsExpression && Line.MustBeDeclaration &&
+!(Tok->Next &&
+ Tok->Next->isOneOf(tok::identifier, tok::string_literal) &&
+ Tok->Next->Next &&
+ Tok->Next->Next->isOneOf(tok::colon, tok::question))) {
Even if there are more negations, I can read this better.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148473/new/
https://reviews.llvm.org/D148473
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148473: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable
MyDeveloperDay added a comment. @exv any thoughts on this one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148473/new/ https://reviews.llvm.org/D148473 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148473: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable
MyDeveloperDay added a comment. The exclusions are not complete for example `cond? foo() : "B";` would still fail. but this moves us a little closer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148473/new/ https://reviews.llvm.org/D148473 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148473: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable
MyDeveloperDay created this revision.
MyDeveloperDay added a reviewer: owenpan.
MyDeveloperDay added a project: clang-format.
Herald added projects: All, clang.
Herald added reviewers: rymiel, HazardyKnusperkeks.
MyDeveloperDay requested review of this revision.
Refactor the CSharpNullable assignment code to be a little easier to read
(Honestly I don't like it when an if expression get really long and
complicated).
Handle the case where '?' is actually a ternary operator.
Fixes: #58067
https://github.com/llvm/llvm-project/issues/58067
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D148473
Files:
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp
Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1688,6 +1688,34 @@
EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
}
+TEST_F(TokenAnnotatorTest, CSharpNullableTypes) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+
+ auto Tokens = annotate("int? a;\n", Style);
+ EXPECT_EQ(Tokens.size(), 5u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int? a = 1;\n", Style);
+ EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?)\n", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?>\n", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("cond? id : id2\n", Style);
+ EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_ConditionalExpr);
+
+ Tokens = annotate("cond ? cond2 ? : id1 : id2\n", Style);
+ EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_ConditionalExpr);
+}
+
} // namespace
} // namespace format
} // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1312,11 +1312,34 @@
if (Style.isCSharp()) {
// `Type?)`, `Type?>`, `Type? name;` and `Type? name =` can only be
// nullable types.
+
+// `Type?)`, `Type?>`, `Type? name;`
+if (Tok->Next &&
+(Tok->Next->startsSequence(tok::question, tok::r_paren) ||
+ Tok->Next->startsSequence(tok::question, tok::greater) ||
+ Tok->Next->startsSequence(tok::question, tok::identifier,
+ tok::semi))) {
+ Tok->setType(TT_CSharpNullable);
+ break;
+}
+
+// `Type? name =`
+if (Tok->Next && Tok->Next->is(tok::identifier) && Tok->Next->Next &&
+Tok->Next->Next->is(tok::equal)) {
+ Tok->setType(TT_CSharpNullable);
+ break;
+}
+
// Line.MustBeDeclaration will be true for `Type? name;`.
-if ((!Contexts.back().IsExpression && Line.MustBeDeclaration) ||
-(Tok->Next && Tok->Next->isOneOf(tok::r_paren, tok::greater)) ||
-(Tok->Next && Tok->Next->is(tok::identifier) && Tok->Next->Next &&
- Tok->Next->Next->is(tok::equal))) {
+// But not
+// cond ? "A" : "B";
+// cond ? id : "B";
+// cond ? cond2 ? "A" : "B" : "C";
+if (!Contexts.back().IsExpression && Line.MustBeDeclaration &&
+!(Tok->Next &&
+ Tok->Next->isOneOf(tok::identifier, tok::string_literal) &&
+ Tok->Next->Next &&
+ Tok->Next->Next->isOneOf(tok::colon, tok::question))) {
Tok->setType(TT_CSharpNullable);
break;
}
Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1688,6 +1688,34 @@
EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
}
+TEST_F(TokenAnnotatorTest, CSharpNullableTypes) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+
+ auto Tokens = annotate("int? a;\n", Style);
+ EXPECT_EQ(Tokens.size(), 5u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int? a = 1;\n", Style);
+ EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?)\n", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens = annotate("int?>\n", Style);
+ EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::question, TT_CSharpNullable);
+
+ Tokens
