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

Extensibility/modular definitions of new rules #85

Open
chriskrycho opened this issue Jan 23, 2021 · 8 comments
Open

Extensibility/modular definitions of new rules #85

chriskrycho opened this issue Jan 23, 2021 · 8 comments
Labels
C-discussion Category: discussion E-Hard Fixing/implementing this issue is hard and likely not fit for a new contributor

Comments

@chriskrycho
Copy link

Hello, excellent project!

What plans (if any) do you have so far for allowing end users to define their own sets of rules? One of the key "wins" provided by ESLint is that users can define their own rules and integrate them simply by providing them at a conventional path and then referring to them with ESLint's config. We use this extensively at LinkedIn to provide app-specific guidance, for rules specific to that project—whether due to technical limitations or simply to keep things consistent.

rslint (appropriately, in my view) is taking a different approach to configuration than ESLint does, and it will by necessity take a different approach to extensibility since the mechanics for doing this kind of thing with a compiled language are very different than they are for an interpreted language like JS/Node.

Obvious candidates:

  • Don't let users supply their own definitions, other than by forking rslint. (I believe this represents the current state of affairs?) This is the simplest, but also the least flexible for users.

  • Define an API for consumers to be able to supply dynamic libraries as “plugins”, which can be looked up from a conventional location and executed at runtime. This would have the best combination of performance and dynamic lookup, and would also most closely align… but would also almost certainly run into issues in short order due to Rust's lack of ABI stability. It would also require end users to actually do that build and/or distribute the binaries they build in some way.

  • Define an API/blueprint for consumers to be able run as separate binaries, the same way that binaries can work alongside something like the way cargo, git, etc. do this. This would probably require some kind of IPC with serialization (JSON?). It would also require end users to actually do that build and/or distribute the binaries they build in some way.

  • Follow SWC’s lead and make this consumable via WASM, which would (if carefully implemented!) still likely provide an order-of-magnitude speedup. I suspect this is the easiest story for extensibility from the perspective of end users… but also would presumably have the worst performance and the most implementation challenges on rslint’s end.

Thanks!

@RDambrosio016
Copy link
Collaborator

This is one of the main issues i have been constantly thinking about, to truly match eslint, custom rules are a must. However, as you stated, it's a huge challenge for a compiled language. These are the ideas i have thought of before:

  • we use Box<dyn CstRule> basically everywhere, in the config, in the core, in the rule store, etc. we can't make trait objects at runtime soundly because the trait object layout is unstable, so we need to make a new structure representing an abstract rule, maybe an enum of some sort.

  • embedding node will never ever work, this can be 100% ruled out.

  • wasm is really hard to make work because:

    • i need each rule to tell me its config stuff, idk how to do this, maybe it could be possible to just give it a json value and let it
      do its thing.
    • i would probably have to copy the entire tree and give it to the memory for the wasm vm (wasmtime probably), which is
      memory intensive, and our rowan tree is a bit weird, its not as simple as a json serializable AST. This could prob easily work if
      the thing compiling to wasm is rust.
    • how does package management work? do we just tell users to give links/paths to wasm files?
  • embedding js using the deno runtime seems possible, deno lint has done it, however, deno lint is a lot less complex and large than rslint, also, they use swc's ast which is easily json-serializeable. We could also just transform the rslint ast to swc's ast and use swc's existing js tools like an AST visitor. But this makes plugin style rules impossible. however, v8 handles are neither Send nor Sync, so we would probably need channels of some sort, more awkwardness.

The main issue that needs to be solved is how do we coordinate with external rules if we rely so heavily on trait objects? and how do we do this fast without compromising the speed we already operate at.

@RDambrosio016 RDambrosio016 added C-discussion Category: discussion E-Hard Fixing/implementing this issue is hard and likely not fit for a new contributor labels Jan 24, 2021
@xacrimon
Copy link
Collaborator

xacrimon commented Jan 24, 2021

So, without WASM you're not ever going to get to the fast native performance of Rust. However, what can be done using including LuaJIT and let users write Lua rules. I think you could have this happen by creating a struct called DynamicCstRule, implementing CstRule for it and have that contain a lua object reference. Users would write a script that exports an object with a standardized set of methods similar to the ones on the CstRule trait. You'd have to supply make a dynamic wrapper around the RuleCtx struct, implementing methods which the Lua code can call to add diagnostics etc.

LuaJIT will at the end of the day give you pretty much the best performance you can get without meddling with AOT compilation to WASM or native and Lua is a pretty nice language for writing short scripts such as rules in. Due to Lua's thread restrictions you'd have to create one instance per thread of this rule object.

I think that's about how good it's going to get sadly. Without using WASM of course.

@Kixiron
Copy link
Contributor

Kixiron commented Jan 25, 2021

Lua and wasm will have about the same complexity to implement, so I don't really see a point in using lua since it just makes targeting it harder

@RDambrosio016
Copy link
Collaborator

The hard part isnt really what language, its how to move away from trait objects and towards rule objects which can accompany things like wasm rules

@Kixiron
Copy link
Contributor

Kixiron commented Jan 25, 2021

Like xacrimon said, the best thing is to have the underlying rule (including the loaded wasm module) itself implement CstRule

@RDambrosio016
Copy link
Collaborator

That doesnt work because i rely on a single CstRule instance representing one rule, therefore we need to make a new object which is an enum which can hold a handle to a wasm rule/lua rule/rust rule, etc

@Kixiron
Copy link
Contributor

Kixiron commented Jan 25, 2021

A single instance can still represent one rule, you change what functions each calls

@RDambrosio016
Copy link
Collaborator

i think we've misunderstood eachother, that's what my plan is, each wasm object will be something like

struct WasmRule {
  name: String,
  function: Func,
  ...
}

Then when we instantiate each plugin we just ask it what each rule is and what their functions are

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: discussion E-Hard Fixing/implementing this issue is hard and likely not fit for a new contributor
Projects
None yet
Development

No branches or pull requests

4 participants