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

Custom lints with Lua #222

Open
Kampfkarren opened this issue Mar 27, 2021 · 10 comments
Open

Custom lints with Lua #222

Kampfkarren opened this issue Mar 27, 2021 · 10 comments
Assignees
Labels
A-bin Area: selene, the program A-plugins Area: Plugins C-enhancement Category: Feature request or improvement E-hard Call for participation: Experience needed to fix: Hard / a lot I-needs-design Issue: Needs design

Comments

@Kampfkarren
Copy link
Owner

I want to be able to write custom lints with Lua, store them in some directory, and have Selene run those. These lints would be similar to the custom JS lints ESLint supports.

Every script would represent one lint, and return a table just like the Lint trait, with information such as its severity, rule type, and most importantly a function that takes in an Ast and returns a table of diagnostics.

These should not run by default for security purposes. Somehow, you must give Selene permission to run them (the extension would just show a basic prompt), where Selene would then remember to trust that directory in a folder outside of the repository, so people cannot simply commit a "yes, trust this" file. I am not excellent at this sort of stuff, so some more work may need to be done, such as allowing different permission levels to reset if any lints change hashes.

Ideally, you might even be able to download repositories of lints from the web, meaning people can create specialized/opinionated linting suites, such as StandardJS.

I also want to allow fixers inside these custom lints as well, and ideally in some of the standard selene lints too. This is because if someone sees a thousand lints from something style related, they'll simply turn it off instead of fixing it. This may be out of scope for this issue though.

These lints would also be under a fixed namespace, such as plugin:: or similar. This is to prevent name collisions with selene lints, especially in the case of a Lua lint becoming so popular that it deserves promotion to the upstream.

This relies on Lua bindings for full-moon, something I will not start work on until 1.0.0 finalizes the types to some degree, which should be coming soon.

@Kampfkarren Kampfkarren added C-enhancement Category: Feature request or improvement A-bin Area: selene, the program I-needs-design Issue: Needs design E-hard Call for participation: Experience needed to fix: Hard / a lot labels Mar 27, 2021
@Kampfkarren
Copy link
Owner Author

We should probably take a permissions route like Deno does, limiting things like io and require'ing C libraries. @LPGhatguy considered https://github.com/bazelbuild/starlark for Remodel and Rojo configuration, I might want to look and see if that's a reasonable path for us as well, though I do think I want the ability to at least allow io interactions.

@Kampfkarren Kampfkarren pinned this issue Jun 21, 2021
@Kampfkarren
Copy link
Owner Author

full-moon keeps making breaking changes as Luau gets more and more complex, in ways that we believe are impossible to protect ourselves from ahead of time (before an RFC is made). This poses a very large problem for stability of lints.

@JohnnyMorganz suggests stabilizing selene lints with just exposing Lua 5.1 types, as we haven't been breaking backwards compatibility on those. There are some changes I want to make, but they're only on the Rust side (renaming TokenReference and such). I think that's a good idea.

In the future, I think it could be possible to package every major full-moon version (especially once it actually does hit 1.0.0), and let the user of a plugin decide what full-moon version they want. Something like either specifying it in a config, or if these are single Lua files, then something like:

return function(bikeshed, tree)
    local whatever = bikeshed.use("1.0.0").parse(tree)

...would work. I think backwards compatibility is important for plugins, as unlike bumping full-moon as a cargo package, you don't have the benefit of rustc to automatically find everything that's changed.

@Kampfkarren
Copy link
Owner Author

As for this:

We should probably take a permissions route like Deno does, limiting things like io and require'ing C libraries

...an idea I had would be, in whatever form you are specifying plugins, like:

[plugins]
my_cool_lint = { source = "plugins/my_cool_lint.lua" }

...you could specify what permissions you actually need, such as:

[plugins]
my_cool_lint = { source = "plugins/my_cool_lint.lua", permissions = ["read_files"] }

...but most notably no wildcards, so everything is laid bare. Attempting to use a function locked behind a permission, such as io.open, would throw an error, telling you what arguments it tried to use, and how to enable the permission.

@Kampfkarren
Copy link
Owner Author

Kampfkarren commented Feb 9, 2022

I think selene.toml would also be completely blocked from file writes? Otherwise you can just edit the TOML yourself. Of course, it's obvious that if there's a permission to run files, then all bets are off the table, but at least you'll see that coming when you give that permission.

Not that I'm really expecting selene to be a huge attack vector, but better safe than sorry...

@Kampfkarren
Copy link
Owner Author

For posterity, this will likely delay on a 1.0.0 full-moon release.

@Kampfkarren
Copy link
Owner Author

I'm thinking also support using GitHub repos directly, that follow some sort of structure. This would be lockfiled, with both branch/revision and a digest. Note to self: look into how Gopkg.lock does it.

[plugins]
cool_plugin_hub = { source = "github.com/Kampfkarren/cool_plugin_hub" } # Can be an entire set of lints or just one

@Kampfkarren
Copy link
Owner Author

Kampfkarren commented Sep 8, 2022

Read/write permissions should be split based on whether or not the file being interacted with is an ancestor of the folder with selene.toml.

Closely audit people using the full version. If it's stuff like cross-repo caching, we can just make a caching library, for instance.

@Kampfkarren
Copy link
Owner Author

Musings on a full-moon Lua API:

block:visit({
  FunctionArgs = function(args)
    args:match({
      TableConstructor = function(table_constructor)
        print("found table constructor call")
      end,
    })
  end,
})

@Kampfkarren
Copy link
Owner Author

Make sure VS Code extension is still able to run default lints until user explicitly opts into using plugins. Probably with/starting the same change of the selene extension being able to spit out lines of JSON that could be lints, or could be other data, to notify when a plugin tried to load/needs a new permission.

@Kampfkarren
Copy link
Owner Author

Make sure VS Code extension is still able to run default lints until user explicitly opts into using plugins. Probably with/starting the same change of the selene extension being able to spit out lines of JSON that could be lints, or could be other data, to notify when a plugin tried to load/needs a new permission.

Other place this was mentioned: #371 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-bin Area: selene, the program A-plugins Area: Plugins C-enhancement Category: Feature request or improvement E-hard Call for participation: Experience needed to fix: Hard / a lot I-needs-design Issue: Needs design
Projects
None yet
Development

No branches or pull requests

1 participant