`_ParseBinaryExpr` builds a node whose range excludes it's children

[vibbix]

Fix for this is here: #514

Background

Been playing around with wkillerud/some-sass and ran into a issue upstream with it. While trying to figure out how to add in support for CSS Custom Properties, I noticed that some of my nested calculations I was testing with failed to properly be identified. I traced it up, and saw the issue was somewhere in VSCode's CSS Language Server, which didn't properly return a range for a calculation. This was verified from the tip of main branch, as well as in the last stable (as well locally from some-sass).

Bug Description

When a value expression (i.e. calc(...)) contains two or more binary operators, the second call into _parseBinaryExpr produces a BinaryExpression, whose offset starts after the subtree's left operand. This means that the range of the parent does contain contains the childs, and getNodeAtOffset cannot look at anything left of it's second operator.

Impact

• Go To Definition doesn't work
• Document Highlights - where it should be 2 highlights, its just one.

Example

.a { width: calc(2 * var(--x) + 1); }

If you put your cursor on --x and ask for a definition, getNodeAtOffset returns Expression instead of Identifier.

Cause

Malformed tree

Expression         [12..34] "calc(2 * var(--x) + 1)"
 BinaryExpression  [12..34] "calc(2 * var(--x) + 1)"
  Term             [12..34] "calc(2 * var(--x) + 1)"
   Function        [12..34] "calc(2 * var(--x) + 1)"
    Identifier     [12..16] "calc"
    Nodelist       [17..33] "2 * var(--x) + 1"
     FunctionArgument [17..33] "2 * var(--x) + 1"
      Expression   [17..33] "2 * var(--x) + 1"
       BinaryExpression [32..33] "1"              <-- range covers only "1"
        BinaryExpression [17..29] "2 * var(--x)"  <-- but this is its child
         Term      [17..18] "2"
         Node      [19..20] "*"
         Term      [21..29] "var(--x)"
          Function [21..29] "var(--x)"
           Identifier [21..24] "var"
           Nodelist [25..28] "--x"
            ...
             Identifier [25..28] "--x"            <-- unreachable
        Node       [30..31] "+"
        Term       [32..33] "1"

However, if we dropped + 1 from the above calculation, it runs fine. In that case, it returns a BinaryExpression from [17..29], which means getNodeAtOffset(26) returns Identifier.

Code Spot

Issue occurs here at cssParser.ts:2012-2035

	public _parseBinaryExpr(preparsedLeft?: nodes.BinaryExpression, preparsedOper?: nodes.Node): nodes.BinaryExpression | null {
		let node = this.create(nodes.BinaryExpression);

		if (!node.setLeft((<nodes.Node>preparsedLeft || this._parseTerm()))) {
			return null;
		}

		if (!node.setOperator(preparsedOper || this._parseOperator())) {
			return this.finish(node);
		}

		if (!node.setRight(this._parseTerm())) {
			return this.finish(node, ParseError.TermExpected);
		}

		// things needed for multiple binary expressions
		node = <nodes.BinaryExpression>this.finish(node);
		const operator = this._parseOperator();
		if (operator) {
			node = <nodes.BinaryExpression>this._parseBinaryExpr(node, operator);
		}

		return this.finish(node);
	}

It's not caught downstream because in cssNodes.ts:124, any branch whose range is outside the offset gets pruned. For the example above, the offset at 26 just gets dropped.

Further examples

expression	getNodeAtOffset on --x
calc(100% - var(--x))	Identifier
calc(100% - 2 * var(--x))	Identifier
calc(1 + 2 + var(--x))	Identifier
calc((100% - var(--x)) * 2)	Identifier
calc(2 * var(--x) + 1)	Expression
calc(var(--x) * 2 / 3)	Expression
calc(-1 * var(--x) + 1 * 2)	Expression
calc((100% - var(--x)) * 2 / 3)	Expression

Suggested Fix

I'm no expert here in languages servers - I'd wager the scanner mark should be passed earlier in, so it doesn't get overshot. That way, the new node begins where the left operand is, as opposed to where the scanner currently is.

LLM disclosure

I have Claude Opus 5 @ medium help me trace it up, and write some examples of other places this occurs.

[vibbix]

Work has been merged in!