Skip to content

bug: Poor error recovery when content is added inside a block #65

@savetheclocktower

Description

@savetheclocktower

Did you check existing issues?

  • I have read all the tree-sitter docs if it relates to using the parser
  • I have searched the existing issues of tree-sitter-css

Tree-Sitter CLI Version, if relevant (output of tree-sitter --version)

No response

Describe the bug

The problem

Consider this selector:

div {
  display: flex;
}
stylesheet (0, 0) – (4, 0)
  rule_set (0, 0) – (2, 1)
    selectors (0, 0) – (0, 3)
      tag_name (0, 0) – (0, 3)
    block (0, 4) – (2, 1)
      declaration (1, 2) – (1, 16)
        property_name (1, 2) – (1, 9)
        plain_value (1, 11) – (1, 15)

Now suppose I want to add a new declaration above display: none;. I insert a line break and start typing:

div {
  padding
  display: flex;
}
stylesheet (0, 0) – (5, 0)
  rule_set (0, 0) – (3, 1)
    selectors (0, 0) – (0, 3)
      tag_name (0, 0) – (0, 3)
    block (0, 4) – (3, 1)
      ERROR (1, 2) – (2, 16)
        descendant_selector (1, 2) – (2, 9)
          tag_name (1, 2) – (1, 9)
          tag_name (2, 2) – (2, 9)
        ERROR (2, 11) – (2, 15)

tree-sitter-css does a naive thing here: since descendant combinators can technically tolerate a line break as their whitespace (I think?), it assumes it's finding a selector called padding display, and doesn't reassess its conclusion when it hits the : (colon plus space, which rules out a pseudoclass).

If you add a colon, the tree changes:

div {
  padding:
  display: flex;
}
stylesheet (0, 0) – (5, 0)
  rule_set (0, 0) – (3, 1)
    selectors (0, 0) – (0, 3)
      tag_name (0, 0) – (0, 3)
    block (0, 4) – (3, 1)
      declaration (1, 2) – (2, 16)
        property_name (1, 2) – (1, 9)
        plain_value (2, 2) – (2, 10)
        plain_value (2, 11) – (2, 15)

Luckily, if you type a ;, the error goes away (which is maybe too aggressive, considering it's not valid CSS):

div {
  padding:;
  display: flex;
}
stylesheet (0, 0) – (5, 0)
  rule_set (0, 0) – (3, 1)
    selectors (0, 0) – (0, 3)
      tag_name (0, 0) – (0, 3)
    block (0, 4) – (3, 1)
      declaration (1, 2) – (1, 11)
        property_name (1, 2) – (1, 9)
        integer_value (1, 10) – (1, 10)
      declaration (2, 2) – (2, 16)
        property_name (2, 2) – (2, 9)
        plain_value (2, 11) – (2, 15)

In Pulsar, this results in distracting re-highlighting of input, as illustrated below:

tree-sitter-css-inside-block-parsing.mp4

Unfortunately, it applies to every property after the cursor until : is inserted — they're all incorrectly reinterpreted as tag names.

The fix

In general, I think it's a good goal for incomplete input on one line to generate an ERROR node but to somehow recover before the valid input on the next line. Easier said than done, of course.

It used to be easier to rule out this scenario, but CSS nesting makes it so that you really could have valid CSS like the following:

div {
  padding display {
    display: flex;
  }
}

It's implausible in this specific case, but I don't think the solution is to try to whitelist tag names or property names (we'd always be playing catch-up).

Maybe this is a job for conflictssomehow? Conceptually, it seems like the best way to proceed is on two parallel paths. If { is encountered first, the “interpret as a selector” path wins. If ; is encountered first (or something else that rules out a selector, like : , then the “interpret as an incomplete property-value pair” path wins.

Steps To Reproduce/Bad Parse Tree

stylesheet (0, 0) – (5, 0)
  rule_set (0, 0) – (3, 1)
    selectors (0, 0) – (0, 3)
      tag_name (0, 0) – (0, 3)
    block (0, 4) – (3, 1)
      ERROR (1, 2) – (2, 16)
        descendant_selector (1, 2) – (2, 9)
          tag_name (1, 2) – (1, 9)
          tag_name (2, 2) – (2, 9)
        ERROR (2, 11) – (2, 15)

Expected Behavior/Parse Tree

stylesheet (0, 0) – (5, 0)
  rule_set (0, 0) – (3, 1)
    selectors (0, 0) – (0, 3)
      tag_name (0, 0) – (0, 3)
    block (0, 4) – (3, 1)
      ERROR (1, 2) – (1, 9)
        identifier (1, 2) – (2, 9)
      declaration (2, 2) – (2, 16)
        property_name (2, 2) – (2, 9)
        plain_value (2, 11) – (2, 15)

Repro

div {
  padding
  display: flex;
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions