linter: integrate tsgolint

[Boshen]

oxc-project/tsgolint#21

[camchenry]

Documenting my thoughts here after working on #12343, which was a proof of concept PR to see if integrating tsgolint would be possible. I believe it was a success.

What was implemented

• typescript-eslint/no-floating-promises rule was added
  • This was done by adding a normal impl Rule for NoFloatingPromises, and then setting should_run to false. This filters it out from ever running.
• Spawned a tsgolint process in a separate thread and created a JSON input matching the expected format, and then parsing the JSON output synchronously once tsgolint finished.
• Parsing tsgolint diagnostics into OxcDiagnostic, matching our format, so that it can be added to the oxlint diagnostics at the end.

Things that work well

• tsgolint seems to be fast enough for practical use. I don't have any official benchmarks to share, but it's not orders of magnitude slower than oxlint at least. We also still have ways to make both oxlint and our tsgolint implementation faster (for example: linter: dynamically skip running rules that do not have the relevant node types #12223).
• Using impl Rule and declare_oxc_lint! for tsgolint rules. I'm not sure there is a practical alternative here, but it is hugely beneficial for us. Even though none of the implementation is in oxlint, we can still add auto-generated documentation and manage all of the configuration for the rules in the same way as native Rust lint rules.

Open questions / risks / things not implemented

• How do we stream and parse the diagnostics from tsgolint? tsgolint is designed to emit diagnostics as it finds them, so we can start parsing right away, but the proof of concept only waited until all diagnostics were emitted.
• How do we integrate this with oxc_language_server? Currently the proof of concept is all implemented in oxlint. We may have to duplicate the implementation.
• How do we convert fixes and suggestions from tsgolint into oxlint fixes? Also, can we add help: text from tsgolint diagnostics?
• Will the performance degrade significantly when we implement all of the rules?

Next steps

• I plan to start working on a more polished version of tsgolint integration in oxlint.
• We will add a --experimental-tsgolint flag to opt-in to enabling the typed lint rules, due to its potentially unstable functionality. We will aim to make it usable, but users should not expect true stability until a major release.
• Since adding new correctness rules by default will cause lots of builds to potentially break, once we reach a major breaking changes release for oxlint (2.0?), we can enable the tsgolint integration by default.
• We will need to explore how to install tsgolint alongside oxlint (Publish to npm tsgolint#31)

[camc314]

Nice work!

Using impl Rule and declare_oxc_lint! for tsgolint rules. I'm not sure there is a practical alternative here, but it is hugely beneficial for us. Even though none of the implementation is in oxlint, we can still add auto-generated documentation and manage all of the configuration for the rules in the same way as native Rust lint rules.

I wonder whether we should codegen the typechecked lint rules (on the rust side), such that the config/docs, ect (the source of truth), can live in tsgolint, while still having the advantage of of website gen being simpler ect

How do we integrate this with oxc_language_server? Currently the proof of concept is all implemented in oxlint. We may have to duplicate the implementation.

My initial thoughts, is that this may be really hard. if memory serves, the language server operates on in-memory documents (not the raw file system), as the user may have made edit, but not written them to the disk. If we wanted to do this in the ideal way, we would have to pass the document to tsgolint (via stdin ?), and get tsgolint to use that version of the file rather than reading from the disk. That said, integration with the lsp can definitly be done as a followup and is more of a nice to have imo rather than essential to release a first version.

How do we convert fixes and suggestions from tsgolint into oxlint fixes? Also, can we add help: text from tsgolint diagnostics?

I think tsgolint should return something in a shape as close to OxcDiagnostic as possible. This means that still have all the advantages of detailed error reporting, while keeping it simple to map from the go to rust type, and will still allow us to support multiple spans, help text ect.

[auvred]

oxlint-tsgolint is now available on npm.

I'm not sure what's the best way to resolve tsgolint binary path from oxlint binary. Perhaps it could be resolved with oxc_resolve or with require.resolve/import.meta.resolve in JS, and then passed as an argument to execFileSync(oxlint-binary).

Also, I think it's worth providing an option to specify a custom path to the tsgolint binary, e.g., via an environment variable like OXLINT_TSGOLINT_PATH. This would simplify testing and make packaging (e.g., in Nixpkgs) easier.

[camchenry]

I'm not sure what's the best way to resolve tsgolint binary path from oxlint binary. Perhaps it could be resolved with oxc_resolve or with require.resolve/import.meta.resolve in JS, and then passed as an argument to execFileSync(oxlint-binary).

Maybe for now (while we are testing) we require it to be installed globally and defer to the $PATH resolution for locating the tsgolint binary. I don't think we need to use oxc_resolve for resolution though in the future, as it shouldn't be too complicated to find the tsgolint binary. It should be installed in one of these places (in order of preference):

• $OXLINT_TSGOLINT_PATH/tsgolint (or whatever we choose to name this env variable)
• $CWD/node_modules/.bin/tsgolint
• $PATH/tsgolint

If necessary, we could also add a new property to the .oxlintrc.json file, but I think it would be preferable to avoid that and just have it work by default. I'm not a npm expert, but I think the ideal experience is that you run npm i oxlint@latest in your project, and then that adds a dependency on @oxlint-tsgolint/core which automatically installs it to node_modules/.bin/tsgolint, which then automatically finds the binary for the project.

Also, I think it's worth providing an option to specify a custom path to the tsgolint binary, e.g., via an environment variable like OXLINT_TSGOLINT_PATH. This would simplify testing and make packaging (e.g., in Nixpkgs) easier.

This is probably a good idea, and it would make testing easier if nothing else 👍

[camc314]

One other thought, which would impact this is, rather than having a separate package for the tsgolint binary, we could just bundle it with the oxlint package? While this would make building the packages/publishing oxlint a little more complicated, it might be better overall (less dependencies for users, simpler binary resolution ect).

[overlookmotel]

Personally I think requiring user to install a separate package alongside oxlint is not ideal.

If we are concerned about imposing cost of installing tsgolint on users who don't use type-aware linting, would it be feasible for oxlint to check if tsgolint binary is present when it finds a type-aware rule enabled, and to download it if it's not?

Or can we assume that almost everyone uses TS these days, and they all want type-aware linting (if you use TS, why wouldn't you?)? In which case, bundling tsgolint in oxlint seems like simplest solution for user.

[camc314]

Or can we assume that almost everyone uses TS these days, and they all want type-aware linting (if you use TS, why wouldn't you?)? In which case, bundling tsgolint in oxlint seems like simplest solution for user.

even for those not using TS, type aware rule will still be relevant, as JS file can still be type inferred (jsdoc or partial inference)

[Boshen]

A few points:

• https://www.npmjs.com/package/@oxlint-tsgolint/darwin-arm64 is 21.1 MB
• it's versioned differently and updated differently
• it ships on less platforms at present, so build matrix don't match

[auvred]

it ships on less platforms at present, so build matrix don't match

Go uses static linking by default, so there's no need to publish separate binaries for glibc and musl. A single binary will work on both. Therefore, build matrices of oxlint and tsgolint are aligned: [win32, linux, darwin] x [x64, arm64].