AST node ranges are not always correct about coffeescriptredux HOT 7 CLOSED

It's very strange that we get the raw value correct but the range incorrect. Especially since we're just using PEG.js's line, column, and offset functions. Unfortunately, this is in one of the hairier parts of the code (read: there's a nasty hack to skip counting of context-free-CoffeeScript control characters).

from coffeescriptredux.

Just in case it's helpful to you or anyone else who runs into this, here's what I'm using to fix the ranges. It's probably pretty inefficient but so far it's been sufficient for my needs:

// Visit every node to fix its range, assumes each node has a
// `_parent` property that refers to its parent node. This is intended
// to operate on the result of `toBasicObject()` on the parsed AST.
traverse(ast, function(node) {
  if (!node.range) { return; }

  if (node._parent) {
    node.range[0] += node._parent._offset;
    node.range[1] += node._parent._offset;
  }

  if (node.raw !== source.slice(node.range[0], node.range[1])) {
    node._offset = fixRange(node, source);
  } else {
    node._offset = 0;
  }
});

function fixRange(node, source) {
  var index = -1;
  var expectedStart = node.range[0];
  var minimumOffsetIndex = -1;
  var minimumOffset = Infinity;

  while ((index = source.indexOf(node.raw, index + 1)) >= 0) {
    if (minimumOffsetIndex < 0) {
      minimumOffsetIndex = index;
      minimumOffset = Math.abs(expectedStart - minimumOffsetIndex);
    } else {
      var thisOffset = Math.abs(expectedStart - index);
      if (thisOffset < minimumOffset) {
        minimumOffset = thisOffset;
        minimumOffsetIndex = index;
      }
    }
  }

  if (minimumOffsetIndex < 0) {
    throw new Error('unable to find location for node: ' + JSON.stringify(node.raw));
  }

  node.range[0] = minimumOffsetIndex;
  node.range[1] = minimumOffsetIndex + node.raw.length;

  return minimumOffsetIndex - expectedStart;
}

from coffeescriptredux.

Actually, it looks like it's even worse. Some nodes that ought to have range and raw do not have either, such as when parsing "a#{b}":

> parse('"a#{b}"', { raw: true }).toBasicObject().body.statements[0]
{ type: 'ConcatOp',
  line: 1,
  column: 1,
  raw: '"a#{b}"',
  range: [ 0, 7 ],
  left:
   { type: 'String',
     line: 1,
     column: 2,
     data: 'a' },
  right:
   { type: 'Identifier',
     line: 1,
     column: 5,
     raw: 'b',
     range: [ 4, 5 ],
     data: 'b' } }

from coffeescriptredux.

Okay, I think I've figured out why this is happening. I'm pretty sure it's due to the "context-free-CoffeeScript control characters" you mentioned. When an input string is pre-processed a number of characters are added, so this:

->
  a.b

Becomes this:

->
  a.b

Those extra characters are what is causing the range property to be incorrect, as it's reporting the range inside the preprocessed input, not the original input. I can think of a few ways to work around this, but I'm not sure how to fix it. Thoughts, @michaelficarra?

from coffeescriptredux.

Ah yes, now I remember. We fixed that issue in #117 (1f314eb) (though, using an extremely hacky approach). I would not be surprised if it broke when upgrading PEGjs, if not for the fact that we do have tests for it. I still can't think of a better way to solve this problem than the hack from #117.

from coffeescriptredux.

I don't believe the problem is exactly the same. That commit does in fact fix line and column numbers, but doesn't fix offsets. I have a workaround from a commit that links to this issue that basically recomputes the range info based on the line and column numbers.

Am I missing something?

from coffeescriptredux.

Oh you're right, I didn't notice it only fixed line/column but not offset.

Your commit looks awesome! Would we be able to also get rid of that nasty hack in the parser?

edit: In case it wasn't clear, I mean integrating that lookup table in the CSR parser and getting rid of the overridden PEGjs function hack.

from coffeescriptredux.