Proposal: CST-like Bogus AST Nodes

[jamiebuilds]

Description

Note: I'm using "bogus" instead of "unknown" (See #1819)

CST nodes have a lot of qualities that make it nice to represent both errors and error recovery.

Take the following code:

function %error% foo() {}

It should look something like this in CST form:

JsProgram:
  - JsFunctionStatement:
    - "function"
    - unexpected: "%error%"
    - JsIdentifierBinding: ["foo"]
    - JsParameterList: ["(", ")"]
    - JsFunctionBody: ["{", "}"]

We can tell a few things from this:

• We've inferred that this is a JsFunctionStatement even though it contains errors.
• We've recovered from the error by guessing that foo is the identifier.

Just as easily we could have created this CST:

JsProgram:
  - JsFunctionStatement:
    - "function"
    - unexpected: "%"
    - JsIdentifierBinding: ["error"]
    - unexpected: "% foo"
    - JsParameterList: ["(", ")"]
    - JsFunctionBody: ["{", "}"]

Or this one:

JsProgram:
  - JsFunctionStatement:
    - "function"
    - unexpected: "%error% foo() {}"

Or this one:

JsProgram:
  - unexpected: "function %error% foo() {}"

It that sense, the CST is a very clear representation of how our error recovery works, and it provides a lot of flexibility for us to improve.

But we throw all that out as soon as we're using the AST.

In our current design we have a limited set of "Bogus" (Unknown) nodes where we can say an error exists. But it doesn't provide us the flexibility to recover inside of the node. It also forces us to pretend an error is more wide-spread than it really should be.

JsProgram:
  - BogusJsStatement

But what if we made these bogus nodes more CST-like?

BogusJsStatement {
  kind: JsFunctionStatement,
  children: Vec<Node | Token | Error>,
}

This provides us with all of the flexibility of the CST:

// Give up immediately:
BogusJsStatement {
  kind: JsFunctionStatement,
  children: [
    JsFunctionKeyword("function"),
    Unexpected("%error% foo() {}"),
  ],
}

// Don't assume the foo is a JsIdentifierBinding:
BogusJsStatement {
  kind: JsFunctionStatement,
  children: [
    JsFunctionKeyword("function"),
    Unexpected("%error% foo"),
    JsParameterList(paren_open: "(", list: List(), paren_close: ")"),
    JsFunctionBody(brace_open: "{", brace_close: "}")
  ],
}

// Assume the foo is a JsIdentifierBinding:
BogusJsStatement {
  kind: JsFunctionStatement,
  children: [
    JsFunctionKeyword("function"),
    JsUnexpected("%error%"),
    IdentifierBinding(text: "foo"),
    JsParameterList(paren_open: "(", list: List(), paren_close: ")"),
    JsFunctionBody(brace_open: "{", brace_close: "}")
  ],
}

AST navigation

Let's take this AST:

JsProgram {
  items: [
    BogusJsStatement {
      kind: JsFunctionStatement,
      children: [
        JsFunctionKeyword("function"),
        JsUnexpected("%error%"),
        IdentifierBinding(text: "foo"),
        JsParameterList(paren_open: "(", list: List(), paren_close: ")"),
        JsFunctionBody(brace_open: "{", brace_close: "}")
      ],
    }
  ]
}

There are a number of different operations an analyzer could run on this:

find_a_node::<JsScript>() // Some(JsScript)
find_a_node::<JsScript>()?.items() // Iterator(AnyJsStatement)
find_a_node::<JsScript>()?.items().get(0) // Some(AnyJsStatement)
find_a_node::<JsScript>()?.items().get(0)?.cast::<JsFunctionStatement>() // Err(BogusJsStatement)
find_a_node::<JsScript>()?.items().get(0)?.cast::<BogusJsStatement>() // Ok(BogusJsStatement)
find_a_node::<JsFunctionStatement>() // None
find_a_node::<BogusJsStatement>() // Some(BogusJsStatement)
find_a_node::<JsFunctionBody>() // Some(JsFunctionBody)

To summarize:

• You can't visit the "function statement" because it doesn't exist, just a "bogus statement" we inferred was a function statement.
• You can visit the nodes inside the "bogus statement" because we recovered them.
• You can still access the BogusJsStatement.

While I think you should be able to access the BogusJsStatement, I don't think it should give you access to its children.

My biggest concern is that analyzers, particularly third-party ones, would eventually end up depending on indexes of the BogusJsStatement and it would make every change to our error recovery a breaking change.

Luckily, Rust allows us to make it so that those indexes are not a public API:

impl BogusJsStatement {
  pub(crate) fn children();
}

However, we should consider what types we return for child nodes trying to reach back up the tree into the bogus node.

find_a_node::<JsFunctionBody>().parent() // ???

FunctionBody::parent() could return any of:

• AnyJsNode
• Result<AnyJsNode, AnyBogusJsNode>
• AnyJsFunction | AnyBogusJsNode
• Some(AnyJsFunction)

I'm not sure which is the best choice, but the last two of those would require us to codegen .parent() methods everywhere.

[MichaReiser]

Luckily, Rust allows us to make it so that those indexes are not a public API:

This is true but I don't think it adds much value because authors can always fall back to bogus.syntax().children() and then continue to depend on offsets.

The question would then also be why we even need the function in the first place because it would then only be visible to the parser crate (or the future, an AST crate) but the parser has no interest in ever iterating over the AST. I assume what you wanted is to make this method "public to rome only". I don't know of a way to do that in Rust that has no "friendly assemblies" that are allowed to poke into internals.

FunctionBody::parent() could return any of:

The AST currently doesn't expose a parent() method. The only way to get a node's parent is to use body.syntax().parent() and it's then to you to cast it to the appropriate type (it returns a SyntaxNode<JsLangauge>). I don't see a reason for not adding it though.

Result<AnyJsNode, AnyBogusJsNode>

AnyBogusJsNode should be part of the AnyJsNode. These are possible JsNodes. It also breaks with the rest of our API where we carefully removed the need for Result to express that a node can either be a valid or a bogus node.

I'm not sure which is the best choice, but the last two of those would require us to codegen .parent() methods everywhere.

or write them by hand... The main challenge I see is that every union needs its dedicated name. What would you call AnyJsFunction | AnyBogusJsNode and how to defer the name inside of the codegen? What about syntaxes that have many possible different parents:

class_declaration.parent() -> ExportDeclarationClause | BlockStatement | FunctionBody 

We would probably need to have a JsAnyClassDeclarationParent but that will lead to many additional unions.

[ematipico]

For @jamiebuilds

My first question is: what's the different between an Unknown node and a Bogus node? Unless they are the same, then I didn't understand the whole proposal.

While I think you should be able to access the BogusJsStatement, I don't think it should give you access to its children.

How would that work with linters? If we decorate the whole statement - like in this example - in a bogus, analyzers won't be able see anything. Unless we will have analyzers with different access rights.

For @MichaReiser

or write them by hand...

I believe .parent() will end up to be a combination of manual code and generated code. A child that doesn't have a union as a parent, can be predicted without issues. For unions, probably we should provide a cast method, similar to what we do when we want to get a certain child.

[MichaReiser]

I believe .parent() will end up to be a combination of manual code and generated code. A child that doesn't have a union as a parent, can be predicted without issues. For unions, probably we should provide a cast method, similar to what we do when we want to get a certain child.

My main worry is about the number of types this will introduce. We also need to consider what parents a node can have if a AST is invalid. For example, a FunctionBody can be a child of an UnknownStatement if the function uses typescript syntax (or is invalid for other reasons).

[MichaReiser]

I spend some more time investing error recovery and explored what Swift's doing. These are my findings.

Error Recovery

This document is still using Unknown when referring to Bogus or Garbage nodes.

Desired properties

• Every child can be visited using the AST facade visitors, including errors. This prevents us from skipping tokens/nodes and appending them to the current parent because the facade then provides no means of accessing these nodes/tokens (issue with original facade).
• Don't insert nodes that weren't present in the original source tree
• It should be clear from the facade if one is operating on a valid or partially broken sub-tree.

Token Rules

• Leave an unexpected token if it may turn out to be a valid child later in this rule (or of one of the sub-rules). For example, we don't want to eat the ( just because it wasn't an identifier, we need it to match the parameter list later.
• Eat the token and convert to Unknown if the token isn't valid for this rule. For example, import 4 from "mod"; The number literal 4 isn't valid in any position of an import statement. So we should eat it, and continue parsing the rest of the import statement (but mark it as unknown). This is something our current parser infrastructure lacks support for (you can implement it manually but is rather annoying).

Rome Parser's approach

• Fixed set of Unknown* nodes, like UnknownStatement, UnknownExpression, UnknownModifier, UnknownImportNamedSpecifier, UnknownBinding, ...
• Adds missing placeholder for missing required children. Missing required children are handled in the AST when accessing the children (return Result<T, SyntaxError>)
• Change the kind of a node to Unknonw* if it contains any syntax error (except missing children)

Downsides

• Many specific Unknown nodes (one for every position where a syntax error can occur) or need to inline the content into the parent and mark the parent as unknown. Many different Unknown nodes also impact pass authors that need to handle them whenever they match on a union that contains an Unknown* kind.
• Only way to know if a node is valid is to call all of it's accessors and make sure non returns a MissingRequiredElement
• Unclear how to translate the SyntaxResult<T> to JavaScript
• Use of SyntaxResult<T> spreads through all the code using our AST even if it isn't ever planning to work on "invalid" nodes.

Swift's approach

• A generic Unknown node kind that covers all nodes with a syntax error (missing child, invalid identifier, mismatching child). The parent of an Unknown is always another Unknown node. This implies that the parents of an invalid node all are changed to Unknown nodes until...
• The tree reaches a specific Unknown* node, for example UnknownStmt or UnknownExpr. These Unknown* nodes act as error-boundaries and prevent that Unknown nodes bubble up all the way to the root.
• The parser may insert missing tokens, for example a closing } but never an opening {.
• The AST facade returns T for required children (no need for SyntaxResult) and Option<T> for optional children.

This is a mixture between Rome's parser and RSLint/Rust-analyzer's approach. It supports a generic Unknown node similar to the Error node in RSLint/Rust analyzer but restricts the places where such a node is valid, only inside of other Unknown nodes.

Upsides

• Limited set of Unknown* nodes: The generic Unknown node and a fixed set to contain errors (top-level elements: members, statements, expressions, binding/assignment?)
• Clear when a node is valid: if it's not an Unknown node.
• JS compatible AST Facade
• No "spread" of Result in AST consumers. They may or may not decide to use Result for their own needs.

Downsides

• All direct children of an Unknown node are no longer accessible. For example, an analyzer can only analyze the condition of the incomplete if statement if (cond) when the condition is modeled as its own Condition node. Modeling the condition as a distinct node ensures it remains intact even if the the kind of the if gets transformed to UnknownStatement.
• Bubbling requires additional support by the parser infrastructure (to avoid spreading let valid = ...). Swift verifies if the children of the constructed node matches the expected children (kind & presence) and changes the kind to the appropriate Unknown kind if not.
• Violates the rule that the parser never inserts anything that didn't exist in the original source code to recover even when closing parentheses, or commas are missing. It should be sufficient to limit the "insertion" to tokens only.
• Requires support for marking tokens as "missing" (if text is empty?).

Implementation Proposal

Ideally, the decision if a node should be wrapped inside of an Unknown node just because a child is missing or doesn't match the expected child shouldn't be scattered all around the parser code. That's why I believe we could do the following:

• SyntaxKind:
  • add is_unknown(&self) -> bool method to test if a kind is an unknown* kind
  • add as_unknown(&self) -> SyntaxKind method that converts the given kind to the best matching unknown kind:
    • Any stmt -> UnknownStatement
    • Any expression -> UnknownExpression
    • ...
    • Anything else -> Unknown
• TreeBuilder:
  • Add a validation step before building the actual node:
    • Calls into Language.fits_shape(kind, children: &[Option<SyntaxKind>]): bool that returns true if all mandatory children are present and match the types of the slots
    • Converts the node's kind to the corresponding unknown kind (kind.as_unknown()) if Language.fits_shape returns false
    • Requires a new trait TreeShape that Language must implement. Needs to be generated.
  • Ideally, we only call fits_shape if the node isn't already cached.
• Rename missing to empty to signal an empty slot
  • Rename ParsedSyntax.or_missing to or_empty
  • Rename p.missing to p.empty()
• Add methods for adding "missing" tokens
  • Add p.missing(kind): Inserts a missing token with the specified kind, an empty text (and range), and a source presence of Missing
  • Add p.expect_or_missing(kind): Inserts a missing token if the expected token isn't present.
• Remove ConditionalSyntax: Simply change the kind of a node to Unknown whenever a node has a SyntaxError. The TreeBuilder then takes care of converting all parents to their corresponding Unknown types. This also works with TS if we have a TypeScript.parse_exclusive_syntax(p, parse_func, error_builder) that parses the syntax, keeps the syntax as is if this is a TS file and otherwise adds the error to the node and changes its kind to Unknown.

Examples

Illegal Class ID

class await {
  a;
}

Rome Parser

- JsClassStatement:
    - class_token: 'class'
    - id: UnknownBinding
        - 'await'
    - l-paren: '{'
    - members: List
        - JsPropertyClassMember:
            - id: Identifier
                - 'a'
            - ';'
    - r-paren: '}'	

Swift

Assuming there's no UnknownBinding, because that is somewhat annoying to deal with when writing passes (requires to match).

- JsUnknownStatement:
    - 'class'
    - Unknown:
        - Unknown:
            - 'await'
    - '{'
    - List:
        - JsPropertyClassMember:
            - id: Identifier
                - 'a'
            - ';'
    - '}'	

Illegal Reference Identifier

await + test;

Rome Parser

- JsExpressionStatement:
    - expression: JsBinaryExpression
        - left: JsUnknownExpression
            - 'await'
        - operator: '+'
        - right: JsIdentifierExpression 
            - identifier: IdentifierReference
                - name: 'test'

Note: This is not the tree that the parser currently creates. It still emits an IdentifierReference node around the await keyword. This is wrong because it's mainly the reference that is invalid

Swift

- JsExpressionStatement:
    - expression: JsBinaryExpression
        - left: JsUnknownExpression
            - Unknown: 
                - 'await'
        - operator: '+'
        - right: JsIdentifierExpression 
            - identifier: IdentifierReference
                - name: 'test'

Keeps the structure "intact" without in-lining the ReferenceIdentifier

Variables

let a, [b];

Rome Parser

- JsVariableStatement:
    - kind: 'let'
    - declarations: List
        - VariableDeclaration:
            - id: JsIdentifierBinding
                - identifier: 'a'
        - ','
        - JsUnknownVariableDeclaration:
            - JsArrayPattern:
                - '['
                - elements: List
                    - JsIdentifierBinding
                        - identifier: 'b'
                - ']'

Swift

- JsUnknownStatement:
    - kind: 'let'
    - Unknown:
        - VariableDeclaration:
            - id: JsIdentifierBinding
                - identifier: 'a'
        - ','
        - Unknown:
            - JsArrayPattern:
                - '['
                - elements: List
                    - JsIdentifierBinding:
                        - identifier: 'b'
                - ']'

Expressions

a + b + 

Rome Parser

JsExpressionStatement:
- expression: JsBinaryExpression
    - left: JsBinaryExpression 
        - left: JsIdentifierExpression
            - identifier: JsIdentifierReference
                - name: 'a'
        - operator: '+'
        - right: JsIdentifierExpression
            - identifier: JsIdentifierReference
                - name: 'b'
    - operator: '+'
    - right: (missing required)

Swift

JsExpressionStatement:
  - expression: JsUnknownExpression
    - JsBinaryExpression:
      - left: JsIdentifierExpression
        - identifier: JsIdentifierReference (identifier)
          - name: 'a'
      - operator: '+'
      - right: JsIdentifierExpression
        - identifier: JsIdentifierReference
          - name: 'b'
  - '+'

Missing Members

{ , }

Rome Parser

- JsObjectExpression:
    - l-paren: '{'
    - members: AstSeparatedList:
        - missing element
        - ','
    - r-paren: '};		

AstSeparatedList.iter() returns Iterator<SyntaxResult>, missing elements must be handled by the pass author

Swift

- JsUnknownExpression
    - '{'
    - Unknown
        - missing element
        - ','
    - '};		

AstSeparatedList.iter() can return Iterator<T> because it's guaranteed that all elements are always present

Swift doesn't convert the list to Unknown if a comma is missing but neither inserts a comma token. This feels off to me, we should either insert the token and mark it as missing or mark the list as invalid. Funnily, the list becomes invalid as soon as two commas are missing.

Unexpected Token

import 4 from "mod"; 

Rome Parser

JsUnknownStatement:
- 'import_kw'
- 4
- 'from_kw'
- JsModuleSource:
    - source: '"mod"'

Not how the parser currently parses this

Swift

JsUnknownStatement:
- 'import_kw'
- Unknown:
    - 4
    - 'from_kw'
    - JsModuleSource:
        - source: '"mod"'

Better captures the original structure

Import Statements

import { a , b } from "mod" assert { type: "json", other: };

Rome Parser

JsImportStatement:
    - 'import'
    - JsNamedImportClause:
        - specifier_list: JsNamedImportSpecifierList
            - l_brack: '{'
            - specifiers: AstSeparatedList
                - JsNamedShorthandImportSpecifier:
                    -  JsBindingIdentifier:
                        - 'a'
                - ','
                - JsNamedShorthandImportSpecifier:
                    - JsBindingIdentifier:
                        - 'b'
            - r_brack: '}'
        - 'from'
        - JsModuleSource:
            source: "mod"
        - JsImportAssertion
            - assert_kw: 'assert'
            - l_brack: '{'
            - entries: AstSeparatedList
                - JsImportAssertionEntry:
                    - key: 'type'
                    - colon: ':'
                    - value: '"json"'
                - JsImportAssertionEntry:
                    - key: 'other'
                    - colon: ':'
                    - value: missing required
            - r_brack: '}'

Swift

JsUnknownStatement:
    - 'import'
    - Unknown:
        - JsNamedImportSpecifierList:
            - l_brack: '{'
            - specifiers: AstSeparatedList
                - JsNamedShorthandImportSpecifier:
                    -  JsBindingIdentifier:
                        - 'a'
                - ','
                - JsNamedShorthandImportSpecifier:
                    - JsBindingIdentifier
                        - 'b'
            - r_brack: '}'
        - 'from'
        - JsModuleSource:
            source: "mod"
        - Unknown
            - 'assert'
            - '{'
            - Unknown:
                - JsImportAssertionEntry:
                    - key: 'type'
                    - colon: ':'
                    - value: '"json"'
                - Unknown:
                    - 'other'
                    - ':'
            - '}'

The unknown bubbles all the way up to the statement, potentially to the point where many analyzers are no longer able to extract useful information.

An option would be to contain the error by introducing an JsUnkownImportAssertionEntry but this needs to be balanced well to avoid too many Unknown nodes.

Imports 2

import { a , a } from "mod";

Rome Parser

JsImportStatement:
    - 'import'
    - JsNamedImportClause:
        - specifier_list: JsNamedImportSpecifierList
            - l_brack: '{'
            - specifiers: AstSeparatedList
                - JsNamedShorthandImportSpecifier:
                    -  JsBindingIdentifier:
                        - 'a'
                - ','
                - JsUnknownNamedImportSpecifier:
                    - 'a'
            - r_brack: '}'
        - 'from'
        - JsModuleSource:
            source: "mod"

Swift

JsUnknownStatement:
    - 'import'
    - Unknown:
        - Unknown:
            - '{'
            - Unknown:
                - JsNamedShorthandImportSpecifier:
                    -  JsBindingIdentifier:
                        - 'a'
                - ','
                - Unknown:
                    - 'a'
            - r_brack: '}'
        - 'from'
        - JsModuleSource:
            source: "mod"

Summary

Swift's approach errors on the safe side by converting a node to an Unknown as soon as it contains a syntax error. This allows to remove the Result type from the AST facade and allows pass authors an easy way to determine if they're operating on a valid or partially valid node. The main downside is that the Unknown often bubble up until they hit a specific error node that acts as an error boundary, throwing away some information that the parser used to have before. This may become a problem for analzysers when nodes often switch between "valid"/"invalid" nodes while the user types, resulting in different diagnostics being reported with every keystroke.

I particularly like Swift's generic approach to determine when a node must be converted to an Unknown node by comparing the children with the AST schema. I belive replacing our ConditionalSyntax with this generic approach would grately simplify our parser regardless if we adopt Swift's approach or not. However, it will mean that we also must introduce a generic Unknown node for the cases when a node without a specific Unknown node contains errornous content.

[MichaReiser]

To expand a bit on the inferred kind which i start to like more and more.

One possible implementation could be to add a has_diagnostic flag to GreenNode (together with a contains_diagnostic flag as wished for by @yassere). The kind method of SyntaxNode<L> can check if has_diagnostic is true and in that case return raw.kind().to_unknown() (convert the node to an unknown node at runtime rather compile-time, to preserve the original kind without any additional storage cost). SyntaxKind<L> can then expose an additional potential_kind, possible_kind or inferred_kind that returns the kind as inferred by the parser.

Inside the parser, we could add a Marker.complete_with_diagnostic(p, kind) method that pushes a FinishNode event with a HasDiagnostic flag set to true. We should also add the has_diagnostic to the CompletedMarker because there are some cases where the parser only wants to add another diagnostic if the completed marker has none yet (for example, a parsed identifier await already has a diagnostic, we don't want to add any other diagnostic for it).

The generated JsUnknownNodes would also expose a potential_kind, possible_kind, or inferred_kind method.

This gives us the benefits of both approaches. The main downsides I see are:

• Changing the kind inside of the SyntaxNode accessor may feel weird if the underlying GreenNode stores another kind.
• There's a risk that many analyzers will have to implement both cases: when the node is an Unknown node and when it is a valid node. The upside is that they may be more aware that they're in "unsafe" territory when working on an Unknown node.
• People may be inclined to overuse potential_kind and poke into children using absolute offsets.

[sebmck]

Meaning, our analysis would report the usages of c (and probably even b) as undefined variables inside of the console statement except if our analysis pokes around in the UnknownStatement, which I believe they shouldn't.

I might challenge this, is there any reason you think they shouldn't? I also assume you mean any unknown nodes shouldn't be used for inference or analysis.

[MichaReiser]

I'm not entirely following. Would it be your expectations that they're reported as undeclared when the type annotation is incomplete? My worry would then be that the variables are declared when the user typed function a(b: number, c) {... } but then suddenly become invalid when the user added the : to add the type annotation of c.

I also assume you mean any unknown nodes shouldn't be used for inference or analysis.

The children can be used inside of analysis. For example, the function's body is in itself valid. It mainly is that extracting any information of an unknown node is somewhat painful because it's unstructured.

[MichaReiser]

I realised after talking with @yassere yesterday that there are two questions involved and it seems @yassere and I are aligned on one of them.

Question 1: Expose the assumed kind

The first question is two-folded and conceptually:

1. Do we still want to go for marking any node with a missing child as an unknown node, and, therefore, change the return type for mandatory children from SyntaxResult<T> to T
2. Do we believe it's useful to expose the assumed_kind on the SyntaxNode and on JsUnknown* nodes? We may later decide to build a dedicated facade that allows operating on a tree with weaker guarantees, or extend our SyntaxNode operations (e.g., add a find_token(kind) etc). The guarantees that are no longer true for such a facade are: 1) children may be missing, 2) children may not have the expected types, 3) there may be more children than expected.

I answer both of these questions as yes because I believe that having these stricter guarantees is useful for many analyzers, but, there are also use cases where we deliberately must trade the convenience of a stricter AST to provide better suggestions (or analysis). Keeping the two apart (or asking users to operate on SyntaxNode) makes it explicit to analyzer authors that they can make fewer assumptions when operating on the weaker tree.

Question 2: How to store the assumed kind

This is a pure implementation question. Exposing the assumed kind for unknown nodes requires that these are stored on the GreenNode. I currently see the following options.

1. Add an additional field to the GreenNode header: This is probably the most straightforward implementation. All it needs is to add another RawSyntaxKind field to the GreenNodeHeader and then expose it on GreenNode and SyntaxNode (and provide means to populate it). The downside is that it increases the size of every GreenNodeHeader by at least 2 bytes (depending on alignment).
2. Store inside of a heap-allocated data structure: Add a data field to GreenNodeHeader that is an optional pointer to additional, but optional data stored for a GreenNode. This may also become handy when we implement tracked and annotated nodes. This approach feels a bit pre-mature in my view as we don't know if and how we want to implement tracked and annotated nodes.
3. Use diagnostics flag: Store a GreenNodeFlags alongside with every node. There would currently be two flags: HAS_DIAGNOSTIC and CONTAINS_DIAGNOSTIC. The GreenNode would always store its assumed kind, but the SyntaxNode kind accessor would convert the kind to an unknown kind if the HAS_DIAGNOSTIC flag is set for this node. It may seem odd to do such a conversion inside of the kind accessor but isn't this similar to the RawSyntaxKindtoLanguage::Kind` conversion we do today? The only difference is that it also takes the flags into account. The main benefit of this approach is that it isn't more storage-intensive than a) but allows us to store additional metadata by supporting flags.

[MichaReiser]

Let's continue my monologue... haha :D

I spent some more time thinking about analyzers, their requirements, and whatever they should operate on unknown/bogus nodes or not.

Our overreaching goal should be to help analyzer authors to avoid false positives:

1. a == should only report an error that the right-hand side of the expression is missing but not that === should be used over ==. Reporting both errors make it difficult to provide an autofix, especially because changing == to === may need to be undone if either side turns out to be null.
2. for (let a =; a < 10; ++a) { console.log(a); } shouldn't report an error for the usage of a in the console statement just because the initializer hasn't been finished.
   3.import {a} from "mod": The import statement should remain valid even if the export statement of a in module mod contains a syntax error. Not doing so would overwhelm the user with tons of errors just because they made a simple change inside of the module.
3. An implementation of the (constructor-super)[https://eslint.org/docs/rules/constructor-super] shouldn't report diagnostics for super calls just because the ExtendsClause has a syntax error (e.g. class A extends). These errors would only distract from the real error, that the user must finish the extends clause

For the analyzer depending on other nodes it's actually important that extracting that "dependent" data is an over-estimation to avoid false positives. This means for example 2. that we should extract all bindings, regardless of whatever they're valid or not. The same is true for cases 3-4. That means, extracting dependent information must also work for nodes with syntax errors.

Another way to think about this is that to prevent false-positives for analyzer that report a diagnostic if another node isn't present requires to over-estimate and also consider nodes with syntax errors.

Does that mean, that analyzers that depend on the presence of another node should only consider valid nodes? Maybe, I don't know yet.