proposal-binary-ast icon indicating copy to clipboard operation
proposal-binary-ast copied to clipboard

Function.prototype.toString()

Open xorgy opened this issue 5 years ago • 12 comments

It seems unnecessary to me that Function.prototype.toString() returns a useless placeholder token (like it does with native functions). For native functions it is fair enough, and the ECMAScript standard reflects this; but the code in this case is JavaScript, even if it's in AST form, so it seems like it would be better to more closely follow the behaviour of real JavaScript. Furthermore, there is at least some code which relies on Function.prototype.toString(), whether you feel that's a prudent idea or not.

How much trouble would it be to have Function.prototype.toString() lazily emit a valid (or standard) JavaScript source representation of the binary AST instead?

xorgy avatar May 17 '19 20:05 xorgy

Yes I agree with @xorgy. There was usage of Function.prototype.toString() at least in the first version of Angular for implementation of dependency injection.

myshov avatar May 18 '19 13:05 myshov

You could always display the arguments, eg.: example toString() result:

function example(foo, bar) {
	[sourceless code]
}

That would avoid breaking the old Angular dependency injection.

ExE-Boss avatar May 19 '19 12:05 ExE-Boss

@ExE-Boss That's an interesting middle ground, though I'm not sure it's quite right either, makes it necessary to use a regex to tell that the code isn't real, though I guess not all that many people are going to be accidentally evaling some garbage they got from Function.prototype.toString().

xorgy avatar May 24 '19 18:05 xorgy

makes it necessary to use a regex to tell that the code isn't real

You already need to do that with native functions, where the result is slightly different between browsers: https://github.com/tc39/Function-prototype-toString-revision/issues/21.

Also, the ESNext native function toString() method will optionally be returning function parameters.

ExE-Boss avatar May 24 '19 21:05 ExE-Boss

It seems unnecessary to me that Function.prototype.toString() returns a useless placeholder token [...] there is at least some code which relies on Function.prototype.toString()

You are right and we want toString() in BinAST to work the same way as it does for plain JS but it's just a matter of implementation priorities. Right now we're working on a new file format for BinAST and decoder performance so we've shelved toString() for the timebeing.

vdjeric avatar May 24 '19 21:05 vdjeric

@vdjeric Fair enough, though I suspect regardless of what BinAST looks like, it is not likely to be difficult for implementers to write some sort of toString, especially the sort that @ExE-Boss is describing (since there's not much that can be done to collapse arguments, especially with object destructuring, so presumably names will be intact).

As long as a slightly more useful Function.prototype.toString is not off the table, I'm happy. :- )

xorgy avatar May 29 '19 12:05 xorgy

One of the things that make JavaScript great is the fact you can eval() most functions' string representations (plus a pair of parentheses) and get a function back. This is not recommended, but is still great for a lot of reasons:

  • You can inline a function into a data: URL (which is useful, for say, creating a Worker on-demand)
  • You can read arguments and comments as said above and perform actions based upon the results

And probably more things I can't remember right now.

IMO the VM should generate a string representation from its internal AST, on-demand, when the toString method is called.

fabiosantoscode avatar Jun 24 '19 14:06 fabiosantoscode

@fabiosantoscode:

  • You can inline a function into a data: URL

That's a bug, not a feature. Unless you want to help attackers.

rossberg avatar Jun 24 '19 15:06 rossberg

@rossberg not like a security issue, more like this:

const worker = new Worker('data:text/javascript,(' + () => {
  /* worker code goes here... */
} + ')()')

In this example, it helps you define a worker next to the place it's going to be used and not in a new file. There's probably more interesting things you can do with data-urls and toString. I can't see how this is a bug.

fabiosantoscode avatar Jun 24 '19 19:06 fabiosantoscode

Also, I can't overstate how many times I use Function.prototype.toString to read the source code of some unknown callback in the browser console or the node console.

fabiosantoscode avatar Jun 24 '19 19:06 fabiosantoscode

Workers do often need data / blob URL initialization tricks in many tooling approaches

Note that module workers are not cross origin by default (although their dependencies can be) so in many cases this data URL trick is absolutely needed in order to support loading module workers from CDN URLs..

On Mon, 24 Jun 2019 at 21:06, Fábio Santos [email protected] wrote:

@rossberg https://github.com/rossberg not like a security issue, more like this:

const worker = new Worker('data:text/javascript,(' + () => { /* worker code goes here... */ } + ')()')

In this example, it helps you define a worker next to the place it's going to be used and not in a new file. There's probably more interesting things you can do with data-urls and toString. I can't see how this is a bug.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-binary-ast/issues/76?email_source=notifications&email_token=AAESFSRCCFFHPF5WFMVG653P4ELMRA5CNFSM4HNYH642YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYN5BTY#issuecomment-505139407, or mute the thread https://github.com/notifications/unsubscribe-auth/AAESFSUINVGW5AWAROJNCNLP4ELMRANCNFSM4HNYH64Q .

guybedford avatar Jun 24 '19 19:06 guybedford

If there is need to have Function.prototype.toString() compatible with eval(), it would be best for comments to not be saved in the AST, because they are not executed.

Python has AST-relevant public APIs built into the standard library as the ast module, and it's evident that the Python AST parser doesn't save comments (Python uses # to denote them):

import ast

print(ast.unparse(ast.parse("""def a():
    # does stuff
    return 0""")))

which outputs:

def a():
    return 0

The comment isn't there because it's not necessary for the function to work. Docstrings are preserved, though.

C-Ezra-M avatar Jan 02 '23 19:01 C-Ezra-M