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: single run mode breaks with fixers #9344

Closed
4 tasks done
hasakilol opened this issue Jun 13, 2024 · 7 comments
Closed
4 tasks done

Bug: single run mode breaks with fixers #9344

hasakilol opened this issue Jun 13, 2024 · 7 comments
Assignees
Labels
bug Something isn't working locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: typescript-estree Issues related to @typescript-eslint/typescript-estree team assigned A member of the typescript-eslint team should work on this.
Milestone

Comments

@hasakilol
Copy link

hasakilol commented Jun 13, 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

https://github.com/hasakilol/react-vite/blob/master/src/Test.tsx

Repro Code

  1. This is update to: Bug: [no-duplicate-type-constituent] fixer removes undefined from type #9230
  2. I can't give you a reproduction on https://typescript-eslint.io/play/ because I'm using latest eslint and typescript-eslint packages.

ESLint Config

import tseslint from 'typescript-eslint';
import globals from 'globals';
import parser from '@typescript-eslint/parser';


export default tseslint.config(
  {
    name: 'plugins',
    plugins: {
      ['@typescript-eslint']: tseslint.plugin,
    },
  },
    
  {
    languageOptions: {
      globals: {
        ...globals.node,
        ...globals.es2015,
      },
      parserOptions: {
        allowAutomaticSingleRunInference: true,
        project: ['tsconfig.json'],
        tsconfigRootDir: '.',
      }
    },
  },

  {
    name: 'src',
    files: ['src/**/*.ts', 'src/**/*.tsx', 'src/**/*.mts', 'src/**/*.cts'],
    extends: tseslint.configs.recommendedTypeChecked,
    languageOptions: {
      parser,
      parserOptions: {
        ecmaFeatures: {
          jsx: true
        },
        sourceType: 'module',
      },
      globals: {
        ...globals.browser,
        ...globals.mocha,
        ...globals.jest,
        ...globals.jasmine,
      },
    },
    rules: {
      '@typescript-eslint/consistent-type-imports': 'error',
      '@typescript-eslint/no-duplicate-type-constituents': 'error'
    }
  },

    
);

tsconfig

{
  "compilerOptions": {
    "target": "ES2020",
    "useDefineForClassFields": true,
    "lib": ["ES2020", "DOM", "DOM.Iterable"],
    "module": "ESNext",
    "skipLibCheck": true,

    /* Bundler mode */
    "moduleResolution": "bundler",
    "allowImportingTsExtensions": true,
    "resolveJsonModule": true,
    "isolatedModules": true,
    "noEmit": true,
    "jsx": "react-jsx",

    /* Linting */
    "strict": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noFallthroughCasesInSwitch": true
  },
  "include": ["src"],
  "references": [{ "path": "./tsconfig.node.json" }]
}

Expected Result

https://github.com/hasakilol/react-vite/blob/master/src/Test.tsx#L8

No matter how you configure @typescript-eslint/consistent-type-imports ('off' or 'error') and @typescript-eslint/no-duplicate-type-constituents ('off' or 'error'),
item: { a: string; } | typeof Null | typeof Undefined, should not change after running npx eslint --fix src/Test.tsx.

Actual Result

When both @typescript-eslint/consistent-type-imports and @typescript-eslint/no-duplicate-type-constituents are configured error, item: { a: string; } | typeof Null | typeof Undefined, is changed to item: { a: string; } | typeof Null , after running npx eslint --fix src/Test.tsx.

When either @typescript-eslint/consistent-type-imports or @typescript-eslint/no-duplicate-type-constituents are configured off, item: { a: string; } | typeof Null | typeof Undefined, will not change (what I expect) after running npx eslint --fix src/Test.tsx.

Additional Info

"@typescript-eslint/eslint-plugin": "^8.0.0-alpha.14",
"@typescript-eslint/parser": "^8.0.0-alpha.14",
"@typescript-eslint/scope-manager": "^8.0.0-alpha.14",
"@typescript-eslint/type-utils": "^8.0.0-alpha.14",
"@typescript-eslint/types": "^8.0.0-alpha.14",
"@typescript-eslint/typescript-estree": "^8.0.0-alpha.14",
"@typescript-eslint/utils": "^8.0.0-alpha.14",
"@typescript-eslint/visitor-keys": "^8.0.0-alpha.14",
"typescript-eslint": "^8.0.0-alpha.14",

"eslint": "^9.3.0",

@hasakilol hasakilol 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 Jun 13, 2024
@bradzacher bradzacher added package: typescript-estree Issues related to @typescript-eslint/typescript-estree accepting prs Go ahead, send a pull request that resolves this issue and removed package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jun 13, 2024
@bradzacher bradzacher changed the title Bug: [no-duplicate-type-constituents] affected by consistent-type-imports rule Bug: single run mode breaks with fixers Jun 13, 2024
@bradzacher bradzacher added this to the 8.0.0 milestone Jun 13, 2024
@bradzacher
Copy link
Member

bradzacher commented Jun 13, 2024

You're mistaken about the cause.
It's got nothing to do with the specific rules you're using - it's a bug with the "single run" inference infrastructure.
If you pass disallowSingleRunInference: true it'll work around things for now.

debug output with single-run inference turned on:

$ DEBUG=typescript-eslint:* npx eslint --fix ./src/Test.tsx
  typescript-eslint:typescript-estree:parser:parseSettings:resolveProjectList parserOptions.project (excluding ignored) matched projects: Map(1) { 'tsconfig.json' => 'tsconfig.json' } +0ms
  typescript-eslint:typescript-estree:useProvidedProgram Retrieving ast for /Users/bjz/temp/react-vite-master/src/Test.tsx from provided program instance(s) +0ms
  typescript-eslint:typescript-estree:parser Detected single-run/CLI usage, creating Program once ahead of time for project: [ 'tsconfig.json', 'tsconfig.json' ] +0ms
  typescript-eslint:parser:parser Resolved libs from program: [ 'es2020', 'dom', 'dom.iterable' ] +0ms
  typescript-eslint:typescript-estree:createIsolatedProgram Getting isolated program in TSX mode for: /Users/bjz/temp/react-vite-master/src/Test.tsx +0ms
  typescript-eslint:parser:parser Resolved libs from program: [ 'esnext.full' ] +71ms
  typescript-eslint:eslint-plugin:utils:types Found an "error" any type +0ms
  typescript-eslint:eslint-plugin:utils:types Found an "error" any type +0ms
  typescript-eslint:eslint-plugin:utils:types Found an "error" any type +1ms
  typescript-eslint:typescript-estree:createIsolatedProgram Getting isolated program in TSX mode for: /Users/bjz/temp/react-vite-master/src/Test.tsx +13ms
  typescript-eslint:parser:parser Resolved libs from program: [ 'esnext.full' ] +12ms
  typescript-eslint:eslint-plugin:utils:types Found an "error" any type +10ms
  typescript-eslint:eslint-plugin:utils:types Found an "error" any type +1ms

/Users/bjz/temp/react-vite-master/src/Test.tsx
   8:28  error  'any' overrides all other types in this union type  @typescript-eslint/no-redundant-type-constituents
  15:9   error  Unsafe assignment of an `any` value                 @typescript-eslint/no-unsafe-assignment
  19:3   error  Unsafe return of an `any` typed value               @typescript-eslint/no-unsafe-return
  19:33  error  Unsafe return of an `any` typed value               @typescript-eslint/no-unsafe-return
  19:33  error  Unsafe call of an `any` typed value                 @typescript-eslint/no-unsafe-call

✖ 5 problems (5 errors, 0 warnings)

Note all of the Found an "error" any type logs.

The first lint pass fixes the first import to add the type specifier.

ESLint then does a second lint pass (it tries to exhaustively apply fixers). During this lint pass for whatever reason the type information is borked and so both the Null and Undefined imports are any (hence there are 3 Found an "error" any type logs - one for each usage of the imports.)

This then causes no-duplicate-type-constituents to see T | any | any - so the rule then acts on this information to remove the second any as it is superfluous based on the types. I.e. it removes the | typeof Undefined from the code.

Then there's the 3rd and final lint pass after that fixer and now there's only 2 Found an "error" any type logs because one of the usages was removed.

Which leaves you with the final broken code and lint errors.


cc @JoshuaKGoldberg this is a blocker for the v8 release as we can't rollout single run inference by default with this bug.

Note: I haven't investigated any further than just doing a debug run in the user's repo.

@bradzacher
Copy link
Member

This then causes no-duplicate-type-constituents to see T | any | any - so the rule then acts on this information to remove the second any as it is superfluous based on the types.

TBH this is a separate bug that should be fixed
no-duplicate-type-constituents should probably detect that a type is an "error any" and treat it as a completely unique type each time so that it doesn't accidentally change code in the case of broken types.

@JoshuaKGoldberg
Copy link
Member

YES this is the bug we were talking about wanting a repro for! (can't find the reference right now, but it's somewhere)

Relevant: the current limitations of ESLint described in eslint/rfcs#102, as with other issues, makes this tricky.

@JoshuaKGoldberg
Copy link
Member

@bradzacher
Copy link
Member

@JoshuaKGoldberg I think this is the real bug we should look at fixing:

ESLint then does a second lint pass (it tries to exhaustively apply fixers). During this lint pass for whatever reason the type information is borked and so both the Null and Undefined imports are any (hence there are 3 Found an "error" any type logs - one for each usage of the imports.)

We can disable single-run inference during a fix run as a band-aid, but we probably want to fix the underlying bug.

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Jul 18, 2024

the real bug we should look at fixing:

... During this lint pass for whatever reason the type information is borked

Agreed. I have no idea how to fix that right now though. 😬

@JoshuaKGoldberg JoshuaKGoldberg added team assigned A member of the typescript-eslint team should work on this. and removed accepting prs Go ahead, send a pull request that resolves this issue labels Jul 18, 2024
@JoshuaKGoldberg JoshuaKGoldberg self-assigned this Jul 18, 2024
@JoshuaKGoldberg
Copy link
Member

Fixed by #9577.

@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 Jul 27, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: typescript-estree Issues related to @typescript-eslint/typescript-estree team assigned A member of the typescript-eslint team should work on this.
Projects
None yet
Development

No branches or pull requests

3 participants