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.

Sign up for GitHub

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

Enforce keyword order between override and static/async #43660

Merged
merged 2 commits into from
Apr 15, 2021
Merged

Enforce keyword order between override and static/async #43660

merged 2 commits into from
Apr 15, 2021

Conversation

andrewbranch
Copy link
Member

The order is static override async.

Fixes #43606

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Apr 13, 2021
@andrewbranch
Copy link
Member Author

Some tests were already using override static. I have literally no opinion on this, aside from that we should enforce an order, since pretty much everything else has an order enforced.

@Kingwl
Copy link
Contributor

Kingwl commented Apr 15, 2021

The current check of modifier order is painful. Are there any possible to map modifiers into some priority. And we just check the order of priority value.

@andrewbranch
Copy link
Member Author

I agree @Kingwl, looking at that function, it’s begging to be refactored into something more declarative. Personally I would love to see something like that implemented.

@andrewbranch andrewbranch merged commit 06f25c0 into microsoft:master Apr 15, 2021
@andrewbranch andrewbranch deleted the bug/43606 branch April 15, 2021 21:03
@weswigham weswigham mentioned this pull request Apr 15, 2021
Merged
@DanielRosenwasser DanielRosenwasser mentioned this pull request Apr 20, 2021
Closed
@DanielRosenwasser
Copy link
Member

@dsherret

@DanielRosenwasser
Copy link
Member

@nicolo-ribaudo

@DanielRosenwasser DanielRosenwasser mentioned this pull request Apr 20, 2021
Merged
@nicolo-ribaudo
Copy link

Thanks for the heads up @DanielRosenwasser!

@nicolo-ribaudo
Copy link

Should there also be an order for abstract/override?

https://www.typescriptlang.org/play?ts=4.3.0-dev.20210426#code/IYIwzgLgTsDGEAJYBthjAgQmgpgYVXQQG8AoBBUSGeBAWwEYAKASgC4FqBLAOwHMA3OUrhocRHQBMrDt35CAvqVJUxtFGgwBZAJ4FNAVgQ4AHhBw8AJhmxh8hDGQoB7AG44oULpZwjq4+mZ2TmheQWFVGkQ3Dy8femlguXClIA

@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Apr 26, 2021
Merged
@andrewbranch
Copy link
Member Author

Yes 🤦

@andrewbranch andrewbranch mentioned this pull request Apr 26, 2021
Merged
@Kingwl
Copy link
Contributor

Kingwl commented Apr 27, 2021

🤦

@g-plane g-plane mentioned this pull request May 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielRosenwasser DanielRosenwasser DanielRosenwasser approved these changes

Assignees

@andrewbranch andrewbranch

Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Require either async override or override async (don't allow both)
5 participants
@andrewbranch @Kingwl @DanielRosenwasser @nicolo-ribaudo @typescript-bot