deno_lint icon indicating copy to clipboard operation
deno_lint copied to clipboard

discussion: plugin support

Open lucacasonato opened this issue 4 years ago • 12 comments

It would be great if deno_lint had support for plugins that can add rules. Here are my thoughts:

Constraints:

  • Plugins should be written in Rust and use the same infrastructure as the rules that are part of this repository.
  • Plugins should be loaded at runtime, and should not have to be available at compile time. This means they need to be dynamically executed.
  • Plugins should not have to re-parse the source code.
  • Plugins should be sandboxed and safe - they should not be able to access the system directly.

Possible solution: Most likely the best solution that satisfies these constraints is WebAssembly. You can write your rules in Rust, and then compile them to WebAssembly that deno_lint could load and run. WebAssembly is also safe because it is sandboxed and only gets access to resources that deno_lint exposes to it. Here is how I imagine this would work:

  1. deno_lint should initialise all plugins at startup, by creating a WebAssembly VM for each plugin.
  2. deno_lint parses the source code into a swc AST. This ASTs raw memory is stored in a shared memory area with the WebAssembly plugin.
  3. deno_lint calls a function in WebAssembly to kick off code checking in the plugin. It can now do its analysis and create diagnostics. These diagnostics are stored in a separate section of shared memory between the plugin and deno_lint.
  4. deno_lint extracts these diagnostics and augments them with information about plugin origin and returns them to the user.

This approach would allow you to start multiple instances of the same plugin easily, to paralelnize code analysis for large modules. There are definitely some issues with this. The biggest one I can see right now is how to pass the AST between plugin and deno_lint. If we only support rust, we might be able to pass the raw backing of the object, but I don't know if this is feasible and how likely it is to break (probably very). Another solution would be to serialise to JSON or protobuf and then deserialise on the plugin side. This is definitely possible, but requires a lot of careful translation of the swc structs into JSON. This might be very complicated and time intensive, and would probably not be great for performance. The third solution would be to have the plugin do the parsing of code itself - this would also slow down the process significantly though.

Prior art: dprint also makes use of WebAssembly for its plugin system. To the end user looks like this: https://dprint.dev/plugin-dev/ (very clean)

lucacasonato avatar Jun 17 '20 19:06 lucacasonato

The protocol for serialization between plugins and deno_lint would be cap'n proto, right? Just to have a standard. This way it wouldn't break.

umutzd avatar Oct 17 '20 21:10 umutzd

The protocol for serialization between plugins and deno_lint would be cap'n proto, right? Just to have a standard. This way it wouldn't break.

That's highly unlikely, there's no need to introduce cap'n'proto because AST is already JSON serializable.

bartlomieju avatar Oct 18 '20 11:10 bartlomieju

Update: the plugin system had been implemented long time ago by @magurotuna in examples/dlint: https://github.com/denoland/deno_lint/blob/8295bc9d4cccbb77de05762f6afd24e7a747ccd2/examples/dlint/main.rs#L181-L184

Once https://github.com/denoland/deno/pull/11776 lands we'll be unblocked to add support for plugins in deno itself.

bartlomieju avatar Aug 27 '21 14:08 bartlomieju

Discussed offline with @magurotuna, we got a clear plan how to ship this feature in the near future.

Most likely we'll go with simple JS API to register multiple rules from a single plugin:

import NoConsole from "./no_console.ts";
import Eqeqeq from "./eqeqeq.ts";


DenoLint.register({
  name: "mySuperPlugin",
  rules: [
    NoConsole,
    Eqeqeq,
  ] 
});

bartlomieju avatar Sep 23 '21 14:09 bartlomieju

The bees knees would be an AST bridge between (deno_lint plugin, swc-ast) => (eslint plugin, babel-ast). i'd be excited to even contribute to it. iirc there was a thread w/ one of the eslint maintainers talkin' about ways to help. maybe we can start talkin along these lines w/ him?

cdaringe avatar Oct 15 '21 20:10 cdaringe

@cdaringe that would require support for emitting ESLint compatible AST (estree) from SWC. There was https://github.com/swc-project/swc/issues/2123 but it hadn't moved anywhere. Current idea by @dsherret is to add a slower path to deno_lint that would re-parse files in typescript-eslint and then pass it down to eslint. However no work has started on this subject, and it might wait some more before we look into it.

bartlomieju avatar Oct 19 '21 01:10 bartlomieju

Gotcha, thanks for the ref. Genuine ask, is the typescript-eslint idea worth it if we can't go fast? As a deno user, I can already work up node to call eslint w/ the ts parser. I don't need a solve from deno for that. I want deno for goin fast--i can use node if i'm ok goin slow.

I'd vote to keep deno developers' precious time (your precious time!) to be reserved for doing the rad stuff, vs shimming in the less rad stuff (wiring in a JS parser runnin in v8 :) ).

cdaringe avatar Oct 19 '21 02:10 cdaringe

+1 on having a js api available. I'm currently using eslint-plugin-rulesdir for custom rules, which essentially makes a folder available for custom rules that you can put your own scripts in. When writing fully fledged plugins I can see the benefits of using rust over js. But if you want to add just a single custom rule for a specific project/repository, the setup needed when using rust wouldn't be worth it for me personally.

jespertheend avatar Dec 22 '21 20:12 jespertheend

@jespertheend the trouble is, we got a bespoke API ready for plugins, but that API is completely different to ESLint's API. Different as in, the AST is different and methods for visiting AST nodes are different. It is still unclear if we want to do that, instead of adding ESLint compatible API that would allow to use existing ESLint plugins in Deno. @dsherret and yours truly are gonna look into PoC for ESLint compatibility in Q1 2022.

bartlomieju avatar Dec 22 '21 21:12 bartlomieju

Just a suggestion for dogfooding this... perhaps leverage mdn/browser-compat-data to add a linting rule to check that Deno code can be deployed to Deno Deploy? You'd need to add a Deploy target to the browser compat JSON but that might have other uses as well.

MarkBennett avatar Aug 30 '22 20:08 MarkBennett

@jespertheend the trouble is, we got a bespoke API ready for plugins, but that API is completely different to ESLint's API. Different as in, the AST is different and methods for visiting AST nodes are different. It is still unclear if we want to do that, instead of adding ESLint compatible API that would allow to use existing ESLint plugins in Deno. @dsherret and yours truly are gonna look into PoC for ESLint compatibility in Q1 2022.

The mantainer of swc is working on a separate package that can emit ESTree AST swc-project/bindings#4

ild0tt0re avatar Sep 02 '22 07:09 ild0tt0re

Any updates on this? For now waiting for ESTree compatibility doesn't seem feasible to me (unless I've overlooked a finished implementation): https://github.com/swc-project/bindings/pull/24. What do you think about exposing an incompatible (low effort from your side) api for now, just to enable the community to recreate plugins if they want to? Personally I wouldn't mind doing it in Rust (compiled to WASM I guess) either, I just want to have the option and have it go fast :D this is what's stopping our project from adopting the linter. Thanks for any feedback!

marcesengel avatar Jun 11 '23 07:06 marcesengel