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

handeling of ternary based assignment changed in indent #8847

Closed
futagoza opened this issue Jun 30, 2017 · 4 comments
Closed

handeling of ternary based assignment changed in indent #8847

futagoza opened this issue Jun 30, 2017 · 4 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion question This issue asks a question about ESLint works as intended The behavior described in this issue is working correctly

Comments

@futagoza
Copy link

Tell us about your environment

  • ESLint Version: 4.1.1
  • Node Version: 8.1.3
  • npm Version: 5.0.3

What parser are you using? babel-eslint (via preset eslint-config-futagozaryuu)

Please show your full configuration:

https://github.com/futagoza/babel-preset-futagozaryuu/blob/master/src/.eslintrc.js

"use strict";

module.exports = {

    "extends": [
        "futagozaryuu/esnext",
        "futagozaryuu/node"
    ],

    "root": true

};

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

https://github.com/futagoza/babel-preset-futagozaryuu/blob/master/src/buildOptions.js#L79

// ...

export default function buildOptions( _config = {} ) {

    // ...

    if ( ! config.node ) {

        const engines = getPackage( config.cwd || defaultOptions.cwd ).engines;

        config.node = engines && engines.node
                    ? majorSemver( String( engines.node ) )
                    : defaultOptions.defaultNodeVersion;

    }

    if ( typeof config.node === "string" ) {

        config.node = config.node.includes( "." )
                    ? parseFloat( config.node )
                    : parseInt( config.node, 10 );

    }

    // ...

}

What did you expect to happen?

Before the upgrade to ESLint v4, the rule ident allowed the above.

What actually happened? Please include the actual, raw output from ESLint.

After the upgrade to ESLint v4, the rule ident throws the error:

https://travis-ci.org/futagoza/babel-preset-futagozaryuu/builds/248821808

C:\projects\babel-preset-futagozaryuu\src\buildOptions.js
  84:1  error  Expected indentation of 12 spaces but found 22  indent
  85:1  error  Expected indentation of 12 spaces but found 22  indent
  92:1  error  Expected indentation of 12 spaces but found 20  indent
  93:1  error  Expected indentation of 12 spaces but found 20  indent

✖ 4 problems (4 errors, 0 warnings)
  4 errors, 0 warnings potentially fixable with the `--fix` option.

Modifiing the eslint command at the top of the file (for this) returns the orignal behaviour, but I'd prefer not to do this:

/* eslint prefer-const: 0, indent: 0, indent-legacy: ["error", 4, { "SwitchCase": 1 }]*/

Suggestion:

  1. Restore the way indent checks ternary based assignments.

  2. Add a new boolean option for the indent rule called ignoreTernaryAssignment

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jun 30, 2017
futagoza added a commit to futagoza/eslint-config-futagozaryuu that referenced this issue Jun 30, 2017
@kaicataldo kaicataldo added question This issue asks a question about ESLint works as intended The behavior described in this issue is working correctly and removed triage An ESLint team member will look at this issue soon labels Jun 30, 2017
@kaicataldo
Copy link
Member

Thanks for the issue. The indent rule in 3.0 was extremely buggy and so was completely rewritten in v4. The new version of the rule is more strict (the trade off is that it's autofixable now!). That being said, we left the legacy indent rule in for those who prefer the old behavior. You can change indent to indent-legacy in your configuration and it will use the old rule instead.

@kaicataldo
Copy link
Member

There's also a proposal to enhance the rule to be able to ignore nodes, which I think would also solve your issue: #8594

@futagoza
Copy link
Author

@kaicataldo Yea, it actually would 😄, thanks for informing me

@kaicataldo
Copy link
Member

Closing this since #8594 exists and is being tracked!

futagoza added a commit to futagoza/babel-preset-futagozaryuu that referenced this issue Jun 30, 2017
futagoza added a commit to futagoza/eslint-config-futagozaryuu that referenced this issue Aug 24, 2017
@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
archived due to age This issue has been archived; please open a new issue for any further discussion question This issue asks a question about ESLint works as intended The behavior described in this issue is working correctly
Projects
None yet
Development

No branches or pull requests

3 participants