add grammar check for labeled declaration#25317
Conversation
b5fe0b8 to
ec04ca5
Compare
| "category": "Error", | ||
| "code": 1343 | ||
| }, | ||
| "Association of a statement label with a function declaration is not allowed.": { |
There was a problem hiding this comment.
how about A function declaration can not be the target of a label.
@DanielRosenwasser any better suggestions here?
There was a problem hiding this comment.
Surprisingly, variable statements can be targets of labeled statements - which I think is really stupid!
If we were willing to make that break, we could just make it something like
Declarations cannot be labeled.
Since you're not allowed to have class declarations be labeled either, and presumably the same should apply to enums and namespaces.
We may also want to consider
'{0}' declarations cannot be labeled.
where 0 is substituted with var, const, let, enum, function, function*, async function, class.
Also, keep in mind #25326.
There was a problem hiding this comment.
module or namespace has an error now:
A namespace declaration is only allowed in a namespace or module.
| @@ -0,0 +1,8 @@ | |||
| error TS2318: Cannot find global type 'IterableIterator'. | |||
There was a problem hiding this comment.
Add @lib to avoid the errors.
| return false; | ||
| }); | ||
| } | ||
| if (compilerOptions.target! >= ScriptTarget.ES2015) { |
There was a problem hiding this comment.
It should also be disallowed when in strict mode. e.g.:
function bar() {
"use strict";
l: function f() { } // Error
}There was a problem hiding this comment.
We do all our handling of strict mode checks in the binder and not in the checker.
280ad14 to
e9aa180
Compare
| (function (E) { | ||
| })(E || (E = {})); | ||
| } | ||
| label: |
e9aa180 to
8dae277
Compare
| // Grammar checking for labeledStatement | ||
| if (inStrictMode && options.target! >= ScriptTarget.ES2015) { | ||
| if (isDeclarationStatement(node.statement) || isVariableStatement(node.statement)) { | ||
| errorOnFirstToken(node.label, Diagnostics._0_declarations_cannot_be_labeled, formatSyntaxKind(node.statement.kind)); |
There was a problem hiding this comment.
I do not think we ever exposed these kinds in error messages..
What we have done is have a generic message, like A label is not allowed here, or Invalid use of label.
There was a problem hiding this comment.
the other alternative is to have a switch statement for the kind, e.g. "function" for SyntaxKind.FunctonDeclaration, but personally I do not think the value you get is worth the effort.
| return isIterationStatement(statement, /*lookInLabeledStatements*/ false) | ||
| ? visitIterationStatement(statement, /*outermostLabeledStatement*/ node) | ||
| : restoreEnclosingLabel(visitNode(statement, visitor, isStatement), node, convertedLoopState && resetLabel); | ||
| : restoreEnclosingLabel(visitNode(statement, visitor, isStatement, liftToBlock), node, convertedLoopState && resetLabel); |
There was a problem hiding this comment.
not sure i understand what this change is doing?
There was a problem hiding this comment.
Fixing #25340 by lifting a sequence of, eg VariableStatement, EndOfDeclarationMarker (like a downlevel class produces) into a block. (Since right now that triggers an assertion violation)
|
@Kingwl Looks good - do you want to also add the various |
e345693 to
2c516bf
Compare
2c516bf to
d6ba356
Compare
|
@weswigham can you please review and merge |
Fixes #25183
Fixes #25326