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

lint: enforce some spelling conventions #207

Merged
merged 4 commits into from
May 27, 2020
Merged

lint: enforce some spelling conventions #207

merged 4 commits into from
May 27, 2020

Conversation

bakkot
Copy link
Contributor

@bakkot bakkot commented May 24, 2020

This rule is tripped 29 times on ECMA-262 master: in 26 places where we say *this* object instead of (as in the other 429 uses of *this*) *this* value", in 1 place where we say behavior instead of (as in the other 132 places) behaviour, and in 2 places where we say the empty string instead of (as in the other 76 places) the empty String.

I'll fix those as part of upstreaming this.

@bakkot
Copy link
Contributor Author

bakkot commented May 24, 2020

Note that it is plausible that we actually do want to say *this* object; @rkirsling here observes that it appears to be a shorthand for "this value (which is an object)". It's only ever used to describe the behavior of built-in methods (e.g. String.prototype.concat). But I don't think it makes sense to have that distinction; in those cases nothing enforces that *this* is in fact an object.

src/lint/collect-spelling-diagnostics.ts Outdated Show resolved Hide resolved
src/lint/collect-spelling-diagnostics.ts Outdated Show resolved Hide resolved
@ljharb
Copy link
Member

ljharb commented May 24, 2020

I'd agree; the receiver only has to be an object in sloppy mode, and builtin methods have RequireObjectCoercible to account for nullish values, so this value seems to be the proper choice.

@bakkot bakkot merged commit 25809b0 into master May 27, 2020
@bakkot bakkot deleted the lint-spelling branch May 27, 2020 02:56
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 this pull request may close these issues.

4 participants