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: incorrect indent check for array property access (fixes #7484) #7485

Merged
merged 1 commit into from
Oct 31, 2016

Conversation

not-an-aardvark
Copy link
Member

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

See #7484

What changes did you make? (Give an overview)

This fixes an issue where array indentation was calculated incorrectly for MemberExpression nodes.

It does this by removing some outdated logic from the checkIndentInArrayOrObjectBlock function. As far as I can tell, this logic is no longer necessary because indent no longer has a special case for nested arrays.

Is there anything you'd like reviewers to focus on?

We should verify that this removed logic wasn't doing something else that I'm unaware of. It passes all the tests and doesn't appear to be necessary anymore, but it would be good to have confirmation of its original purpose. (It looks like it was introduced in b11c3c4) /cc @gyandeeps @BYK

@eslintbot
Copy link

LGTM

@mention-bot
Copy link

@not-an-aardvark, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vitorbal, @mysticatea and @gyandeeps to be potential reviewers.

@not-an-aardvark not-an-aardvark added bug ESLint is working incorrectly rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion regression Something broke patch candidate This issue may necessitate a patch release in the next few days labels Oct 30, 2016
@kaicataldo
Copy link
Member

LGTM. Thanks!

@kaicataldo kaicataldo merged commit 2012258 into master Oct 31, 2016
@not-an-aardvark not-an-aardvark deleted the indent-array-7484 branch October 31, 2016 18:40
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly patch candidate This issue may necessitate a patch release in the next few days regression Something broke rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants