Disallow whitespace before non-null assertion #56384
Conversation
typescript-bot
added
Author: Team
For Uncommitted Bug
|
@typescript-bot test top200 |
|
Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at f0079d2. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at f0079d2. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at f0079d2. You can monitor the build here. Update: The results are in! |
|
@jakebailey Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Something interesting changed - please have a look. Details
|
|
Hey @jakebailey, the results of running the DT tests are ready. |
| value/* this is a comment */! in String; | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS1289: A non-null assertion may not be preceded by whitespace. |
There was a problem hiding this comment.
Obviously this is not the way I want to emit this error; we can discuss this later of course.
There was a problem hiding this comment.
What's wrong with it? The inclusion of the ! itself?
There was a problem hiding this comment.
Complaining about a comment when there's no whitespace; I would have expected at least one tool to support some sort of interleaved comment a la /*@__PURE__*/ or something. Maybe that's silly. If we do want to complain about comments, then my message would need to be updated (which is fine).
|
@jakebailey Here are the results of running the top-repos suite comparing Everything looks good! |
|
Wow, really, nobody had this? |
In the top 200? Seems reasonable. From the search @gorosgobe did over at tc39/proposal-negated-in-instanceof#1 (comment), there are only 13 results where |
src/compiler/diagnosticMessages.json
Outdated
| @@ -951,6 +951,10 @@ | |||
| "category": "Error", | |||
| "code": 1288 | |||
| }, | |||
| "A non-null assertion may not be preceded by whitespace.": { | |||
There was a problem hiding this comment.
What about something like
Whitespace and comments are not allowed between the argument and '!' operator in a non-null assertion.
There was a problem hiding this comment.
Also shockingly, we don't use the term "whitespace" anywhere in our messages.
There was a problem hiding this comment.
Non-null assertion operator must immediately follow its expression
I like Daniel's too though
| @@ -1801,6 +1801,10 @@ | |||
| "category": "Error", | |||
| "code": 1537 | |||
| }, | |||
| "A non-null assertion must be preceded by an identifier.": { | |||
There was a problem hiding this comment.
identifier? Isn't that misleading?
|
Hey, may I ask why you closed your own PR here? Just curious if there was any outside discussion or change of circumstance! (Here via tc39/proposal-negated-in-instanceof#1 (reference)) |
|
It didn't catch any bugs and wasn't the right implementation anyway. Just keeping my PR list clean. |
Prototype implementation just to run the test suite; this is wrong for comments and such and probably trailing trivia (if that is even possible).
A formatting test fails because we don't format code with syntax errors, which this of course introduces.