Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting/setting built-in methods should be rewritten or disallowed #57

Open
domenic opened this issue Aug 26, 2019 · 0 comments
Open

Getting/setting built-in methods should be rewritten or disallowed #57

domenic opened this issue Aug 26, 2019 · 0 comments
Milestone

Comments

@domenic
Copy link
Collaborator

domenic commented Aug 26, 2019

Seen in #56.

const a = new Array();
const globalFunc = a.concat;

currently gets rewritten to

import Array, { concat_get as Array_concat_get } from "std:global/Array";
const a = new Array();
const globalFunc = Reflect_apply(Array_concat_get, a);

but should be rewritten to

import Array, { concat as Array_concat } from "std:global/Array";
const a = new Array();
const globalFunc = Array_concat;

The relevant criteria here is that the property concat is a function, and thus a method, not a getter/setter. I think we should basically special-case things that look like get/sets but whose type is Function.

(Technically it's possible to have a getter/setter that returns/accepts a function value, but I don't know of any instances of that across all the specs, and I doubt TypeScript has the type information to distinguish such cases. So, we'd probably need to handle them with a hard-coded list anyway, if such cases do exist.)


Similar, but different: consider

const a = [];
a.concat = 5;

This should not be rewritten at all. This code (which is bad code) creates a data property on a; it does not try to set or override Array.prototype.concat. We should let it do that, I guess. Or, we could just disallow it; whichever is easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants