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

Jump to bottom

Throw a syntax error for a constructor with type parameters #12065

Merged
merged 3 commits into from Sep 17, 2020
Merged

Throw a syntax error for a constructor with type parameters #12065

merged 3 commits into from Sep 17, 2020

Conversation

sosukesuzuki
Copy link
Member

@sosukesuzuki sosukesuzuki commented Sep 16, 2020 •
edited by gitpod-io bot

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

sosukesuzuki
sosukesuzuki commented Sep 16, 2020
View reviewed changes
scripts/parser-tests/typescript/allowlist.txt Outdated
@@ -420,3 +420,4 @@ withStatement.ts
withStatementErrors.ts
withStatementInternalComments.ts
withStatementNestedScope.ts
parserConstructorDeclaration12.ts
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/microsoft/TypeScript/blob/master/tests/cases/compiler/parserConstructorDeclaration12.ts

class C {
  constructor<>() { }
  constructor<> () { }
  constructor <>() { }
  constructor <> () { }
  constructor< >() { }
  constructor< > () { }
  constructor < >() { }
  constructor < > () { }
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update this line:

babel/Makefile

Line 3 in bbe0cf0

TYPESCRIPT_COMMIT = ffa35d3272647fe48ddf173e1f0928f772c18630

to update the TS tests?
And then run make test-typescript-update-allowlist

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed at 7a02112! But I'm not familiar with how babel-parser tests work, so I'm not sure if it's correct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's correct! Btw, allowlist contain tests that we don't pass and not tests that we are correctly handling.

@babel-bot
Copy link
Collaborator

babel-bot commented Sep 16, 2020 •
edited

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 16, 2020 •
edited

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 7a02112:

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

@kaicataldo kaicataldo added area: typescript PR: Bug Fix 🐛 A type of pull request used for our changelog categories labels Sep 16, 2020
kaicataldo
kaicataldo approved these changes Sep 16, 2020
View reviewed changes
Copy link
Member

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

nicolo-ribaudo
nicolo-ribaudo approved these changes Sep 17, 2020
View reviewed changes
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@sosukesuzuki
Update typescript parser tests
7a02112 
Update allowlist.txt
@JLHwung
Copy link
Contributor

JLHwung commented Sep 17, 2020

The CI error is not related. I will address it in a separate PR.

@JLHwung JLHwung merged commit 7028a14 into babel:main Sep 17, 2020
@sosukesuzuki sosukesuzuki deleted the fix-12064 branch September 17, 2020 17:22
@fisker fisker mentioned this pull request Oct 15, 2020
Merged
3 tasks
sosukesuzuki added a commit to sosukesuzuki/prettier that referenced this pull request Oct 15, 2020
@sosukesuzuki
Add test for babel/babel#12065
58cbf8b
sosukesuzuki added a commit to prettier/prettier that referenced this pull request Oct 20, 2020
@sosukesuzuki
Update @babel/parser to 7.12 (#9408)
3b7a7ba 
* Install babel/parser 7.12

* Add tests for babel/babel#12161

* Add test for babel/babel#12076

* Add test for babel/babel#12085

* Add test for babel/babel#12108

* Add test for babel/babel#12120

* Add test for babel/babel#12054

* Add test for babel/babel#12061

* Add test babel/babel#12093

* Add test for babel/babel#12065

* Add test for babel/babel#12111

* Add test for babel/babel#12072

* Switch syntax-module-attributes to syntax-import-assertion

* Support "String import/export specifier"

* Remove tests for module-attributes

* Add changelog

* Update to 7.12.3

* Fix by linter

* Fix by spellchecker

* Add tests for module attributes to errors

* Add error test for module string name with import

* Remove TSTypeCastExpression

* Add tests for funny import-assertions

* Update snapshots|

* Add more tests
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Dec 18, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

@existentialism existentialism existentialism approved these changes

@kaicataldo kaicataldo kaicataldo approved these changes

Assignees
No one assigned
Labels
area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No error thrown for constructors with type parameters
6 participants
@sosukesuzuki @babel-bot @JLHwung @existentialism @nicolo-ribaudo @kaicataldo