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?: type-utils > TypeOrValueSpecifier's file detection is case insensitive #6697

Closed
4 tasks done
JoshuaKGoldberg opened this issue Mar 20, 2023 · 5 comments
Closed
4 tasks done
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: type-utils Issues related to the @typescript-eslint/type-utils package
Milestone

Comments

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Mar 20, 2023

Before You File a Proposal Please Confirm You Have Done The Following...

Relevant Package

type-utils

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

Following up from https://github.com/typescript-eslint/typescript-eslint/pull/4436/files#r1134626465: in order to get the PR merged, the "is this type declared in one of these files?" logic (typeDeclaredInFile) logic was changed to be case insensitive. It .toLowerCase()s all strings.

That means this config:

{ from: 'file', name: 'MyType', path: './filename' }

...will match ./filename, ./Filename, ./FILENAME, and so on.

This case insensitivity probably won't matter in any projects. But most file systems are in fact case sensitive. So we really should get it to work at some point.

Additional Info

To start on this issue, remove the .toLowerCase() calls in typeDeclaredInFile.

Re-run unit tests for type-utils and eslint-plugin. You'll also want to add unit tests to TypeOrValueSpecifier.test.ts to make sure that case sensitivity is respected.

@JoshuaKGoldberg JoshuaKGoldberg added enhancement New feature or request accepting prs Go ahead, send a pull request that resolves this issue labels Mar 20, 2023
@JoshuaKGoldberg JoshuaKGoldberg changed the title Bug?: TypeOrValueSpecifier's file detection is case insensitive Bug?: type-utils > TypeOrValueSpecifier's file detection is case insensitive Mar 20, 2023
@JoshuaKGoldberg JoshuaKGoldberg added bug Something isn't working package: type-utils Issues related to the @typescript-eslint/type-utils package and removed enhancement New feature or request labels Mar 20, 2023
@bradzacher bradzacher added this to the 6.0.0 milestone Mar 20, 2023
@marekdedic
Copy link
Contributor

From the linked review, on the machine of @JoshuaKGoldberg:

  • relativePath is 'tests/fixtures/file.ts'
  • absolutePath is '/Users/josh/repos/typescript-eslint/packages/type-utils/tests/fixtures/file.ts
  • declarationFiles[0].fileName is '/users/josh/repos/typescript-eslint/packages/type-utils/tests/fixtures/file.ts'

@bradzacher
Copy link
Member

There's a function in typescript-estree called getCannonicalFilename that you can use to make sure it matches TS

@marekdedic
Copy link
Contributor

On my machine:

  • realtivePath is tests/fixtures/file.ts
  • absolutePath is /home/marekdedic/Documents/git/typescript-eslint/packages/type-utils/tests/fixtures/file.ts
  • declarationFiles[0].fileName is /home/marekdedic/Documents/git/typescript-eslint/packages/type-utils/tests/fixtures/file.ts

I'm on Linux and I presume @JoshuaKGoldberg is on a Mac. It seems like declarationFiles[0].fileName is getting lower-cased on Mac and not on Linux...

@JoshuaKGoldberg
Copy link
Member Author

Btw, yes, I'm on a Mac. 👍

@JoshuaKGoldberg
Copy link
Member Author

Fixed in #6781, will be released in v6 🚀. Thanks @marekdedic as always!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: type-utils Issues related to the @typescript-eslint/type-utils package
Projects
None yet
Development

No branches or pull requests

3 participants