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

Rule function-paren-newline incorrectly formats a method with a ordered array as a return type #15091

Closed
1 task
simonkarman opened this issue Sep 22, 2021 · 3 comments · Fixed by #15541
Closed
1 task
Assignees
Labels
3rd party plugin This is an issue related to a 3rd party plugin, config, or parser 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 rule Relates to ESLint's core rules
Projects

Comments

@simonkarman
Copy link

simonkarman commented Sep 22, 2021

Environment

Node version: v16.9.1
npm version: 7.21.1
Local ESLint version: 7.32.0
Global ESLint version: none
Operating System: Ubuntu 20.04

What parser are you using?

@typescript-eslint/parser

What did you do?

Rule function-paren-newline incorrectly formats a function with a ordered array as a return type, when that function has its parameters on new lines and the return type has a method in its type. Only happens when the function is an inline function and the return type of that function is specified on the function itself.
// works when all on the same line
const method1: (abc: number, def: () => void) => [string, number] = () => ['a', 2];
method1(3, () => {});

// works when function is part of the params
const method2: (
  abc: number,
  def: () => void
) => [string, number] = () => ['a', 2];
method2(3, () => {});

// works when function definition is defined separate from the implementation
const method3: (
  abc: number,
  def: () => void
) => [string, () => void] = () => ['a', () => {}];
method3(3, () => {});

// works when function definition is defined separate from the implementation even if array is on multiple lines
const method4: (
  abc: number,
  def: () => void
) => [
  string,
  () => void
] = () => ['a', () => {}];
method4(3, () => {});

// works when the function parameters are on the same line
const method5 = (abc: number, def: () => void): [
  string,
  () => void,
] => [`a${abc}`, def];
method5(3, () => {});

// doesn't work when the function itself is given a return type, now we need to use a 'disable next line' for the rule
const method6 = (
  abc: number,
  def: () => void,
): [
  string,
  // eslint-disable-next-line function-paren-newline
  () => void
] => [`a${abc}`, def];
method6(3, () => {});

What did you expect to happen?

I expect the following to work even without the disable next line.

const method6 = (
  abc: number,
  def: () => void,
): [
  string,
  // eslint-disable-next-line function-paren-newline
  () => void
] => [`a${abc}`, def];
method6(3, () => {});

Unfortunately it transforms it to:

const method6 = (
  abc: number,
  def: () => void,
): [
  string,
  (
) => void
] => [`a${abc}`, def];
method6(3, () => {});

What actually happened?

Expected newline before ')'.eslintfunction-paren-newline

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

No response

@simonkarman simonkarman added bug ESLint is working incorrectly repro:needed labels Sep 22, 2021
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Sep 22, 2021
@mdjermanovic mdjermanovic moved this from Needs Triage to Evaluating in Triage Sep 22, 2021
@mdjermanovic
Copy link
Member

Hi @simonkarman, thanks for the issue!

I think we can improve the function-paren-newline rule to better support Typescript code in this particular case.

Can you please provide one example where this rule doesn't work as expected with Typescript, so we could test the rule on that example? In particular:

  1. Your configuration for function-paren-newline.
  2. Code where this rule reports a parenthesis that shouldn't be reported (please don't include eslint-disable comments in the example).

@github-actions
Copy link

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

@github-actions github-actions bot added the Stale label Nov 21, 2021
@mdjermanovic mdjermanovic added 3rd party plugin This is an issue related to a 3rd party plugin, config, or parser accepted There is consensus among the team that this change meets the criteria for inclusion rule Relates to ESLint's core rules and removed repro:needed Stale labels Nov 22, 2021
@mdjermanovic mdjermanovic self-assigned this Nov 22, 2021
@mdjermanovic mdjermanovic moved this from Evaluating to Ready to Implement in Triage Nov 22, 2021
@mdjermanovic
Copy link
Member

I can reproduce this with ESLint v8.3.0, @typescript-eslint/parser v5.4.0, and the following code:

/* eslint function-paren-newline: ["error", "multiline"] */

const method6 = (
  abc: number,
  def: () => void,
): [
  string,
  () => void
] => [`a${abc}`, def];
method6(3, () => {});

I'll work on this.

@mdjermanovic mdjermanovic moved this from Ready to Implement to Pull Request Opened in Triage Jan 25, 2022
Triage automation moved this from Pull Request Opened to Complete Feb 8, 2022
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Aug 8, 2022
@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 Aug 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3rd party plugin This is an issue related to a 3rd party plugin, config, or parser 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 rule Relates to ESLint's core rules
Projects
Archived in project
Triage
Complete
Development

Successfully merging a pull request may close this issue.

2 participants