jlongster / es6-macros

A collection of sweet.js macros that implement ES6 features for ES5
BSD 2-Clause "Simplified" License
238 stars 18 forks source link

Add a failing test for handling `arguments` correctly. #12

Closed eventualbuddha closed 10 years ago

eventualbuddha commented 10 years ago

Per the relevant section of the ES6 spec, arguments inside an arrow function refers to the arguments in the lexically enclosing function.

I tried to fix this using a case infix macro, but I don't understand it well enough to know exactly how to start.

jlongster commented 10 years ago

Nice, I did not know that about arrow functions. I will think about the best way to do this. I think we'll need a few helper macros, and only one of them needs to be a small case macro: the one that detects if arguments exists. Will follow up on this.

jlongster commented 10 years ago

This has been fixed in 74695b85285830b6175fdf69c2d3f0844280fecc

https://github.com/jlongster/es6-macros/blob/master/macros/fat-arrow.sjs

eventualbuddha commented 10 years ago

Thanks, @jlongster! I'm glad you found a solution. Unfortunately, I'm going to make it harder. Given this code:

var a = () => function(){ return arguments; }

You get this:

var a = function (__fa_args) {
        return function () {
            return __fa_args;
        };
    }.bind(this, arguments);

Which is incorrect. The arguments reference inside the inner function should not be re-written. Also, matching arguments as an identifier is a little too aggressive:

var a = () => b.arguments;

Becomes:

var a = function (__fa_args) {
        return b.__fa_args;
    }.bind(this, arguments);
jlongster commented 10 years ago

My quick 2AM hacking was obviously not thought through. There's actually a pretty elegant solution that involves much better parsing. I'll fix soon.

jlongster commented 10 years ago

Fixed on master.