[ts]Fix syntax error for modifier name class methods with type parameters #12356

sosukesuzuki · 2020-11-13T10:05:18Z

Q	A
Fixed Issues?	Fixes #12353
Patch: Bug Fix?	Y
Major: Breaking Change?	N
Minor: New Feature?	N
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

babel-bot · 2020-11-13T10:07:52Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/32858/

codesandbox-ci · 2020-11-13T10:13:02Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 92f8321:

Sandbox	Source
babel-repl-custom-plugin	Configuration
babel-plugin-multi-config	Configuration

JLHwung · 2020-11-16T15:32:56Z

packages/babel-parser/src/plugins/typescript/index.js

@@ -166,7 +166,8 @@ export default (superClass: Class<Parser>): Class<Parser> =>
        !this.match(tt.colon) &&
        !this.match(tt.eq) &&
        !this.match(tt.question) &&
-        !this.match(tt.bang)
+        !this.match(tt.bang) &&
+        !this.match(tt.relational)


I think we could align to TS implementation here:

https://github.com/microsoft/TypeScript/blob/4885dec80d184ef1570ac97739679ca66af7181d/src/compiler/parser.ts#L1796

An allowlist instead of denylist is better to reason about and avoid any surprise cases that we did not rule out.

JLHwung · 2020-11-16T16:23:38Z

...bel-parser/test/fixtures/typescript/class/method-modifier-name-with-type-parameters/input.ts

+class C {
+  declare<T>() {}
+  readonly<T>() {}
+  abstract<T>() {}


Can you also add test cases for private, public and protected?

nicolo-ribaudo · 2020-11-18T13:17:08Z

packages/babel-parser/test/fixtures/typescript/class/modifier-name-parameters/input.ts

@@ -0,0 +1,4 @@
+class Foo {
+  constructor(private, public, static) {


This should be an error, because private, public and static are reserved words in strict mode.

babel-parser doesn't check reserved word for TypeScript?

babel/packages/babel-parser/src/plugins/typescript/index.js

Lines 1962 to 1972 in b564368

checkReservedWord(

word: string, // eslint-disable-line no-unused-vars

startLoc: number, // eslint-disable-line no-unused-vars

checkKeywords: boolean, // eslint-disable-line no-unused-vars

// eslint-disable-next-line no-unused-vars

isBinding: boolean,

): void {

// Don't bother checking for TypeScript code.

// Strict mode words may be allowed as in `declare namespace N { const static: number; }`.

// And we have a type checker anyway, so don't bother having the parser do it.

}

Oh right, I didn't remember about that.

nicolo-ribaudo · 2020-11-18T14:28:17Z

Thanks!

nicolo-ribaudo · 2020-11-18T14:29:29Z

(I reworded the commit title a bit otherwise it was too long when adding (#12356))

sosukesuzuki added area: typescript pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories labels Nov 13, 2020

JLHwung reviewed Nov 16, 2020

View reviewed changes

sosukesuzuki added 2 commits November 18, 2020 13:10

Fix syntax error for modifier name class methods with type parameters

ffa7f50

Address reviewed points

f4fba0b

sosukesuzuki force-pushed the fix-12353 branch from 0574a07 to f4fba0b Compare November 18, 2020 06:53

sosukesuzuki added 2 commits November 18, 2020 19:04

Add test and update allowlist for modifer name parameters

7fdfb6b

Modify to avoid syntax error related to ASI

92f8321

nicolo-ribaudo reviewed Nov 18, 2020

View reviewed changes

JLHwung approved these changes Nov 18, 2020

View reviewed changes

nicolo-ribaudo approved these changes Nov 18, 2020

View reviewed changes

nicolo-ribaudo merged commit 94d1160 into babel:main Nov 18, 2020

sosukesuzuki deleted the fix-12353 branch November 18, 2020 14:29

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Feb 18, 2021

github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ts]Fix syntax error for modifier name class methods with type parameters #12356

[ts]Fix syntax error for modifier name class methods with type parameters #12356

sosukesuzuki commented Nov 13, 2020 •

edited by gitpod-io bot

babel-bot commented Nov 13, 2020 •

edited

codesandbox-ci bot commented Nov 13, 2020 •

edited

JLHwung Nov 16, 2020

JLHwung Nov 16, 2020 •

edited

nicolo-ribaudo Nov 18, 2020

sosukesuzuki Nov 18, 2020

nicolo-ribaudo Nov 18, 2020

nicolo-ribaudo commented Nov 18, 2020

nicolo-ribaudo commented Nov 18, 2020 •

edited

		@@ -0,0 +1,4 @@
		class Foo {
		constructor(private, public, static) {

	checkReservedWord(
	word: string, // eslint-disable-line no-unused-vars
	startLoc: number, // eslint-disable-line no-unused-vars
	checkKeywords: boolean, // eslint-disable-line no-unused-vars
	// eslint-disable-next-line no-unused-vars
	isBinding: boolean,
	): void {
	// Don't bother checking for TypeScript code.
	// Strict mode words may be allowed as in `declare namespace N { const static: number; }`.
	// And we have a type checker anyway, so don't bother having the parser do it.
	}

[ts]Fix syntax error for modifier name class methods with type parameters #12356

[ts]Fix syntax error for modifier name class methods with type parameters #12356

Conversation

sosukesuzuki commented Nov 13, 2020 • edited by gitpod-io bot

babel-bot commented Nov 13, 2020 • edited

codesandbox-ci bot commented Nov 13, 2020 • edited

JLHwung Nov 16, 2020

Choose a reason for hiding this comment

JLHwung Nov 16, 2020 • edited

Choose a reason for hiding this comment

nicolo-ribaudo Nov 18, 2020

Choose a reason for hiding this comment

sosukesuzuki Nov 18, 2020

Choose a reason for hiding this comment

nicolo-ribaudo Nov 18, 2020

Choose a reason for hiding this comment

nicolo-ribaudo commented Nov 18, 2020

nicolo-ribaudo commented Nov 18, 2020 • edited

sosukesuzuki commented Nov 13, 2020 •

edited by gitpod-io bot

babel-bot commented Nov 13, 2020 •

edited

codesandbox-ci bot commented Nov 13, 2020 •

edited

JLHwung Nov 16, 2020 •

edited

nicolo-ribaudo commented Nov 18, 2020 •

edited