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

feat: add eslint/no-obj-calls #508

Merged
merged 7 commits into from
Jul 3, 2023

Conversation

DonIsaac
Copy link
Contributor

@DonIsaac DonIsaac commented Jul 2, 2023

Adds support for no-obj-calls. Provides progress towards completing #479.

Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! Only a small change is required.

crates/oxc_linter/src/rules/eslint/no_obj_calls.rs Outdated Show resolved Hide resolved
crates/oxc_linter/src/rules/eslint/no_obj_calls.rs Outdated Show resolved Hide resolved
crates/oxc_linter/src/rules/eslint/no_obj_calls.rs Outdated Show resolved Hide resolved
@DonIsaac
Copy link
Contributor Author

DonIsaac commented Jul 3, 2023

@Boshen thanks for the review, I'll address your comments. I just updated my PR to support references to global objects, e.g.

// Bad
let a = JSON;
let b = a;
let c = b;
c();
let m = Math
function foo() {
  new m();
}

// Good
let m = Math
function foo() {
  let m = x => x;
  m();
}

I'd appreciate a second look if you can.

@Boshen Boshen merged commit 8fdb6b6 into oxc-project:main Jul 3, 2023
@Boshen
Copy link
Member

Boshen commented Jul 3, 2023

I don't want #510 blocking this PR so I merged it.

Thank you so much for the contribution!

@Maneren
Copy link
Contributor

Maneren commented Jul 3, 2023

I know this is already merged and closed but I don't want to open a new issue when it's so recent.

When running the current main branch against the eslint codebase I get the following panic!:

     Running `/home/maneren/.cache/cargo/target/debug/oxc_cli lint /home/maneren/git-repos/eslint/lib/`
thread '<unnamed>' panicked at 'No binding id found, but this IdentifierReference is not a global', crates/oxc_linter/src/rules/eslint/no_obj_calls.rs:94:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Rayon: detected unexpected panic; aborting
[1]    17749 IOT instruction (core dumped)  cargo r --bin oxc_cli -- lint /home/maneren/git-repos/eslint/lib/

That doesn't seem like it's supposed to happen.

@Boshen Boshen mentioned this pull request Jul 4, 2023
@Boshen
Copy link
Member

Boshen commented Jul 4, 2023

@Maneren Thank you for testing! I'll fix this in #510. And if you interested: #511 😁

@DonIsaac
Copy link
Contributor Author

DonIsaac commented Jul 4, 2023

@Boshen would you like me to look into this separately or will you cover this in one of your following PRs?

@Boshen
Copy link
Member

Boshen commented Jul 4, 2023

@Boshen would you like me to look into this separately or will you cover this in one of your following PRs?

A quick removal of that panic would be nice.

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.

3 participants