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

no-sequences doesn't work correctly with assignment in arrow function expression #14572

Closed
jramstedt opened this issue May 6, 2021 · 6 comments · Fixed by #14578
Closed

no-sequences doesn't work correctly with assignment in arrow function expression #14572

jramstedt opened this issue May 6, 2021 · 6 comments · Fixed by #14578
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 documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before repro:yes Issues with a reproducible example

Comments

@jramstedt
Copy link

Node version: v15.5.0
npm version: v7.3.0
Local ESLint version: v7.25.0
Global ESLint version: Not found
Operating System: win32 10.0.19042

@typescript-eslint/parser

Please show your full configuration:

Configuration
module.exports = {
  env: {
    browser: true,
    es2021: true
  },
  parser: '@typescript-eslint/parser',
  parserOptions: {
    ecmaVersion: 12,
    sourceType: 'module'
  },
  extends: [
    'standard',
    'eslint:recommended',
    'plugin:@typescript-eslint/recommended'
  ],
  plugins: [
    '@typescript-eslint'
  ],
  rules: {
    indent: ['error', 2],
    'no-useless-return': 'off',
    curly: ['error', 'multi-or-nest', 'consistent'],
    'comma-dangle': ['error', 'only-multiline'],
    'no-labels': ['error', { allowLoop: true }],
    'prefer-const': 'error',
    'dot-notation': 'warn',
    '@typescript-eslint/no-use-before-define': ['error', { functions: false }],
    '@typescript-eslint/no-unused-vars': ['warn', { argsIgnorePattern: '^_' }],
    '@typescript-eslint/no-require-imports': 'error',
  }
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

let aa = 0
const test = () => ((aa = 123), 10)

console.log(aa, test())
npx eslint test.ts

What did you expect to happen?
No errors.

What actually happened? Please copy-paste the actual, raw output from ESLint.

  2:31  error  Unexpected use of comma operator  no-sequences

✖ 1 problem (1 error, 0 warnings)

Steps to reproduce this issue:

Test the code above.

This code works without errors:

let aa = 0
const test = () => { return ((aa = 123), 10) }

Are you willing to submit a pull request to fix this bug?

@jramstedt jramstedt added bug ESLint is working incorrectly repro:needed This issue should include a reproducible example labels May 6, 2021
@jramstedt jramstedt changed the title no-sequences doesn't work correctly when combined with no-return-assign no-sequences doesn't work correctly when combined with arrow function expressions May 6, 2021
@jramstedt jramstedt changed the title no-sequences doesn't work correctly when combined with arrow function expressions no-sequences doesn't work correctly when combined with assignment in arrow function expression May 6, 2021
@jramstedt jramstedt changed the title no-sequences doesn't work correctly when combined with assignment in arrow function expression no-sequences doesn't work correctly with assignment in arrow function expression May 6, 2021
@nzakas nzakas added repro:yes Issues with a reproducible example and removed repro:needed This issue should include a reproducible example labels May 7, 2021
@nzakas
Copy link
Member

nzakas commented May 7, 2021

Confirmed, This is a bug. Both examples should have the same result, which is no warnings. The intent is for parentheses to indicate the sequence is intentional (reference).

@nzakas nzakas added the good first issue Good for people who haven't worked on ESLint before label May 7, 2021
@nzakas
Copy link
Member

nzakas commented May 7, 2021

@jramstedt do you want to submit a pull request to fix this?

@jramstedt
Copy link
Author

No.
I don't currently have time to figure this out.

@snitin315
Copy link
Contributor

I can take it if there are no objections.

@mdjermanovic
Copy link
Member

This is intended behavior, per #6082.

The rule requires double parentheses around arrow function body:

/* eslint no-sequences: error */

const test1 = () => ((aa = 123), 10); // error

const test2 = () => (((aa = 123), 10)); // ok

Online Demo

@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion documentation Relates to ESLint's documentation and removed bug ESLint is working incorrectly labels May 8, 2021
@nzakas
Copy link
Member

nzakas commented May 8, 2021

Wow nice catch. This isn’t obvious from the docs for this rule, so I’d like to leave this open to improve the documentation. This special case should be called out explicitly.

@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Nov 25, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Nov 25, 2021
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 documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before repro:yes Issues with a reproducible example
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants