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

Bug: [no-unnecessary-type-assertion] false positive when casting to HTMLElement (from Element) #9007

Closed
4 tasks done
bschlenk opened this issue Apr 26, 2024 · 6 comments
Closed
4 tasks done
Labels
bug Something isn't working duplicate This issue or pull request already exists package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin working as intended Issues that are closed as they are working as intended

Comments

@bschlenk
Copy link

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

https://typescript-eslint.io/play/#ts=5.2.2&fileType=.tsx&code=FAYw9gdgzgLgBAUwDZwLxwCZhAVwLYIQwB0AjjggE4CeAysgiDGJQBQDkGAlgIZJgBzdgEo4PKHAASAFQCyAGQCiSBASLBgAek1wYErhABmVCQAMQACy5IMp3WDgAjBHDCHd1AA4vTy1YRg7AHcrSzguCQgweBgLFygeAjEzGQU-NUDQSFg4S2sMNDhWZDIKGnoVJhYOAG0eHGZDbBwoAF0ROAAfTsQkUXEpOSUVDKA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1tiacTJTIAhtEK0ipWkOTJE0fJQ5N0UOdA7RI4MAF8QOoA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

Repro Code

const el = document.querySelector('dialog') as HTMLElement

// ts infers `child` to be of type `Element` which is not the same as `HTMLElement`
const child = (el.querySelector('[autofocus]') || el) as HTMLElement

// `focus` doesn't exist on Element, which is why we cast in the first place
child.focus()

ESLint Config

module.exports = {
  "rules": {
    "@typescript-eslint/no-unnecessary-type-assertion": "error"
  }
}

tsconfig

{
  "compilerOptions": {
    "strictNullChecks": true
  }
}

Expected Result

I expect this not to trigger, since HTMLElement is not the same as the Element type ts would infer.

Actual Result

This rule triggers and automatically removes the type assertion, causing the following lines to start failing.

Additional Info

No response

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

kirkwaiblinger commented Apr 26, 2024

@bschlenk
Interesting! This appears to have to do with overload resolution complications

interface AType {
    aProperty: string;
}

interface AChildType extends AType {
    aNotherProperty: string;
}

// no bug
declare const parentType: AType
const needsToBeSubType = parentType as AChildType;
needsToBeSubType.aNotherProperty;

// bug! 
declare function getTypeOrSubType<T extends AType>(): T;
// generic argument is inferred as AChildType with the assertion, AType without. 
const needsToBeSubType = getTypeOrSubType() as AChildType;
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
needsToBeSubType.aNotherProperty;

@kirkwaiblinger kirkwaiblinger added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for maintainers to take a look labels Apr 26, 2024
@kirkwaiblinger
Copy link
Member

kirkwaiblinger commented Apr 26, 2024

There is a chance this could be pretty hard to solve, or might run into some known blockers, along the lines of #709. Curious if anyone in @typescript-eslint/triage-team knows off the top of their head

@kirkwaiblinger
Copy link
Member

kirkwaiblinger commented Apr 26, 2024

fyi @bschlenk if you want a way around this bug, I might recommend the following as a workaround/better code anyway:

// just specify the type argument explicitly... This reveals that a nonnull assertion is being hidden by the first assertion
const el = document.querySelector<HTMLElement>('dialog')!;
// no type assertion needed here anymore
const child = (el.querySelector<HTMLElement>('[autofocus]') || el);
child.focus();

(playground link)

@bradzacher
Copy link
Member

bradzacher commented Apr 26, 2024

This is working as intended.
Duplicate of #8388, #7207, #5808, #5359, #528 etc

@bradzacher bradzacher closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2024
@bradzacher bradzacher added working as intended Issues that are closed as they are working as intended duplicate This issue or pull request already exists and removed accepting prs Go ahead, send a pull request that resolves this issue labels Apr 26, 2024
@kirkwaiblinger
Copy link
Member

@bradzacher Do you think we should try to document this more prominently on the rule page? Seems to have high volume of duplicates for what's basically an unfixable bug

@bradzacher
Copy link
Member

We could do. It doesn't come up too often but no harm in documenting it.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working duplicate This issue or pull request already exists package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin working as intended Issues that are closed as they are working as intended
Projects
None yet
Development

No branches or pull requests

3 participants