Report all the js file errors and skip only the nodes that are not allowed in JS file #11978
Conversation
| // Return directly from the case if the given node doesnt want to visit each child | ||
| // Otherwise break to visit each child | ||
|
|
||
| switch (parent.kind) { |
There was a problem hiding this comment.
why are we using parent instead of node?
There was a problem hiding this comment.
Here we are checking if the node is 'questiontoken' or 'type' of parent and hence the check.
There was a problem hiding this comment.
but what was wrong with the previous pattern where we check parameter node, and then check it has a questiontoken set?
There was a problem hiding this comment.
There was no control over what to skip.. With this we visit all nodes except the nodes that aren't supported. (Eg. checkout how we use to return true when we found ? token, which would mean we wouldn't look into parameter children.. While that might be ok there it isn't if say for example we want to report error say on decorator of the class declaration and also verify say methods of class..) With this if we have say found typeNode of the variable declaration/function return type we don't go it that type node. But still visit other nodes of the variable declaration/function declaration which means we would be reporting other errors.
| case SyntaxKind.ExportKeyword: | ||
| case SyntaxKind.ConstKeyword: | ||
| case SyntaxKind.DefaultKeyword: | ||
| case SyntaxKind.AbstractKeyword: |
There was a problem hiding this comment.
why is abstract and const legal modifiers?
There was a problem hiding this comment.
hmm, I just copied from earlier implementation.. const should be valid when variable statement to handle const a = 10 and abstract shouldn't be valid. I will update this
| case SyntaxKind.VariableStatement: | ||
| // Check modifiers | ||
| if (nodes === (<ClassDeclaration | FunctionLikeDeclaration | VariableStatement>parent).modifiers) { | ||
| return checkModifiers(<NodeArray<Modifier>>nodes); | ||
| return checkModifiers(<NodeArray<Modifier>>nodes, isConstValid); |
There was a problem hiding this comment.
forgot to amend the changes before pushing it out. Updated it.
sheetalkamat
force-pushed
the
errorReportingInJsFile
branch
from
0208345 to
6456325
Compare
| return true; | ||
| // Check modifiers | ||
| if (nodes === (<ClassDeclaration | FunctionLikeDeclaration | VariableStatement>parent).modifiers) { | ||
| return checkModifiers(<NodeArray<Modifier>>nodes, !isConstInvalid); |
There was a problem hiding this comment.
return checkModifiers(<NodeArray<Modifier>>nodes, parent.kind === SyntaxKind.VariableStatement)There was a problem hiding this comment.
Just want to avoid doing that check again as its already done as part of case statement?
There was a problem hiding this comment.
it much more readable and better than accessing an undefined variable to be frank.
Fixes #11800