Re: [PATCH] D21204: clang-format: [JS] post-fix non-null assertion operator.

[Martin Probst via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL272524: clang-format: [JS] post-fix non-null assertion 
operator. (authored by mprobst).

Changed prior to commit:
  http://reviews.llvm.org/D21204?vs=60311=60482#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D21204

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestJS.cpp

Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2126,6 +2126,11 @@
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   return false;
+// Postfix non-null assertion operator, as in `foo!.bar()`.
+if (Right.is(tok::exclaim) && (Left.isOneOf(tok::identifier, tok::r_paren,
+tok::r_square, tok::r_brace) ||
+   Left.Tok.isLiteral()))
+  return false;
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
   return true;
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -1293,5 +1293,14 @@
"var x   =  hello();");
 }
 
+TEST_F(FormatTestJS, NonNullAssertionOperator) {
+  verifyFormat("let x = foo!.bar();\n");
+  verifyFormat("let x = foo ? bar! : baz;\n");
+  verifyFormat("let x = !foo;\n");
+  verifyFormat("let x = foo[0]!;\n");
+  verifyFormat("let x = (foo)!;\n");
+  verifyFormat("let x = {foo: 1}!;\n");
+}
+
 } // end namespace tooling
 } // end namespace clang


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2126,6 +2126,11 @@
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   return false;
+// Postfix non-null assertion operator, as in `foo!.bar()`.
+if (Right.is(tok::exclaim) && (Left.isOneOf(tok::identifier, tok::r_paren,
+tok::r_square, tok::r_brace) ||
+   Left.Tok.isLiteral()))
+  return false;
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
   return true;
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -1293,5 +1293,14 @@
"var x   =  hello();");
 }
 
+TEST_F(FormatTestJS, NonNullAssertionOperator) {
+  verifyFormat("let x = foo!.bar();\n");
+  verifyFormat("let x = foo ? bar! : baz;\n");
+  verifyFormat("let x = !foo;\n");
+  verifyFormat("let x = foo[0]!;\n");
+  verifyFormat("let x = (foo)!;\n");
+  verifyFormat("let x = {foo: 1}!;\n");
+}
+
 } // end namespace tooling
 } // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21204: clang-format: [JS] post-fix non-null assertion operator.

[Martin Probst via cfe-commits]

mprobst added inline comments.


Comment at: lib/Format/TokenAnnotator.cpp:2128
@@ +2127,3 @@
+if (Right.is(tok::exclaim) && (Left.isOneOf(tok::identifier, tok::r_paren,
+tok::r_square, tok::r_brace) ||
+   Left.Tok.isLiteral()))

Tests added. A literal cannot really be null, but the operator is allowed after 
any kind of expression, and it's straight forward to test for it here.


http://reviews.llvm.org/D21204



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21204: clang-format: [JS] post-fix non-null assertion operator.

[Martin Probst via cfe-commits]

mprobst updated this revision to Diff 60311.
mprobst added a comment.

- more tests


http://reviews.llvm.org/D21204

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1291,5 +1291,14 @@
"var x   =  hello();");
 }
 
+TEST_F(FormatTestJS, NonNullAssertionOperator) {
+  verifyFormat("let x = foo!.bar();\n");
+  verifyFormat("let x = foo ? bar! : baz;\n");
+  verifyFormat("let x = !foo;\n");
+  verifyFormat("let x = foo[0]!;\n");
+  verifyFormat("let x = (foo)!;\n");
+  verifyFormat("let x = {foo: 1}!;\n");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2123,6 +2123,11 @@
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   return false;
+// Postfix non-null assertion operator, as in `foo!.bar()`.
+if (Right.is(tok::exclaim) && (Left.isOneOf(tok::identifier, tok::r_paren,
+tok::r_square, tok::r_brace) ||
+   Left.Tok.isLiteral()))
+  return false;
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
   return true;


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1291,5 +1291,14 @@
"var x   =  hello();");
 }
 
+TEST_F(FormatTestJS, NonNullAssertionOperator) {
+  verifyFormat("let x = foo!.bar();\n");
+  verifyFormat("let x = foo ? bar! : baz;\n");
+  verifyFormat("let x = !foo;\n");
+  verifyFormat("let x = foo[0]!;\n");
+  verifyFormat("let x = (foo)!;\n");
+  verifyFormat("let x = {foo: 1}!;\n");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2123,6 +2123,11 @@
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   return false;
+// Postfix non-null assertion operator, as in `foo!.bar()`.
+if (Right.is(tok::exclaim) && (Left.isOneOf(tok::identifier, tok::r_paren,
+tok::r_square, tok::r_brace) ||
+   Left.Tok.isLiteral()))
+  return false;
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21204: clang-format: [JS] post-fix non-null assertion operator.

[Daniel Jasper via cfe-commits]

djasper accepted this revision.
This revision is now accepted and ready to land.


Comment at: lib/Format/TokenAnnotator.cpp:2128
@@ +2127,3 @@
+if (Right.is(tok::exclaim) &&
+(Left.isOneOf(tok::identifier, tok::r_paren, tok::r_square) ||
+ Left.Tok.isLiteral()))

Can you add a test for each of these? Also, how can a literal be null?


http://reviews.llvm.org/D21204



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21204: clang-format: [JS] post-fix non-null assertion operator.

[Martin Probst via cfe-commits]

mprobst updated this revision to Diff 60308.
mprobst marked an inline comment as done.
mprobst added a comment.

- invert check to whitelist known tokens that can precede a non-null assertion.


http://reviews.llvm.org/D21204

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1291,5 +1291,12 @@
"var x   =  hello();");
 }
 
+TEST_F(FormatTestJS, NonNullAssertionOperator) {
+  verifyFormat("let x = foo!.bar();\n");
+  verifyFormat("let x = foo ? bar! : baz;\n");
+  verifyFormat("let x = !foo;\n");
+  verifyFormat("let x = foo!;\n");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2123,6 +2123,11 @@
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   return false;
+// Postfix non-null assertion operator, as in `foo!.bar()`.
+if (Right.is(tok::exclaim) &&
+(Left.isOneOf(tok::identifier, tok::r_paren, tok::r_square) ||
+ Left.Tok.isLiteral()))
+  return false;
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
   return true;


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1291,5 +1291,12 @@
"var x   =  hello();");
 }
 
+TEST_F(FormatTestJS, NonNullAssertionOperator) {
+  verifyFormat("let x = foo!.bar();\n");
+  verifyFormat("let x = foo ? bar! : baz;\n");
+  verifyFormat("let x = !foo;\n");
+  verifyFormat("let x = foo!;\n");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2123,6 +2123,11 @@
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   return false;
+// Postfix non-null assertion operator, as in `foo!.bar()`.
+if (Right.is(tok::exclaim) &&
+(Left.isOneOf(tok::identifier, tok::r_paren, tok::r_square) ||
+ Left.Tok.isLiteral()))
+  return false;
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21204: clang-format: [JS] post-fix non-null assertion operator.

[Martin Probst via cfe-commits]

mprobst added inline comments.


Comment at: lib/Format/TokenAnnotator.cpp:2127
@@ +2126,3 @@
+// Postfix non-null assertion operator, as in `foo!.bar()`.
+if (Right.is(tok::exclaim) && Right.Next &&
+Right.Next->isNot(tok::identifier) && !Right.Next->Tok.isLiteral())

djasper wrote:
> From a quick look at our codebase, I think you need to also rule out the 
> following for Right.Next:
> 
>   - (, [, {
>   - !  ('!!' seems to be a thing)
>   - -, +, ~ (don't know exactly what this does to a JS boolean value)
>   - %, $ (not entirely sure whether these become part of the identifier)
> 
> 
Actually, I think inverting the condition and "whitelisting" preceding tokens 
makes more sense and is probably a lot more robust. PTAL, now checking left for 
identifier, r_paren, r_square, literal.


http://reviews.llvm.org/D21204



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21204: clang-format: [JS] post-fix non-null assertion operator.

[Daniel Jasper via cfe-commits]

djasper added inline comments.


Comment at: lib/Format/TokenAnnotator.cpp:2127
@@ +2126,3 @@
+// Postfix non-null assertion operator, as in `foo!.bar()`.
+if (Right.is(tok::exclaim) && Right.Next &&
+Right.Next->isNot(tok::identifier) && !Right.Next->Tok.isLiteral())

From a quick look at our codebase, I think you need to also rule out the 
following for Right.Next:

  - (, [, {
  - !  ('!!' seems to be a thing)
  - -, +, ~ (don't know exactly what this does to a JS boolean value)
  - %, $ (not entirely sure whether these become part of the identifier)




http://reviews.llvm.org/D21204



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D21204: clang-format: [JS] post-fix non-null assertion operator.

[Martin Probst via cfe-commits]

mprobst created this revision.
mprobst added a reviewer: djasper.
mprobst added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

Do not insert whitespace preceding the "!" postfix operator. This is an
incomplete fix, but should cover common usage.

http://reviews.llvm.org/D21204

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1291,5 +1291,12 @@
"var x   =  hello();");
 }
 
+TEST_F(FormatTestJS, NonNullAssertionOperator) {
+  verifyFormat("let x = foo!.bar();\n");
+  verifyFormat("let x = foo ? bar! : baz;\n");
+  verifyFormat("let x = !foo;\n");
+  verifyFormat("let x = foo!;\n");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2123,6 +2123,10 @@
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   return false;
+// Postfix non-null assertion operator, as in `foo!.bar()`.
+if (Right.is(tok::exclaim) && Right.Next &&
+Right.Next->isNot(tok::identifier) && !Right.Next->Tok.isLiteral())
+  return false;
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
   return true;


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1291,5 +1291,12 @@
"var x   =  hello();");
 }
 
+TEST_F(FormatTestJS, NonNullAssertionOperator) {
+  verifyFormat("let x = foo!.bar();\n");
+  verifyFormat("let x = foo ? bar! : baz;\n");
+  verifyFormat("let x = !foo;\n");
+  verifyFormat("let x = foo!;\n");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2123,6 +2123,10 @@
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   return false;
+// Postfix non-null assertion operator, as in `foo!.bar()`.
+if (Right.is(tok::exclaim) && Right.Next &&
+Right.Next->isNot(tok::identifier) && !Right.Next->Tok.isLiteral())
+  return false;
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits