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

fix(no-dupe-keys): handle nested objects & getter and setter with the same name #406

Merged
merged 8 commits into from
Oct 18, 2020

Conversation

magurotuna
Copy link
Member

@magurotuna magurotuna commented Oct 17, 2020

This is a part of #330

What I did In this PR:

  • adding more tests (mostly from ESLint)
  • allowing it to handle nested objects e.g. const obj = { a: { b: 1, b: 2 } };
  • adding proper handling for getter and setter with the same name e.g. const obj = { get a() {}, set a() {} };

@magurotuna magurotuna changed the title [WIP] fix(no-dupe-args) fix(no-dupe-args): handle nested objects & getter and setter with the same name Oct 17, 2020
@magurotuna magurotuna marked this pull request as ready for review October 17, 2020 18:34
@magurotuna
Copy link
Member Author

@bartlomieju ready for review. PTAL

@magurotuna magurotuna changed the title fix(no-dupe-args): handle nested objects & getter and setter with the same name fix(no-dupe-keys): handle nested objects & getter and setter with the same name Oct 17, 2020
Copy link
Member

@bartlomieju bartlomieju 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 for the fix @magurotuna

Comment on lines +283 to +286
// TODO(magurotuna): this leads to panic due to swc error
// It seems like tsc v4.0.2 cannot handle this either
// playground: https://www.typescriptlang.org/play?target=99&ts=4.0.2#code/MYewdgzgLgBCBGArGBeGBvAUDGBGMAXDACwBMANJgL4DcQA
// assert_lint_err::<NoDupeKeys>(r#"var x = { 1n: 1, 1: 2 };"#, 8);
Copy link
Member

Choose a reason for hiding this comment

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

@magurotuna could you report that in SWC repo?

Copy link
Member Author

Choose a reason for hiding this comment

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

@bartlomieju I thought I would do so, but as I commented, the TypeScript compiler can't parse this either. So I felt like it's okay for now. But I'll report it anyway 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

swc-project/swc#1173

Filed an issue about it

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.

2 participants