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

Type Aware Linting #3105

Open
Boshen opened this issue Apr 25, 2024 · 7 comments
Open

Type Aware Linting #3105

Boshen opened this issue Apr 25, 2024 · 7 comments
Labels
A-linter Area - Linter

Comments

@Boshen Boshen added the A-linter Area - Linter label Apr 25, 2024
@Boshen Boshen self-assigned this Apr 25, 2024
@Bhavyajain21
Copy link

Can I be assigned?

@Boshen
Copy link
Member Author

Boshen commented Apr 27, 2024

@Bhavyajain21 #2912 is getting there, do you wanna help that out or do you want to experiment with other setups?

btw the bounty was just a demonstration, it is now removed.

@algora-pbc algora-pbc bot removed the 💎 Bounty label Apr 27, 2024
@Boshen
Copy link
Member Author

Boshen commented Apr 28, 2024

https://typescript-eslint.io/getting-started/typed-linting/#how-is-performance

@valeneiko I have another idea. Is it viable to build and get all .d.ts + @types files and statically check against them? We would end up supporting some of the typing rules without sacrificing performance.

I think no-floating-promise can be done statically by marking a function as async just like how #[must_use] work in rustc 🤔

I'm not sure about the others rules, but the most important rules oxlint provide should be the ones checking for correctness. All the other ones can deprioritized.

@valeneiko
Copy link

I had a similar optimization in mind to make us call typechecker less. Basically if we have a list of functions marked async or returning a Promise we could use it to check some of the CallExpressions. And we would not need .d.ts, at least for the ones explicitly marked async or with explicit Promise return type annotation.

Generating .d.ts would take quite a long time for a limited benefit:

  • TSC removes all async annotations, so we would have to look for functions returning Promise
  • Return type on function that weren't annotated async in source will often be a union with a Promise instead of simple Promise
  • It would not help with class X extends Promise, where function returns X. Which is a pretty common pattern.

I don't think this alone would be sufficient to cover no-floating-promise. It would be fairly easy to do for standalone functions, but once member method calls get involved, we would have to have a significat portion of TypeScript in order to resolve them.

@magic-akari
Copy link
Collaborator

The key issue is cross-file knowledge.
A lot of Type information relies on information from other files.

Do we need this cross-file support?
If so, then .d.ts is a shortcut to implementation.

@Boshen
Copy link
Member Author

Boshen commented Apr 28, 2024

Do we need this cross-file support?

Multi-file analysis works under the --import-plugin flag.

@Boshen Boshen removed their assignment Apr 29, 2024
@Boshen
Copy link
Member Author

Boshen commented May 8, 2024

Current progress and conclusion from #2912:

The PR managed to build a ts server and communicate with it from the Rust side.

But we hit a blocker where we would end up rebuilding @typescript-eslint: we need to convert the typescript ast to our ast so we can have a matching span position to query on.

@valeneiko and I felt like it's not worth it to maintain all these code and see no performance improvements.

So instead, I propose the following:

Given that we are a static analysis tool, we should use static type information from typescript instead of getting them dynamically.

TypeScript is heading towards this direction by having explicit-module-boundary-types and isolated-declarations.

Since we now have the infrastructure for multi-file analysis, we can leverage this by reading and parsing .d.ts files, work out the semantics and query typing information statically.

There is going to be a lot edge cases, so what I'm going to do next is think of a minimal scenario, experiment and test on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter
Projects
None yet
Development

No branches or pull requests

4 participants