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

[indent] Ternary doesn't allow extended indentation #7698

Closed
xixixao opened this issue Dec 5, 2016 · 6 comments
Closed

[indent] Ternary doesn't allow extended indentation #7698

xixixao opened this issue Dec 5, 2016 · 6 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion indent Relates to the `indent` rule rule Relates to ESLint's core rules

Comments

@xixixao
Copy link

xixixao commented Dec 5, 2016

Tell us about your environment

  • ESLint Version:
    whatever is on the DEMO on eslint.org, unfortunately it doesn't show
  • Node Version:
    n/a
  • npm Version:
    m/a
  • What parser (default, Babel-ESLint, etc.) are you using?
    dunno, environments is set to node, es6

Please show your full configuration:
indent is on

What did you do? Please include the actual source code causing the issue.

const foo = Math.random()
  ? {
      a: 3,
    }
  : {};

What did you expect to happen?
No warning.
What actually happened? Please include the actual, raw output from ESLint.
4:5 - Expected indentation of 2 spaces but found 4. (indent)

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Dec 5, 2016
@xixixao
Copy link
Author

xixixao commented Dec 5, 2016

@not-an-aardvark does your #7618 fix this too?

@not-an-aardvark
Copy link
Member

not-an-aardvark commented Dec 5, 2016

No, I think this is (mostly) working as intended. The correct output would be:

3:7 - Expected indentation of 4 spaces but found 6. (indent)
4:5 - Expected indentation of 2 spaces but found 4. (indent)

For what it's worth, I think this is a duplicate of #6606.

@not-an-aardvark
Copy link
Member

To elaborate a bit more on why I think reporting an error here is correct behavior:

const foo = Math.random()
  ? {
      a: 3,
    }
  : {};

The question mark is indented at two spaces, but the a is indented at 6 spaces. Presumably the location of the a is based on the column of the opening {, but the opening { isn't actually at the beginning of the line. So the ? and the space after it are actually contributing to the indentation here, which isn't something that happens anywhere else in the indent rule.

To make this easier to see, suppose you decided to change your indent size to 4 spaces instead of 2:

const foo = Math.random()
    ? {
        a: 3,
      }
    : {};

In this case the closing } is at an offset of 6 spaces. I think it would be very unexpected for the indent rule to enforce a 6-space offset when your config was trying to enforce indentation in 4-space levels.

@kaicataldo kaicataldo added bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Dec 6, 2016
@xixixao
Copy link
Author

xixixao commented Dec 10, 2016

I agree this is duplicate of #6606. We use this style pretty consistently, including with JSX and other expressions, where lint doesn't error:

// works
const foo = Math.random()
  ? <div
      id="test",
    />
  : null;
// works
const foo = Math.random()
  ? parseInt(
      "42",
      16,
    )
  : 0;

I see that this doesn't work as well when the indentation isn't two spaces and don't really have an answer for that, as I personally don't have experience with JS using different config. Perhaps if this was a configuration flag, it would only be used by people who have indent set to 2.

@Janpot
Copy link

Janpot commented Jun 21, 2017

Could you consider the following for 4 space indentation?

const foo = Math.random()
    ?   {
            a: 3,
        }
    :   {};

Or is that too far off?

@not-an-aardvark
Copy link
Member

Closing because this is working as intended.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 12, 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 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion indent Relates to the `indent` rule rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

6 participants