Skip to content

[array-callback-return] False positive when in a map that uses a switch case without default because all possibilities are handled but ESLint isn't aware of that #2841

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

Closed
3 tasks done
MaximeCheramy opened this issue Dec 2, 2020 · 4 comments
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new base rule extension New base rule extension required to handle a TS specific case locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@MaximeCheramy
Copy link

MaximeCheramy commented Dec 2, 2020

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

Repro

playground link

The rule array-callback-return is enabled with default settings. There are no TypeScript equivalent (that's what would fix the issue).

    const l = [{ type: 'a' }, { type: 'b' }] as {type: 'a' | 'b'}[]
    l.map(el => {
      switch (el.type) {
        case 'a': return 'type a'
        case 'b': return 'type b'
      }
    })
{
  "compilerOptions": {
    "declaration": true,
    "declarationMap": true,
    "lib": [
      "dom",
      "es2018",
      "es2018.regexp"
    ],
    "module": "es6",
    "moduleResolution": "node",
    "outDir": "dist",
    "sourceMap": true,
    "target": "es5",
    "types": [
      "jest",
      "node"
    ]
  },
  "include": [
    "src/**/*",
    "tests/**/*"
  ]
}

Versions

package version
@typescript-eslint/eslint-plugin 4.9.0
@typescript-eslint/parser 4.9.0
TypeScript 4.1.2
ESLint 7.14.0
node 12.19.0
@MaximeCheramy MaximeCheramy added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Dec 2, 2020
@bradzacher bradzacher added enhancement: new base rule extension New base rule extension required to handle a TS specific case and removed triage Waiting for team members to take a look labels Dec 2, 2020
@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
@bradzacher bradzacher changed the title [array-callback-return] False positive when in a map that uses a switch case without default because all possibilities are handled but ESLint isn't aware of that Base rule extension: [array-callback-return] False positive when in a map that uses a switch case without default because all possibilities are handled but ESLint isn't aware of that May 3, 2022
@bradzacher bradzacher changed the title Base rule extension: [array-callback-return] False positive when in a map that uses a switch case without default because all possibilities are handled but ESLint isn't aware of that [array-callback-return] False positive when in a map that uses a switch case without default because all possibilities are handled but ESLint isn't aware of that May 3, 2022
@nicolassanmar
Copy link

nicolassanmar commented Jul 20, 2023

As a workaround while we don't get this implemented as a TS rule, we can do the following to ensure we both always return a value and also did not miss any case in the switch statement , without having to disable array-callback-return or no-unreachable:

const exhaustivenessSafetyCheck = (never: never) => {
  throw new Error(`Unknown value ${never}`)
}

// Example usage

const type: 'A' | 'B'

...

 switch (type) {
  case 'A':
    return ...
  case 'B':
    return ...
  default:
    return exhaustivenessSafetyCheck(type)
  }

This only accepts never as a type (triggering type errors if you pass anything else) and throws an error when called. If we forget to check one case in the switch, TS will complain that the argument passed to this function is not of type never

@kirkwaiblinger
Copy link
Member

I think we could close this as being better handled by TS's noImplicitReturns option. (playground link with this enabled)

AFAICT the only case it doesn't cover is when there is no return statement at all (as opposed to some branches that don't have a return statement). But in TS this isn't a big deal, since the returned type will have void, which you can't do anything with.

@typescript-eslint/triage-team ?

@kirkwaiblinger
Copy link
Member

kirkwaiblinger commented Oct 16, 2024

To elaborate, noImplicitReturns essentially solves .map(), .reduce(), and friends.

TS itself solves .sort() and friends (by requiring a number to be returned)

And #10106 will solve .some()/.every() and .find() and friends.

So I don't see much remaining area of applicability for an extension rule, that's not already addressed somewhere else.

@JoshuaKGoldberg
Copy link
Member

Agreed that this is yet another case of a core ESLint rule that is made unnecessary by TypeScript. Hooray, noImplicitReturns!

@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Oct 17, 2024
@github-actions github-actions bot added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Oct 25, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new base rule extension New base rule extension required to handle a TS specific case locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

5 participants