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

param-names fixer breaks code functionality #145

Closed
adrianmcli opened this issue Sep 7, 2018 · 4 comments · Fixed by #146
Closed

param-names fixer breaks code functionality #145

adrianmcli opened this issue Sep 7, 2018 · 4 comments · Fixed by #146

Comments

@adrianmcli
Copy link
Contributor

Description

The fixer for the param-names rule breaks code functionality.

The fixer renames arguments in a Promise constructor function, but does not propagate the name change to the calling code.

Steps to Reproduce

Run the ESLint fixer on the following code:

new Promise((yes, no) => { yes("hello") })'

Expected behavior:

new Promise((resolve, reject) => { resolve("hello") })'

Actual behavior:

new Promise((resolve, reject) => { yes("hello") })'

Additional Information

Rather than trying to change people's code, I think it's better to remove the fixer rather than change variable names everywhere.

@adrianmcli adrianmcli changed the title Fixer for the param-names rule does not change calls inside the construction function Fixer for the param-names rule breaks code functionality Sep 7, 2018
@adrianmcli adrianmcli changed the title Fixer for the param-names rule breaks code functionality param-names fixer breaks code functionality Sep 7, 2018
@adrianmcli
Copy link
Contributor Author

Can submit a PR for this if we can agree that it's better to remove the fixer.

@macklinu
Copy link
Contributor

macklinu commented Sep 7, 2018

Removing the fixer sounds like a good idea. Thanks for pointing this out - PR welcome for that!

@adrianmcli
Copy link
Contributor Author

Sounds good! Working on it now.

@adrianmcli
Copy link
Contributor Author

@macklinu PR submitted: #146

macklinu pushed a commit that referenced this issue Sep 7, 2018
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 a pull request may close this issue.

2 participants