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

ESLint incorrectly flagging variable as any #6314

Closed
4 tasks done
mia1024 opened this issue Jan 8, 2023 · 11 comments
Closed
4 tasks done

ESLint incorrectly flagging variable as any #6314

mia1024 opened this issue Jan 8, 2023 · 11 comments
Labels
bug Something isn't working external This issue is with another package, not typescript-eslint itself

Comments

@mia1024
Copy link

mia1024 commented Jan 8, 2023

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.

Issue Description

When running Typescript ESLint on our code, we get

/hyperschedule/frontend/src/lib/search.ts
  46:1   warning  Unsafe member access .course on an `any` value             @typescript-eslint/no-unsafe-member-access
  48:9   warning  Unsafe member access .course on an `any` value             @typescript-eslint/no-unsafe-member-access
...

However, if you look at affected code, (GitHub permanent link)

export function matchesText(section: Section, text: string): boolean {
    const terms: string[] = [];
    for (const match of text.matchAll(/[A-Za-z]+|\d+/g))
        terms.push(match[0].toLocaleLowerCase());

    const items = [
        section.course.code.department,
        section.course.code.courseNumber.toString().padStart(3, "0"),
        section.course.code.affiliation,
        section.identifier.sectionNumber.toString().padStart(2, "0"),
        section.course.title,
    ];
...
}

Typescript ESLint thinks that the section variable is of type any, while it is explicitly defined to be of type Section. This cascaded into further warnings about unsafe-call because it thinks some functions are undefined.

I would expect no linter warning is triggered on this particular snippet of code.

Reproduction Repository Link

https://github.com/MuddCreates/hyperschedule/

Repro Steps

  1. clone the repo and check out commit feafaaa599ab282af83babed490346a3c579ed08
  2. yarn install
  3. yarn lint

Versions

package version
@typescript-eslint/eslint-plugin 5.48.0
@typescript-eslint/parser 5.48.0
TypeScript 4.9.4
ESLint 8.30.0
node 18.8.0
@mia1024 mia1024 added bug Something isn't working triage Waiting for team members to take a look labels Jan 8, 2023
@JoshuaKGoldberg
Copy link
Member

Funny, I was just poking at a similar bug in t3-oss/create-t3-app#1036 (comment). I'd hoped microsoft/TypeScript#51914 would fix things but trying out typescript@5.0.0-dev.20230108 on your repo still shows the issue you're reporting. More research needed. Thanks for filing this issue!

@bradzacher
Copy link
Member

Are you able to create a more distilled reproduction here?
There's a lot of code and config here which makes it really hard for us to investigate - really looking for a needle in a haystack.

If you can help us by reducing this to a minimal reproduction in its own repo, we will be able to better prioritise this.
For context - as volunteer maintainers we don't have a lot of spare bandwidth, which makes prioritising this kind of vague and widely scoped issue hard for us to prioritise given it could take many hours to find the issue. A minimal repro narrows that scope down so that we can debug faster and easier.

I'd suggest forking the repo and deleting irrelevant files (non-eslint or ts config files, files not in the problem file's dependency tree) and irrelevant code from the problem file(s) until you distill it down completely.

@bradzacher
Copy link
Member

Worth noting that on the commit you linked - I can see there are dozens of unsafe any lint reports - not just in the one file.

/home/runner/work/hyperschedule/hyperschedule/backend/src/hmc-api/parser.ts
Warning:   122:22  warning  Unsafe argument of type `any` assigned to a parameter of type `Record<keyof Fields, string>`  @typescript-eslint/no-unsafe-argument

/home/runner/work/hyperschedule/hyperschedule/backend/src/logger.ts
Warning:   36:15  warning  Unsafe assignment of an `any` value  @typescript-eslint/no-unsafe-assignment

/home/runner/work/hyperschedule/hyperschedule/backend/src/pom-api/utils.ts
Warning:   1:18  warning  'APIv4' is defined but never used  @typescript-eslint/no-unused-vars

/home/runner/work/hyperschedule/hyperschedule/backend/src/routes/courses.ts
Warning:   24:54  warning  Unsafe assignment of an `any` value  @typescript-eslint/no-unsafe-assignment
Warning:   30:52  warning  Unsafe assignment of an `any` value  @typescript-eslint/no-unsafe-assignment
Warning:   74:54  warning  Unsafe assignment of an `any` value  @typescript-eslint/no-unsafe-assignment
Warning:   86:52  warning  Unsafe assignment of an `any` value  @typescript-eslint/no-unsafe-assignment

/home/runner/work/hyperschedule/hyperschedule/backend/src/server.ts
Warning:   14:37  warning  'address' is defined but never used  @typescript-eslint/no-unused-vars

/home/runner/work/hyperschedule/hyperschedule/frontend/src/hooks/api.ts
Warning:   8:5  warning  Unsafe return of an `any[]` typed value  @typescript-eslint/no-unsafe-return

/home/runner/work/hyperschedule/hyperschedule/frontend/src/lib/config.ts
Warning:   1:14  warning  Unsafe assignment of an `any` value  @typescript-eslint/no-unsafe-assignment

/home/runner/work/hyperschedule/hyperschedule/frontend/src/lib/search.ts
Warning:   48:9   warning  Unsafe member access .course on an `any` value             @typescript-eslint/no-unsafe-member-access
Warning:   49:9   warning  Unsafe member access .padStart on an `any` value           @typescript-eslint/no-unsafe-member-access
Warning:   49:9   warning  Unsafe call of an `any` typed value                        @typescript-eslint/no-unsafe-call
Warning:   49:9   warning  Unsafe member access .course on an `any` value             @typescript-eslint/no-unsafe-member-access
Warning:   49:9   warning  Unsafe call of an `any` typed value                        @typescript-eslint/no-unsafe-call
Warning:   50:9   warning  Unsafe member access .course on an `any` value             @typescript-eslint/no-unsafe-member-access
Warning:   51:9   warning  Unsafe member access .padStart on an `any` value           @typescript-eslint/no-unsafe-member-access
Warning:   51:9   warning  Unsafe call of an `any` typed value                        @typescript-eslint/no-unsafe-call
Warning:   51:9   warning  Unsafe member access .identifier on an `any` value         @typescript-eslint/no-unsafe-member-access
Warning:   51:9   warning  Unsafe call of an `any` typed value                        @typescript-eslint/no-unsafe-call
Warning:   52:9   warning  Unsafe member access .course on an `any` value             @typescript-eslint/no-unsafe-member-access
Warning:   57:13  warning  Unsafe member access .includes on an `any` value           @typescript-eslint/no-unsafe-member-access
Warning:   57:13  warning  Unsafe call of an `any` typed value                        @typescript-eslint/no-unsafe-call
Warning:   57:13  warning  Unsafe member access .toLocaleLowerCase on an `any` value  @typescript-eslint/no-unsafe-member-access
Warning:   57:13  warning  Unsafe call of an `any` typed value                        @typescript-eslint/no-unsafe-call

✖ 25 problems (0 errors, 25 warnings)

github action

so there's a lot of problems across all packages in your codebase. Which says to me that it's unlikely this is a bug, more likely this is a config issue instead.

Distilling down a repro will help identify that.

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for team members to take a look labels Jan 8, 2023
@kwshi
Copy link

kwshi commented Jan 8, 2023

@bradzacher we've distilled it down to a simple repro containing only the relevant files and type definitions here: https://github.com/MuddCreates/hyperschedule/tree/typescript-eslint%236314-repro. The same lint warnings remain.

Please let me know if that helps.

@JoshuaKGoldberg JoshuaKGoldberg added triage Waiting for team members to take a look and removed awaiting response Issues waiting for a reply from the OP or another party labels Jan 9, 2023
@jwerre
Copy link

jwerre commented Feb 10, 2023

I'm having a similar issue. I'm not sure if this helps but...

export let validationRules: string[] | null = null;
let isRequired = validationRules?.includes('require');

// linting errors:
52:6   error  Unsafe assignment of an `any` value  @typescript-eslint/no-unsafe-assignment
52:19  error  Unsafe call of an `any` typed value  @typescript-eslint/no-unsafe-call

However if I give validationRules value instead of null no more errors:

export let validationRules: string[]  = [];

@Josh-Cena
Copy link
Member

Josh-Cena commented Feb 10, 2023

@jwerre I've replied on Discord, but cross-posting here: this is a TS feature (which usually causes issues for prototyping): CFA eagerly narrows validationRules to its initializer, null, because validationRules is never re-assigned in your code. So TS thinks includes never exists. If you use null as string[] | null or declare let validationRules: string[] | null, it will work.

@Gekkio
Copy link

Gekkio commented Mar 6, 2023

I think I've just encountered this in our codebase...in our case the DOM window object becomes any, and triggers an error in every place where it's used (300+ eslint errors). This started happening when I upgraded one dependency to a newer version, but as far as I can tell, the dependency doesn't alter global types in any way, and no transitive dependencies were updated either. I've done some further investigation, and I'm not willing to blame this dependency just yet. We're using TS 4.9.5, typescript-eslint 5.54.0.

In our code base, I can reproduce the problem with one tiny test file, called bug.ts:

const problem: string = window
console.info(problem)

If I compile this with tsc, I get the expected compiler error TS2322: Type 'Window & typeof globalThis' is not assignable to type 'string'., because the assignment has an obvious type error. However, if I lint this file, I get these errors:

  1:7  error  Unsafe assignment of an `any` value  @typescript-eslint/no-unsafe-assignment
  2:1  error  Unexpected console statement         no-console

Note how it's not really complaining about the type error in the assignment, but the use of an any-typed value.

Running eslint with DEBUG=typescript-eslint:* gives some interesting information:

  typescript-eslint:typescript-estree:parser:parseSettings:resolveProjectList parserOptions.project (excluding ignored) matched projects: Set(1) {
  '/home/joonas/projects/espoo/evaka/frontend/tsconfig.eslint.json'
} +0ms
  typescript-eslint:typescript-estree:createProjectProgram Creating project program for: /home/joonas/projects/espoo/evaka/frontend/src/lib-common/bug.ts +0ms
  typescript-eslint:typescript-estree:createWatchProgram File did not belong to any existing programs, moving to create/update. /home/joonas/projects/espoo/evaka/frontend/src/lib-common/bug.ts +0ms
  typescript-eslint:typescript-estree:createWatchProgram Creating watch program for /home/joonas/projects/espoo/evaka/frontend/tsconfig.eslint.json. +0ms
  typescript-eslint:typescript-estree:createWatchProgram Found program for file. /home/joonas/projects/espoo/evaka/frontend/src/lib-common/bug.ts +3s
  typescript-eslint:parser:parser Resolved libs from program: [ 'dom', 'es2020' ] +0ms
  typescript-eslint:eslint-plugin:utils:types Found an "error" any type +0ms

This is the most interesting bit: Found an "error" any type. If I lint our entire project, I can see hundreds of these log lines, so clearly every lint error seems connected to this.

I also did a session with a debugger, and managed to narrow down the problem to this code in typescript-eslint:

const senderType = checker.getTypeAtLocation(esTreeNodeToTSNodeMap.get(senderNode));

In the case of the above bug.ts file, senderNode is basically {type: 'Identifier', name: 'window', ...}, and the returned senderType is basically {id: 4, intrinsicName: 'error', flags: 1, objectFlags: 0}, which then triggers later the log line mentioned earlier and seems to cause the lint error.

If we continue a bit further, here's getTypeAtLocation which seems to be a part of TS compiler itself:

getTypeAtLocation: function (nodeIn) {
    var node = ts.getParseTreeNode(nodeIn);
    return node ? getTypeOfNode(node) : errorType;
},

The returned node from getParseTreeNode is not null/undefined, so we end up calling getTypeOfNode and several other functions. Eventually we end up in a function called checkIdentifier, which is where the error type is ultimately returned.

function checkIdentifier(node, checkMode) {
    if (ts.isThisInTypeQuery(node)) {
        return checkThisExpression(node);
    }
    var symbol = getResolvedSymbol(node);
    if (symbol === unknownSymbol) {
        return errorType;
    }

The last if statement is true, so this is where the error type is returned. It seems that getResolvedSymbol considers window to be an unknown symbol for some reason, which leads to the expression to having the any "error" type, which leads to the lint errors. But this seems to only happen with eslint and not when invoking tsc directly.

Sorry for the wall of text 😅 I'm totally out of my league here, and don't have a good guess of where the problem ultimately is. And if it was a config problem, I don't understand why updating one random dependency would suddenly cause it and reverting the upgrade would fix it. My recommendation for others having the same issue is to try to use the DEBUG=typescript-eslint:* environment variable and see if you get the same error any type log lines as we do.

@bradzacher
Copy link
Member

@Gekkio ths problem is that whatever config you're using to lint your codebase with does not include the DOM lib, so window is not globally defined.
So when we attempt to resve the type of window, TS sees an untyped variable - hence the "error" any.

@Gekkio
Copy link

Gekkio commented Mar 6, 2023

@Gekkio ths problem is that whatever config you're using to lint your codebase with does not include the DOM lib, so window is not globally defined. So when we attempt to resve the type of window, TS sees an untyped variable - hence the "error" any.

Note that typescript-eslint parser says in the debug logs I provided that the resolved libs were [ 'dom', 'es2020' ]. Unless there are somehow multiple configurations that apply to this simple test case of linting a single file, I don't see how the library configuration would be the problem. I also can't think of a way how upgrading/downgrading a single dependency that isn't even used by this test file would suddenly break/fix linting if it was a simple configuration problem.

I can literally remove the dependency and its transitive dependencies from node_modules and it doesn't fix anything. Simply the original act of updating this unrelated dependency triggers the problem:

❯ npx eslint --ext .js,.jsx,.ts,.tsx src/lib-common/bug.ts

/home/joonas/projects/espoo/evaka/frontend/src/lib-common/bug.ts
  2:1  error  Unexpected console statement  no-console

✖ 1 problem (1 error, 0 warnings)       <-- no "any" lint errors

❯ yarn up react-spring
....(omitted for brevity)
➤ YN0000: Done with warnings in 3s 918ms

❯ npx eslint --ext .js,.jsx,.ts,.tsx src/lib-common/bug.ts

/home/joonas/projects/espoo/evaka/frontend/src/lib-common/bug.ts
  1:7  error  Unsafe assignment of an `any` value  @typescript-eslint/no-unsafe-assignment
  2:1  error  Unexpected console statement         no-console

✖ 2 problems (2 errors, 0 warnings)       <-- "any" lint error appears

❯ rm -rf node_modules/react-spring node_modules/@react-spring

❯ npx eslint --ext .js,.jsx,.ts,.tsx src/lib-common/bug.ts   

/home/joonas/projects/espoo/evaka/frontend/src/lib-common/bug.ts
  1:7  error  Unsafe assignment of an `any` value  @typescript-eslint/no-unsafe-assignment
  2:1  error  Unexpected console statement         no-console

✖ 2 problems (2 errors, 0 warnings)       <-- "any" lint error remains

Maybe I'm overthinking this, but it smells to me like some kind of ordering problem...like something traversing node_modules and being sensitive to the order. And when I update (and remove) this unrelated dependency, the file system returns something in a different order when directory contents are queried.

@Gekkio
Copy link

Gekkio commented Mar 7, 2023

I debugged our problem a bit more by using strace to see which files get opened by the node process. In the problem cases TS lib.dom.d.ts wasn't listed, so it definitely looks like a config issue after all, despite eslint logging that dom lib is configured. Since everybody else has the lint errors coming from custom types instead of built-in lib types, our issue is clearly a different thing despite the symptoms being similar...I'll investigate more and continue in a separate issue if I find anything wrong with typescript-eslint (unlikely, but possible). Sorry for the spam here! 😅

@JoshuaKGoldberg
Copy link
Member

Ha, I wouldn't count this as spam at all. It's a super interesting investigation. And someone else is bound to have previously had this kind of situation. +1 to having the investigation on record & searchable in the issue tracker. Thanks @Gekkio! 🙌

@JoshuaKGoldberg JoshuaKGoldberg added external This issue is with another package, not typescript-eslint itself and removed triage Waiting for team members to take a look labels Mar 9, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working external This issue is with another package, not typescript-eslint itself
Projects
None yet
Development

No branches or pull requests

7 participants