[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
shafik added a comment. Herald added a project: All. Herald added a reviewer: rymiel. Is this change still relevant or can we close this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
mprobst added inline comments.
Comment at: clang/lib/Format/TokenAnnotator.cpp:3522
+ if ((Left.is(TT_JsTypeOperator) && Right.isTypeOrIdentifier()) ||
+ (Left.isTypeOrIdentifier() || Left.is(TT_TemplateCloser)) &&
+ Right.is(TT_JsTypeOperator))
MyDeveloperDay wrote:
> mprobst wrote:
> > Why do we need this further qualification here? I'd have expect that you
> > can simply "return Style.SpacesInJavaScriptUnion;"? identifier || template
> > closer also sounds oddly specific, why exactly those?
> Left = template closer, Right =JsTypeOperator is for this case
>
> ```
> type Foo = Bar | Baz;
>^
> ```
>
> Left = type or identifier, Right =JsTypeOperator is for this case
>
>
> ```
> let x: A | B = A | B;
> ^
> ```
What about:
```
let x: {foo: string}&{bar: number};
```
I'm not sure including curlies would be comprehensive. It might be worth
checking the TS grammar.
But coming back, did you try this:
```
if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) {
return Style.SpacesInJavaScriptUnion;
```
(We might also need to do something in `spaceRequiredBefore`, maybe that fires
earlier?)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117197/new/
https://reviews.llvm.org/D117197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. Looks ok from my side. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3522 + if ((Left.is(TT_JsTypeOperator) && Right.isTypeOrIdentifier()) || + (Left.isTypeOrIdentifier() || Left.is(TT_TemplateCloser)) && + Right.is(TT_JsTypeOperator)) mprobst wrote: > Why do we need this further qualification here? I'd have expect that you can > simply "return Style.SpacesInJavaScriptUnion;"? identifier || template closer > also sounds oddly specific, why exactly those? Left = template closer, Right =JsTypeOperator is for this case ``` type Foo = Bar | Baz; ^ ``` Left = type or identifier, Right =JsTypeOperator is for this case ``` let x: A | B = A | B; ^ ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
mprobst added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3522 + if ((Left.is(TT_JsTypeOperator) && Right.isTypeOrIdentifier()) || + (Left.isTypeOrIdentifier() || Left.is(TT_TemplateCloser)) && + Right.is(TT_JsTypeOperator)) Why do we need this further qualification here? I'd have expect that you can simply "return Style.SpacesInJavaScriptUnion;"? identifier || template closer also sounds oddly specific, why exactly those? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
MyDeveloperDay updated this revision to Diff 399922.
MyDeveloperDay marked 10 inline comments as done.
MyDeveloperDay added a comment.
Make suggested improvements
Add additional test case
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117197/new/
https://reviews.llvm.org/D117197
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/FormatTestJS.cpp
Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -1674,6 +1674,31 @@
verifyFormat("export type X = {\n"
" a: Foo|Bar;\n"
"};");
+
+ auto Style = getGoogleJSStyleWithColumns(60);
+ Style.SpacesInJavaScriptUnion = true;
+ verifyFormat("let x: A | B = A | B;", Style);
+ verifyFormat("let x?: A | B = A | B;", Style);
+ verifyFormat("let x: A & B | C = A & B;", Style);
+ verifyFormat("let x: Foo = new Foo();", Style);
+ verifyFormat("function(x: A | B): C & D {}", Style);
+ verifyFormat("function(x: A | B = A | B): C & D {}", Style);
+ verifyFormat("function x(path: number | string) {}", Style);
+ verifyFormat("function x(): string | number {}", Style);
+ verifyFormat("type Foo = Bar | Baz;", Style);
+ verifyFormat("type Foo = Bar | Baz;", Style);
+ verifyFormat("type Foo = (Bar | Baz);", Style);
+ verifyFormat("let x: Bar | Baz;", Style);
+ verifyFormat("let x: Bar | Baz;", Style);
+ verifyFormat("let x: (Foo | Bar)[];", Style);
+ verifyFormat("type X = {\n"
+ " a: Foo | Bar;\n"
+ "};",
+ Style);
+ verifyFormat("export type X = {\n"
+ " a: Foo | Bar;\n"
+ "};",
+ Style);
}
TEST_F(FormatTestJS, UnionIntersectionTypesInObjectType) {
@@ -1686,6 +1711,19 @@
"b: b|null,\n"
" }) {}\n"
"}");
+
+ auto Style = getGoogleJSStyleWithColumns(60);
+ Style.SpacesInJavaScriptUnion = true;
+ verifyFormat("let x: {x: number | null} = {x: number | null};", Style);
+ verifyFormat("let nested: {x: {y: number | null}};", Style);
+ verifyFormat("let mixed: {x: [number | null, {w: number}]};", Style);
+ verifyFormat("class X {\n"
+ " contructor(x: {\n"
+ "a: a | null,\n"
+ "b: b | null,\n"
+ " }) {}\n"
+ "}",
+ Style);
}
TEST_F(FormatTestJS, ClassDeclarations) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18789,6 +18789,7 @@
CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
CHECK_PARSE_BOOL(SpacesInContainerLiterals);
CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
+ CHECK_PARSE_BOOL(SpacesInJavaScriptUnion);
CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
CHECK_PARSE_BOOL(SpaceAfterLogicalNot);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3517,8 +3517,13 @@
return true;
if (Right.isOneOf(TT_JsTypeColon, TT_JsTypeOptionalQuestion))
return false;
-if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator))
+if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) {
+ if ((Left.is(TT_JsTypeOperator) && Right.isTypeOrIdentifier()) ||
+ (Left.isTypeOrIdentifier() || Left.is(TT_TemplateCloser)) &&
+ Right.is(TT_JsTypeOperator))
+return Style.SpacesInJavaScriptUnion;
return false;
+}
if ((Left.is(tok::l_brace) || Right.is(tok::r_brace)) &&
Line.First->isOneOf(Keywords.kw_import, tok::kw_export))
return false;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -818,6 +818,7 @@
Style.SpacesInCStyleCastParentheses);
IO.mapOptional("SpacesInLineCommentPrefix",
Style.SpacesInLineCommentPrefix);
+IO.mapOptional("SpacesInJavaScriptUnion", Style.SpacesInJavaScriptUnion);
IO.mapOptional("SpacesInParentheses", Style.SpacesInParentheses);
IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
IO.mapOptional("SpaceBeforeSquareBrackets",
@@ -1221,6 +1222,7 @@
LLVMStyle.SpacesInContainerLiterals = true;
LLVMStyle.SpacesInCStyleCastParentheses = false;
LLVMStyle.SpacesInLineCommentPrefix = {/*Minimum=*/1, /*Maximum=*/-1u};
+ LLVMStyle.SpacesInJavaScriptUnion = false;
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3524 return false; if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) return false; mprobst wrote: > curdeius wrote: > > HazardyKnusperkeks wrote: > > > mprobst wrote: > > > > MyDeveloperDay wrote: > > > > > mprobst wrote: > > > > > > shouldn't you change this line here? > > > > > You know I thought the same and this was where I first put the code > > > > > change in, but this code doesn't seem to have any effect and I've > > > > > been caught out by this before... (anyone else seen that?) > > > > > > > > > > I'm not sure if something has been changed, but I'm finding that > > > > > often I add code into spaceRequiredBetween() and despite it being > > > > > executed correctly it doesn't have the desired effect, which is why > > > > > the code is in spaceRequiredBefore() > > > > Generally we put space between two operators. So this line must have > > > > some effect, as otherwise we'd always emit `A | B`. Given that, I think > > > > we need more debugging here to make this change - working around by > > > > some more code somewhere else doesn't seem like a good long term > > > > approach. > > > > > > > > Also, note that this is all in `spaceRequiredBefore`, right? > > > It's on my plan to refactor these 2 methods, because I think they can and > > > should be made easier to understand and reason about. > > > > > > So from my point of view one can accept this patch. > > I modified it like this and it passes all the tests. Please recheck. > I appreciate the intention to refactor this in the future. > > I still believe we really have to understand how this code works though, and > why changing this line doesn't have the effect we'd expect, before we land > the fix. Chances are there is some substantial misunderstanding how this all > fits together, which usually leads to bugs. @mprobst I do agree, let me take another look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
curdeius added inline comments.
Comment at: clang/unittests/Format/FormatTestJS.cpp:1705-1713
verifyFormat("let x: {x: number|null} = {x: number | null};");
verifyFormat("let nested: {x: {y: number|null}};");
verifyFormat("let mixed: {x: [number|null, {w: number}]};");
verifyFormat("class X {\n"
" contructor(x: {\n"
"a: a|null,\n"
"b: b|null,\n"
Suggestion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117197/new/
https://reviews.llvm.org/D117197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
mprobst added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3524 return false; if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) return false; curdeius wrote: > HazardyKnusperkeks wrote: > > mprobst wrote: > > > MyDeveloperDay wrote: > > > > mprobst wrote: > > > > > shouldn't you change this line here? > > > > You know I thought the same and this was where I first put the code > > > > change in, but this code doesn't seem to have any effect and I've been > > > > caught out by this before... (anyone else seen that?) > > > > > > > > I'm not sure if something has been changed, but I'm finding that often > > > > I add code into spaceRequiredBetween() and despite it being executed > > > > correctly it doesn't have the desired effect, which is why the code is > > > > in spaceRequiredBefore() > > > Generally we put space between two operators. So this line must have some > > > effect, as otherwise we'd always emit `A | B`. Given that, I think we > > > need more debugging here to make this change - working around by some > > > more code somewhere else doesn't seem like a good long term approach. > > > > > > Also, note that this is all in `spaceRequiredBefore`, right? > > It's on my plan to refactor these 2 methods, because I think they can and > > should be made easier to understand and reason about. > > > > So from my point of view one can accept this patch. > I modified it like this and it passes all the tests. Please recheck. I appreciate the intention to refactor this in the future. I still believe we really have to understand how this code works though, and why changing this line doesn't have the effect we'd expect, before we land the fix. Chances are there is some substantial misunderstanding how this all fits together, which usually leads to bugs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
curdeius requested changes to this revision. curdeius added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/TokenAnnotator.cpp:3524-3525 return false; if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) return false; if ((Left.is(tok::l_brace) || Right.is(tok::r_brace)) && HazardyKnusperkeks wrote: > mprobst wrote: > > MyDeveloperDay wrote: > > > mprobst wrote: > > > > shouldn't you change this line here? > > > You know I thought the same and this was where I first put the code > > > change in, but this code doesn't seem to have any effect and I've been > > > caught out by this before... (anyone else seen that?) > > > > > > I'm not sure if something has been changed, but I'm finding that often I > > > add code into spaceRequiredBetween() and despite it being executed > > > correctly it doesn't have the desired effect, which is why the code is in > > > spaceRequiredBefore() > > Generally we put space between two operators. So this line must have some > > effect, as otherwise we'd always emit `A | B`. Given that, I think we need > > more debugging here to make this change - working around by some more code > > somewhere else doesn't seem like a good long term approach. > > > > Also, note that this is all in `spaceRequiredBefore`, right? > It's on my plan to refactor these 2 methods, because I think they can and > should be made easier to understand and reason about. > > So from my point of view one can accept this patch. I modified it like this and it passes all the tests. Please recheck. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
curdeius added inline comments.
Comment at: clang/unittests/Format/FormatTestJS.cpp:1708-1713
verifyFormat("class X {\n"
" contructor(x: {\n"
"a: a|null,\n"
"b: b|null,\n"
" }) {}\n"
"}");
Oh, you might need to add a test for this case too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117197/new/
https://reviews.llvm.org/D117197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM apart from nits. I agree though that spaceRequired(Before|Between) need some refactoring love. Comment at: clang/docs/ReleaseNotes.rst:327 +- Option ``SpacesInJavaScriptUnion`` has been added to allow spaces around + JavsScript Union and Intersection Type operators. + Also, why capital letters on union and intersection type? Comment at: clang/include/clang/Format/Format.h:3618 + // If ``true``, spaces will be inserted before and after the ``|`` of + // a JavaScript/TypeScript union + /// \code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3475 return false; +if (Left.is(TT_JsTypeOperator) && Right.isTypeOrIdentifier() || +(Left.isTypeOrIdentifier() || Left.is(TT_TemplateCloser)) && Be explicit on the binding. Comment at: clang/lib/Format/TokenAnnotator.cpp:3524 return false; if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) return false; mprobst wrote: > MyDeveloperDay wrote: > > mprobst wrote: > > > shouldn't you change this line here? > > You know I thought the same and this was where I first put the code change > > in, but this code doesn't seem to have any effect and I've been caught out > > by this before... (anyone else seen that?) > > > > I'm not sure if something has been changed, but I'm finding that often I > > add code into spaceRequiredBetween() and despite it being executed > > correctly it doesn't have the desired effect, which is why the code is in > > spaceRequiredBefore() > Generally we put space between two operators. So this line must have some > effect, as otherwise we'd always emit `A | B`. Given that, I think we need > more debugging here to make this change - working around by some more code > somewhere else doesn't seem like a good long term approach. > > Also, note that this is all in `spaceRequiredBefore`, right? It's on my plan to refactor these 2 methods, because I think they can and should be made easier to understand and reason about. So from my point of view one can accept this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
mprobst added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3524 return false; if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) return false; MyDeveloperDay wrote: > mprobst wrote: > > shouldn't you change this line here? > You know I thought the same and this was where I first put the code change > in, but this code doesn't seem to have any effect and I've been caught out by > this before... (anyone else seen that?) > > I'm not sure if something has been changed, but I'm finding that often I add > code into spaceRequiredBetween() and despite it being executed correctly it > doesn't have the desired effect, which is why the code is in > spaceRequiredBefore() Generally we put space between two operators. So this line must have some effect, as otherwise we'd always emit `A | B`. Given that, I think we need more debugging here to make this change - working around by some more code somewhere else doesn't seem like a good long term approach. Also, note that this is all in `spaceRequiredBefore`, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3524 return false; if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) return false; mprobst wrote: > shouldn't you change this line here? You know I thought the same and this was where I first put the code change in, but this code doesn't seem to have any effect and I've been caught out by this before... (anyone else seen that?) I'm not sure if something has been changed, but I'm finding that often I add code into spaceRequiredBetween() and despite it being executed correctly it doesn't have the desired effect, which is why the code is in spaceRequiredBefore() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
mprobst added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3524 return false; if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) return false; shouldn't you change this line here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types
MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: mprobst, krasimir, HazardyKnusperkeks,
curdeius, owenpan.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.
https://github.com/llvm/llvm-project/issues/49858
The following issue highlights a problem where we cannot add a space between
A|B when detecting a JSTypeOperator
This patch allows clang-format to be configured to support that (which seems to
match most references I can find)
https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#union-types
https://flow.org/en/docs/types/intersections/
Fixes: #49858
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D117197
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/FormatTestJS.cpp
Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -1674,6 +1674,31 @@
verifyFormat("export type X = {\n"
" a: Foo|Bar;\n"
"};");
+
+ auto Style = getGoogleJSStyleWithColumns(60);
+ Style.SpacesInJavaScriptUnion = true;
+ verifyFormat("let x: A | B = A | B;", Style);
+ verifyFormat("let x?: A | B = A | B;", Style);
+ verifyFormat("let x: A & B | C = A & B;", Style);
+ verifyFormat("let x: Foo = new Foo();", Style);
+ verifyFormat("function(x: A | B): C & D {}", Style);
+ verifyFormat("function(x: A | B = A | B): C & D {}", Style);
+ verifyFormat("function x(path: number | string) {}", Style);
+ verifyFormat("function x(): string | number {}", Style);
+ verifyFormat("type Foo = Bar | Baz;", Style);
+ verifyFormat("type Foo = Bar | Baz;", Style);
+ verifyFormat("type Foo = (Bar | Baz);", Style);
+ verifyFormat("let x: Bar | Baz;", Style);
+ verifyFormat("let x: Bar | Baz;", Style);
+ verifyFormat("let x: (Foo | Bar)[];", Style);
+ verifyFormat("type X = {\n"
+ " a: Foo | Bar;\n"
+ "};",
+ Style);
+ verifyFormat("export type X = {\n"
+ " a: Foo | Bar;\n"
+ "};",
+ Style);
}
TEST_F(FormatTestJS, UnionIntersectionTypesInObjectType) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18789,6 +18789,7 @@
CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
CHECK_PARSE_BOOL(SpacesInContainerLiterals);
CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
+ CHECK_PARSE_BOOL(SpacesInJavaScriptUnion);
CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
CHECK_PARSE_BOOL(SpaceAfterLogicalNot);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3472,6 +3472,10 @@
if (Right.is(tok::star) &&
Left.isOneOf(Keywords.kw_function, Keywords.kw_yield))
return false;
+if (Left.is(TT_JsTypeOperator) && Right.isTypeOrIdentifier() ||
+(Left.isTypeOrIdentifier() || Left.is(TT_TemplateCloser)) &&
+Right.is(TT_JsTypeOperator))
+ return Style.SpacesInJavaScriptUnion;
if (Right.isOneOf(tok::l_brace, tok::l_square) &&
Left.isOneOf(Keywords.kw_function, Keywords.kw_yield,
Keywords.kw_extends, Keywords.kw_implements))
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -818,6 +818,7 @@
Style.SpacesInCStyleCastParentheses);
IO.mapOptional("SpacesInLineCommentPrefix",
Style.SpacesInLineCommentPrefix);
+IO.mapOptional("SpacesInJavaScriptUnion", Style.SpacesInJavaScriptUnion);
IO.mapOptional("SpacesInParentheses", Style.SpacesInParentheses);
IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
IO.mapOptional("SpaceBeforeSquareBrackets",
@@ -1221,6 +1222,7 @@
LLVMStyle.SpacesInContainerLiterals = true;
LLVMStyle.SpacesInCStyleCastParentheses = false;
LLVMStyle.SpacesInLineCommentPrefix = {/*Minimum=*/1, /*Maximum=*/-1u};
+ LLVMStyle.SpacesInJavaScriptUnion = false;
LLVMStyle.SpaceAfterCStyleCast = false;
LLVMStyle.SpaceAfterLogicalNot = false;
LLVMStyle.SpaceAfterTemplateKeyword = true;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3614,6 +3614,15 @@
/// \version 14
SpacesInLineComment
