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

($470 Bounty) Type Aware Linting #3105

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

($470 Bounty) Type Aware Linting #3105

Boshen opened this issue Apr 25, 2024 · 9 comments
Labels

Comments

@Boshen
Copy link
Member

Boshen commented Apr 25, 2024

Background: https://typescript-eslint.io/getting-started/typed-linting/

Attempt 1:

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

This comment was marked as outdated.

@Boshen

This comment was marked as outdated.

@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
Contributor

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.

@Boshen Boshen changed the title Type Aware Linting ($400 Bounty) Type Aware Linting Jun 11, 2024
@Boshen Boshen pinned this issue Jun 11, 2024
@chenglou
Copy link

How would file-local type linting work though? Since the TS proposal only types exports

@algora-pbc algora-pbc bot removed the 💎 Bounty label Jun 11, 2024
Copy link

algora-pbc bot commented Jun 11, 2024

💎 $400 bounty • chenglou92

💎 $50 bounty • KubaJastrz

💎 $20 bounty • eric.erwan.le.maitre

Steps to solve:

  1. Start working: Comment /attempt #3105 with your implementation plan
  2. Submit work: Create a pull request including /claim #3105 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to oxc-project/oxc!

Add a bountyShare on socials

@Boshen Boshen changed the title ($400 Bounty) Type Aware Linting ($470 Bounty) Type Aware Linting Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants