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: [@typescript-eslint/no-unnecessary-type-assertion] False positives for "as const" assertions #8721

Open
4 tasks done
ashvinsrivatsa opened this issue Mar 19, 2024 · 12 comments
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look

Comments

@ashvinsrivatsa
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.4.2&fileType=.tsx&code=KYDwDg9gTgLgBAYwgOwM7wLYEMoGthRwC8cARNngaXFqoiugNwCwAUKJLHAJbIwEAzLAmBwAIlhhY4AbzZw4FfFABccGAE8wwCAMU5lLVgoBuWADYBXYGvRReAcyMBfNgMvIEMbikRRgksASUgAUAJRqwdJyxvRo8ABGtKIkMvqUqunKcM5GCv4wllDIsnAAdBVJqMAANHBmVjZk1LlszkA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1tiacTJTIAhtEK0ipWkOTJE0fJQ5N0UOdA7RI4MAF8QOoA&tsconfig=N4KAvkA&tokens=false

Repro Code

export const marker = "marker" as const;
export interface Data {
  marker: typeof marker;
  value: string;
}
function createData(): Data {
  const base = { marker: marker };
  return { ...base, value: "" };
}

ESLint Config

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

tsconfig

{
}

Expected Result

The first line of this code should not report an error for the as const assertion, since this assertion is necessary - removing it is a TypeScript compilation error (see TypeScript playground).

Actual Result

Starting in typescript-eslint@7.3 (presumably #8558), the first line of this code reports a @typescript-eslint/no-unnecessary-type-assertion error.

Additional Info

No response

@ashvinsrivatsa ashvinsrivatsa 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 Mar 19, 2024
@bradzacher
Copy link
Member

bradzacher commented Mar 19, 2024

This seems like a bug in TS.
Without the as const the type of marker is "marker". With it the type is the same.

This is evidenced by the fact that TS types your interface as requiring the property marker: "marker", not marker: string.

The fact that TS widens the object type to have marker: string unless the variable declaration is specifically marked with as const seems like counter-intuitive and incorrect behaviour.

It's probably worth filing an issue upstream.

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for maintainers to take a look labels Mar 19, 2024
@klassm

This comment was marked as off-topic.

@bradzacher
Copy link
Member

@klassm you can remove the first as const just fine and TS will correctly infer all of the types. and report no errors.
It's the second one that is required and is incorrectly flagged by the rule.
That is a separate issue to this one though - please file it separately.

This issue is specifically about const foo = 'string' as const which is completely unnecessary based on types.
Your issue is specifically about const foo = `${string}string` as const which is necessary based on types.

@JulienBara-360
Copy link

Hi @bradzacher,
I met the same problem today.
If it helps, rolling back from v7.4.0 to v6.21.0 removed the false positive.
In the meantime, I kept typescript on v5.4.3.

@LukeNotable
Copy link

The fact that TS widens the object type to have marker: string unless the variable declaration is specifically marked with as const seems like counter-intuitive and incorrect behaviour.

It's probably worth filing an issue upstream.

Maybe, but as it stands the rule is doing more harm than good by wrongly flagging such as const as safely removable.

@kgregory
Copy link

Ran into this as well.

It appears to have been introduced with 7.3.0, possibly with #8558 - Does not occur after rolling back to 7.2.0.

@kirkwaiblinger
Copy link
Member

Hmmmm, note also that

-export const marker = "marker" as const;
+export const marker = "marker";
export interface Data {
  marker: typeof marker;
  value: string;
}
function createData(): Data {
- const base = { marker: marker };
+ const base = { marker: marker } as const;
  return { ...base, value: "" };
}

works... and makes a whole lot more sense anyway. It seems really odd that using as const in the first line would have an effect on the type of base granted marker has the same type whether or not as const is explicitly used.

@kgregory
Copy link

kgregory commented May 10, 2024

works... and makes a whole lot more sense anyway.

In this case, yes, but that will affect all other attributes of the object. For example, if base were of a type with an array attribute, applying as const to base would type that array as readonly. (see this playground)

@kirkwaiblinger
Copy link
Member

-export const marker = "marker" as const;
+export const marker = "marker";
export interface Data {
  marker: typeof marker;
  value: string;
}
function createData(): Data {
- const base = { marker: marker };
+ const base: { marker: typeof marker } = { marker: marker };
  return { ...base, value: "" };
}

works also.

As does

-export const marker = "marker" as const;
+export const marker = "marker";
export interface Data {
  marker: typeof marker;
  value: string;
}
function createData(): Data {
- const base = { marker: marker };
+ const base = { marker: marker as typeof marker };
  return { ...base, value: "" };
}

and, of course

-export const marker = "marker" as const;
+export const marker = "marker";
export interface Data {
  marker: typeof marker;
  value: string;
}
function createData(): Data {
 const base = { marker: marker };
- return { ...base, value: "" };
+ return { marker, value: "" };
}

So it has something to do specifically with the widening behavior when creating a mutable object whose type is inferred. More I think about it, it really just seems surprising to me personally that the example in the issue report works in the first place.

@kirkwaiblinger
Copy link
Member

works... and makes a whole lot more sense anyway.

In this case, yes, but that will affect all other attributes of the object. For example, if base were of a type with an array attribute, applying as const to base would type that array as readonly. (see this playground)

@kgregory yeah, that's a fair point. and that irked me too. See #8721 (comment) for 3 more examples that don't though 🤣

@kirkwaiblinger
Copy link
Member

kirkwaiblinger commented May 10, 2024

I also note that putting const marker = 'marker' as 'marker' also has the same impact in these examples as const marker = 'marker' as const.

I guess it seems like we're running into a case where a type assertion has broader effect than setting the type of the thing being asserted on.

Curious what other team members think

@kirkwaiblinger kirkwaiblinger added triage Waiting for maintainers to take a look and removed awaiting response Issues waiting for a reply from the OP or another party labels May 10, 2024
@kirkwaiblinger
Copy link
Member

kirkwaiblinger commented May 17, 2024

I've just learned that TS appears to have intentional behavior around the "freshness" of a const variable type (see microsoft/TypeScript#52473 (comment)), though it's not clear that it's a particularly external-facing concept. Would have to do some more research.

In any case, I think it explains what we're seeing here, though I'm not sure that it is obvious what we should actually do about it.

@bradzacher, thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look
Projects
None yet
Development

No branches or pull requests

7 participants