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 crash in jquery-ember-run rule #1431

Merged
merged 1 commit into from Feb 14, 2022
Merged

Fix crash in jquery-ember-run rule #1431

merged 1 commit into from Feb 14, 2022

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Feb 13, 2022

The included test demonstrates a case that would trigger an assertion within eslint.

It's not correct to try to report(expression.callee) without checking first that the expression is a CallExpression. Lots of other kinds of nodes can appear here instead, and they will cause us to pass undefined to report(), which generates the assertion within eslint.

The refactoring into two helper functions here was forced by the complexity eslint rule this repo uses.

The included test demonstrates a case that would trigger an assertion
within eslint. It's not correct to try to `report(expression.callee)`
without asserting that the expression is a CallExpression. Lots of other
kinds of nodes can appear here instead, and the AssignmentExpression in
my example is only one of the possibilities.

The refactoring into two helper functions here was forced by the
complexity eslint rule this repo uses.
@bmish bmish added the Bug label Feb 13, 2022
Copy link
Member

@bmish bmish left a comment

Choose a reason for hiding this comment

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

Thanks!

@bmish bmish changed the title Fix an eslint assertion within jquery-ember-run Fix crash in jquery-ember-run rule Feb 14, 2022
@bmish bmish merged commit af0026a into master Feb 14, 2022
@bmish bmish deleted the jquery-ember-run-crash branch February 14, 2022 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants