Skip to content

Bug: [no-misused-promises] inheritedMethods check should flag all statically analyzable declarations, not just identifiers and numbers. #10211

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

Open
4 tasks done
kirkwaiblinger opened this issue Oct 26, 2024 · 4 comments · May be fixed by #10310
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@kirkwaiblinger
Copy link
Member

kirkwaiblinger commented Oct 26, 2024

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Playground Link

LINK

Repro Code

const staticSymbol = Symbol.for('static symbol');

interface Interface {
  identifier: () => void;
  1: () => void;
  2: () => void;
  stringLiteral: () => void;
  computedStringLiteral: () => void;
  // well known symbol
  [Symbol.iterator]: () => void;

  // less sure if this one is possible to lint for
  [staticSymbol]: () => void;
}


class Clazz implements Interface {
  async identifier() { }
  async 1() { }
  async [2]() { }
  async 'stringLiteral'() { }
  async ['computedStringLiteral']() { }
  async [Symbol.iterator]() { };
  async [staticSymbol]() { };
}

ESLint Config

module.exports = {
  "rules": {
    "@typescript-eslint/no-misused-promises": "warn"
  }
}

tsconfig

{
  "compilerOptions": {
    "strict": true
  }
}

Expected Result

All method declarations should flag

Actual Result

Only async identifier() {} and async 1() {} flagged.

Additional Info

No response

@kirkwaiblinger kirkwaiblinger added bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Oct 26, 2024
@kirkwaiblinger
Copy link
Member Author

We should take a look at object literal assignments too, which suffer most of the same defects:
(playground)

const foo: Interface = {
  async identifier() { },
  async 1() { },
  async [2]() { },
  async 'stringLiteral'() { },
  async ['computedStringLiteral']() { },
  async [Symbol.iterator]() { },
  async [staticSymbol]() { },
}

This one additionally gets the 'stringLiteral' case correct though.

@bradzacher
Copy link
Member

async [2]() { }
async ['computedStringLiteral']() { }

In general we ignore all computed property names in rules because they can be any expression at all.

We could special case the literal cases but generally we just treat computed members as a black box that we can't truly resolve cos doing the above cases is really rare.

async [Symbol.iterator]() { };

This falls into the same boat - it's a computed member so we ignore it. We could special case the Symbol special members but it's also a difficult thing because like Symbol.notIterator is valid code too - so would we special case it or ignore it?

Then you would also wanna handle and ignore like

const Symbol = {iterator: 1};
class Foo {
  [Symbol.iterator](){}
}

(people do like to reuse the name Symbol more than you'd like - eg the TS codebase itself!)

async [staticSymbol]() { };

Same case again except now the general symbol case. You'd need to check - idk if it creates a unique type which allows us to uniquely identify the symbol. Regardless it's not possible to check this case without type info which does increase the cost of the check. Often not super worth checking this either as it's really really rare to use a unique symbol in an inheritance case like this.

@bradzacher
Copy link
Member

We should probably build some primitives around this more generally across all rules to allow us to get a property key handling the simple computed cases and similar special named cases like iterator.

@kirkwaiblinger
Copy link
Member Author

yeah, we do have getStaticMemberAccessValue, which I think should handle most if not all of these already. Not super certain if it will get us the well-known symbols, but I know it handles things like Symbol.for('foo') 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
2 participants