anko / eslisp

un-opinionated S-expression syntax and macro system for JavaScript
ISC License
528 stars 31 forks source link

Cannot Assign Arrow Function Macro #63

Open popeguy opened 4 months ago

popeguy commented 4 months ago

My arrow function macro is as follows:

(macro =>
 (function ()
           (= env this)
           (= atoms ((. Array from) (. arguments 0 values)))

  (= propertyIdentifiers
     ((. atoms map)
      (function (atom) (return ((. env compile) atom)))))

  (= properties ((. propertyIdentifiers map)
                   (function (identifier)
                    (return (object type "Property"
                                    key identifier
                                    value identifier
                                    shorthand true)))))

  (= estree (object 
                                  type "ArrowFunctionExpression"
                                  generator false
                                  async false
                                  params properties
                                  body ((. this compile) (. arguments 1))))

  (return estree)))

I show the Ecmascript it produces with:

(macro showEcma
 (lambda (expression)
    (return `((. console log)
              ,((. this string)
                ((. this compileToJs) 
                 ((. this compile) expression)))))
    (return null)))

And I show the AST representation with:

(macro showAST
       (lambda (expression)
               (return ((. console log)                         
                           ((. this compile) expression))))) 

When I run :

    (showEcma (= var1
             (=> (a b c)
                 (block ((. console log) a)
                        ((. console log) b)
                        ((. console log) c)))))

I get:

var1 = (a, b, c) => {
    console.log(a);
    console.log(b);
    console.log(c);
};

When I showAST I get:

{
  type: 'AssignmentExpression',
  operator: '=',
  left: {
    type: 'Identifier',
    name: 'var1',
    loc: { start: [Object], end: [Object] }
  },
  right: {
    type: 'ArrowFunctionExpression',
    generator: false,
    async: false,
    params: [ [Object], [Object], [Object] ],
    body: { type: 'BlockStatement', body: [Array], loc: [Object] },
    loc: { start: [Object], end: [Object] }
  },
  loc: {
    start: { offset: 22, line: 1, column: 22 },
    end: { offset: 23, line: 1, column: 23 }
  }
}

Looks good to me. But when I go live with it:

(= var1 (=> (a b c)
                 (block ((. console log) a)
                        ((. console log) b)
                        ((. console log) c))))

I get:

[Error] AssignmentExpression `right` member must be an expression node
At line 1, offset 1:

Any ideas?

Kevin

anko commented 4 months ago

Hi, and thanks for your bug report.

I haven't looked at this project's code in a while, so I'll talk through my thoughts:

That error comes from esvalid, which is the package used internally by the compiler here to sanity-check any AST before it's fed to escodegen for code generation, because escodegen doesn't do validation.

I'm doing this validation because writing macros that construct estree is complex enough as-is, so I'd like to catch and display errors early, for users' benefit.

However, esvalid hasn't been updated for a long time, so it doesn't support newer estree spec additions, such as arrow functions, even though escodegen can generate code for them. So in a792d031ff59a16b7ed80aba59fc72e4fad7d58a I introduced this filter which is supposed to ignore esvalid errors relating to nodes that it doesn't support. It's not ideal, but the intention is to ignore errors with the unsupported new stuff, while retaining useful errors for estree nodes which validation is supported.

These are the errors for your code, before filtering:

[
  [InvalidAstError: AssignmentExpression `right` member must be an expression node] {
    node: {
      type: 'AssignmentExpression',
      operator: '=',
      left: [Object],
      right: [Object],
      loc: [Object]
    }
  },
  [InvalidAstError: unrecognised node type: "ArrowFunctionExpression"] {
    node: {
      type: 'ArrowFunctionExpression',
      generator: false,
      async: false,
      params: [Array],
      body: [Object],
      loc: [Object]
    }
  }
]

It looks like when an AssignmentExpression has an estree type that esvalid doesn't recognise (such as a ArrowFunctionExpression), it reports two errors: one for the unrecognised estree type (which gets removed by my filter), and also one for the AssignmentExpression that contains it (which isn't removed by my filter, because esvalid supports that estree type).

My error-filter is based on the false assumption that estree would only report an error for the unrecognised estree type in such cases.


I'm leaning toward fixing this by removing esvalid altogether. It is badly out of date, and it's clear that filtering out its errors is more cumbersome than I expected.

That would mean invalid estree would be passed to escodegen. That means it may generate invalid JS, or accidentally generate valid JS for invalid estree. Those problems would not be caught at compile-time. But at least then valid programs like yours would be accepted.

I may get around to that at some point. If you get to it first, I would welcome a PR.


In a perfect world, esvalid (or a fork) would support the full and current estree spec, and we could validate it all. But that would be quite a lot of really boring work.