From fbc77f744549d4377cc4eb02acf3b6331118d8dc Mon Sep 17 00:00:00 2001
From: Alan Pierce <alangpierce@gmail.com>
Date: Sun, 12 Feb 2017 18:07:07 -0800
Subject: [PATCH] Properly update location data when setting a call to use
 `new`

This is an upstream port of https://github.com/decaffeinate/coffeescript/pull/24

In a case like `new A().b(c)`, the jison structure ends up being different from
the resulting AST. To the jison parser, this is the `new` unary operator applied
to the expression `A().b(c)`. When the unary operator is applied, the
`Call.prototype.newInstance` function traverses into the leftmost function call
and sets the `isNew` flag to true, and the `Op` constructor returns the `Call`
node so that the call is used in place of the unary operator. However, the code
wasn't updating the node location data, so this commit fixes that.

It's sort of hard to get the location data in `newInstance`, so we set a flag on
every affected node in `newInstance` and override `updateLocationDataIfMissing`
(which is called with the location data after the fact) so that it updates just
the starting position.
---
 lib/coffee-script/nodes.js | 17 ++++++++++++++++
 src/nodes.coffee           | 16 +++++++++++++++
 test/parser.coffee         | 40 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+)

diff --git a/lib/coffee-script/nodes.js b/lib/coffee-script/nodes.js
index a87a386c..d64c8c94 100644
--- a/lib/coffee-script/nodes.js
+++ b/lib/coffee-script/nodes.js
@@ -1024,6 +1024,22 @@
 
     Call.prototype.children = ['variable', 'args'];
 
+    Call.prototype.updateLocationDataIfMissing = function(locationData) {
+      var base, ref3;
+      if (this.locationData && this.needsUpdatedStartLocation) {
+        this.locationData.first_line = locationData.first_line;
+        this.locationData.first_column = locationData.first_column;
+        base = ((ref3 = this.variable) != null ? ref3.base : void 0) || this.variable;
+        if (base.needsUpdatedStartLocation) {
+          this.variable.locationData.first_line = locationData.first_line;
+          this.variable.locationData.first_column = locationData.first_column;
+          base.updateLocationDataIfMissing(locationData);
+        }
+        delete this.needsUpdatedStartLocation;
+      }
+      return Call.__super__.updateLocationDataIfMissing.apply(this, arguments);
+    };
+
     Call.prototype.newInstance = function() {
       var base, ref3;
       base = ((ref3 = this.variable) != null ? ref3.base : void 0) || this.variable;
@@ -1032,6 +1048,7 @@
       } else {
         this.isNew = true;
       }
+      this.needsUpdatedStartLocation = true;
       return this;
     };
 
diff --git a/src/nodes.coffee b/src/nodes.coffee
index 56b5cdc3..d4f727f1 100644
--- a/src/nodes.coffee
+++ b/src/nodes.coffee
@@ -633,6 +633,21 @@ exports.Call = class Call extends Base
 
   children: ['variable', 'args']
 
+  # When setting the location, we sometimes need to update the start location to
+  # account for a newly-discovered `new` operator to the left of us. This
+  # expands the range on the left, but not the right.
+  updateLocationDataIfMissing: (locationData) ->
+    if @locationData and @needsUpdatedStartLocation
+      @locationData.first_line = locationData.first_line
+      @locationData.first_column = locationData.first_column
+      base = @variable?.base or @variable
+      if base.needsUpdatedStartLocation
+        @variable.locationData.first_line = locationData.first_line
+        @variable.locationData.first_column = locationData.first_column
+        base.updateLocationDataIfMissing locationData
+      delete @needsUpdatedStartLocation
+    super
+
   # Tag this invocation as creating a new instance.
   newInstance: ->
     base = @variable?.base or @variable
@@ -640,6 +655,7 @@ exports.Call = class Call extends Base
       base.newInstance()
     else
       @isNew = true
+    @needsUpdatedStartLocation = true
     this
 
   # Soaked chained invocations unfold into if/else ternary structures.
diff --git a/test/parser.coffee b/test/parser.coffee
index 33fbff1e..fce29393 100644
--- a/test/parser.coffee
+++ b/test/parser.coffee
@@ -38,3 +38,43 @@ test "operator precedence for binary ? operator", ->
   eq expression.second.first.base.value, 'b'
   eq expression.second.operator, '&&'
   eq expression.second.second.base.value, 'c'
+
+test "new calls have a range including the new", ->
+  source = '''
+    a = new B().c(d)
+  '''
+  block = CoffeeScript.nodes source
+
+  assertColumnRange = (node, firstColumn, lastColumn) ->
+    eq node.locationData.first_line, 0
+    eq node.locationData.first_column, firstColumn
+    eq node.locationData.last_line, 0
+    eq node.locationData.last_column, lastColumn
+
+  [assign] = block.expressions
+  outerCall = assign.value
+  innerValue = outerCall.variable
+  innerCall = innerValue.base
+
+  assertColumnRange assign, 0, 15
+  assertColumnRange outerCall, 4, 15
+  assertColumnRange innerValue, 4, 12
+  assertColumnRange innerCall, 4, 10
+
+test "location data is properly set for nested `new`", ->
+  source = '''
+    new new A()()
+  '''
+  block = CoffeeScript.nodes source
+
+  assertColumnRange = (node, firstColumn, lastColumn) ->
+    eq node.locationData.first_line, 0
+    eq node.locationData.first_column, firstColumn
+    eq node.locationData.last_line, 0
+    eq node.locationData.last_column, lastColumn
+
+  [outerCall] = block.expressions
+  innerCall = outerCall.variable
+
+  assertColumnRange outerCall, 0, 12
+  assertColumnRange innerCall, 4, 10