rspivak / slimit

SlimIt - a JavaScript minifier/parser in Python
MIT License
552 stars 94 forks source link

Lose "()" after parse and cause Syntax error in js #114

Open forthlsss opened 4 years ago

forthlsss commented 4 years ago

here is a content:

function _fAddItem(_aoContainer, _aoItem, _anIndex) { ({}).toString.call([]) != "[object Array]" && (_aoItem = [_aoItem]); return _aoContainer.slice(0, _anIndex).concat(_aoItem).concat(_aoContainer.slice(_anIndex, _aoContainer.length)); };

and this is my code

from slimit.parser import Parser
parser = Parser()
tree = parser.parse(content)
print tree.to_ecma()

and then i got this

function _fAddItem(_aoContainer, _aoItem, _anIndex) { { }.toString.call([]) != "[object Array]" && (_aoItem = [_aoItem]); return _aoContainer.slice(0, _anIndex).concat(_aoItem).concat(_aoContainer.slice(_anIndex, _aoContainer.length)); } ; I found that the "()" in the "({}}"" in line 3 of the content is gone and casued Syntax error in js i wonder which part of code what reason caused this. I have not learned lexer, parser or something,i want to know whether i have the ability to learn and fix it by myself or not ,and if it will takes me too much time. I will be appreciate if someone can help

metatoaster commented 4 years ago

The package calmjs.parse, a fork of slimit I have created, will correctly parse the grouping operator (the cause of the problem here in slimit). Following is a similar example using calmjs.parse:

>>> content = '''function _fAddItem(_aoContainer, _aoItem, _anIndex)
... {
... ({}).toString.call([]) != "[object Array]" && (_aoItem = [_aoItem]);
... return _aoContainer.slice(0, _anIndex).concat(_aoItem).concat(_aoContainer.slice(_anIndex, _aoContainer.length));
... };'''
>>> from calmjs.parse import es5
>>> tree = es5(content)
>>> print(tree)
function _fAddItem(_aoContainer, _aoItem, _anIndex) {
  ({}).toString.call([]) != "[object Array]" && (_aoItem = [_aoItem]);
  return _aoContainer.slice(0, _anIndex).concat(_aoItem).concat(_aoContainer.slice(_anIndex, _aoContainer.length));
}
;
forthlsss commented 4 years ago

The package calmjs.parse, a fork of slimit I have created, will correctly parse the grouping operator (the cause of the problem here in slimit). Following is a similar example using calmjs.parse:

>>> content = '''function _fAddItem(_aoContainer, _aoItem, _anIndex)
... {
... ({}).toString.call([]) != "[object Array]" && (_aoItem = [_aoItem]);
... return _aoContainer.slice(0, _anIndex).concat(_aoItem).concat(_aoContainer.slice(_anIndex, _aoContainer.length));
... };'''
>>> from calmjs.parse import es5
>>> tree = es5(content)
>>> print(tree)
function _fAddItem(_aoContainer, _aoItem, _anIndex) {
  ({}).toString.call([]) != "[object Array]" && (_aoItem = [_aoItem]);
  return _aoContainer.slice(0, _anIndex).concat(_aoItem).concat(_aoContainer.slice(_anIndex, _aoContainer.length));
}
;

Thank you very much, I tried this method, it works very well. But i found that after es5(), I got a <class'calmjs.parse.factory.ES5Program'>, and slemit.parser.Parser() got <class'calmjs.parse.factory.ES5Program'>. This caused some problems because I used a lot of APIs in slimit.ast in rest of the code.Mainly some type judgments such as if isinstance(node, ast.Default). So it can't fit now. I browsed calmjs.parse.asttypes but they are some differences. Is there a good solution to this problem? Or do I have to modify my code for calmjs.Thanks again.

metatoaster commented 4 years ago

The calmjs.parse.asttypes module originated from slimit.ast, although there are certain differences between certain classes as there were small amounts of assumptions made in slimit that does not reflect the actual ECMA-262 5.1 specification (most notably with switch/default/case statements and issue like this with the grouping operator). Given how something like if isinstance(node, ast.Default) is used, here is an example of how one might walk through the AST to get Default nodes:

from calmjs.parse import es5
from calmjs.parse import asttypes as ast
from calmjs.parse.walkers import Walker

tree = es5(source)
walker = Walker()
for node in walker.filter(tree, lambda node: isinstance(node, ast.Default)):
    print(node)  # just print them out as a demo

Otherwise the "visitor" pattern that slimit uses mostly works, aside from the renamed classes, such as Program -> ES5Program as mentioned; new classes introduced in calmjs.parse such as the GroupingOp class (that was source of the issue just experienced here in slimit) may need to be handled. Another project (js2xml) has made a successful switch from slimit to calmjs.parse and the changes to the visitor appears to be rather minimal. Hopefully this level of work is also limited in scope for your project, should you decide to make this change.