I'm looking into using ast-grep fo linting ReScript. What rules should I investigate?

21

Amazing! Do you think that can that somehow be integrated in the editor-tooling?

22

Probably? I think the first step is getting a separate NPM package published that can handle linting with the existing ast-grep plugin and from the command line, then getting a handful of rules into it that are useful.

1 Like
23

As long as there’s some way to produce error mesages with locations, integrating into the editor tooling should be doable.

1 Like
24

The existing ast-grep plugin does that and works with what we have so far for ReScript.

1 Like
25

I spent last night integrating it into flychecker for emacs, so if we prefer to integrate it into the main tooling, I can confirm that ast-grep scan --report-style short <source-file> gives usable plaintext output.

2 Likes
26

I managed to get it working! Now I have a portable package that can init itself based on a toplevel config, and it sets the vscode extension config to use the ast-grep config in the node_modules linter folder!

Any objections to me publishing it as rescript-lint? If something better comes along I have no issues surrendering the package.

2 Likes
27

I just need to get it all running from node_modules now. I have the vscode extension showing warnings, but the CLI isn’t running. This is fun lol.

28

I have it running on Node 20, but I’m struggling with Node 22.

This works on node 20:

sg scan --config "./node_modules/rescript-lint/sgconfig.yml" src

warning[no-console]: Unexpected console statement.
  ┌─ src/index.res:2:3
  │
2 │   Console.log("foo!")
  │   ^^^^^^^
  │
  = Remove the console statement.

But on node 22:

sg: group 'scan' does not exist
1 Like
29

Of note: as far as I can tell, there’s not a way to track individual variables with ast-grep, so I don’t think we’d be able to use it for, say, checking dependency arrays. I might play around with using tree-sitter directly.

30

Still looking for ideas?

In our codebase we sometimes make use of Obj.magic(x), typically when dealing with JSON or js interop in an area we don’t care that much to do strict decoding

Sometimes though the type resolves to something unexpected and then the error is hard to trace.

A useful lint rule would be to require obj.magic to always specify the type you are expecting to cast it to

let x = Obj.magic(1) // lint error!

let x: int = Obj.magic(1) // Ok!
let x = (Obj.magic(1) : int) // Ok!
2 Likes
31

hi, I think the issue comes from node22’s host image, say it is a different linux container. So the sg command resolves to a builtin linux command. See Command name alias: ast-grep and sg · Issue #56 · ast-grep/ast-grep · GitHub

Changing the command to ast-grep should solve the issue.

32

I’d love to help get this over the line, though it seems it’s kind of stalled?

1 Like
33

I was able to get ast-grep working, but not in a way that was portable. If you install it into your project and set up rescript tree sitter you can write linting rules.

1 Like
34

Nice, thanks. Yeah I see that ast-grep’s dynamic library binaries make this quite unpleasant to set up through npm. So close yet so far!

35

There is @ast-grep/cli npm package, so I think the only obstacle to a portable linter is the rescript grammar DLL/SO file, right? I’m sure we could compile one for each platform, as part of the tree-sitter-rescript repo’s github actions? @aspeddro sorry to ping you personally but I think you were the most recent contributor to that repo!

36

The sgconfig.yml handles platform-specific parsers straightforwardly:

customLanguages:
    rescript:
        libraryPath: # paths to dynamic library
            aarch64-apple-darwin: ./parsers/mac.so
            x86_64-unknown-linux-gnu: ./parsers/linux.so

I have working Linux and Mac parser files locally, so I think we now have everything we need. It would now be a question of designing the DX/UX of the linter. How should it fit into the development workflow, and most importantly, how should rules work? Getting into some highly complex extensibility territory like eslint seems unattractive…

@zth do you have thoughts on any of this?

37

Ast-grep has a good vscode plugin, and writing rules is fairly easy.
The ones I had setup are:

  • no-console, this is a good one because I forget.
  • No Js., because it made it easier to migrate a project.
  • no className, I am using Preact instead of React so I can just use class

These all all just matching text for the most part, but I would be down to try and create something more complicated. I know the React rules of hooks would be good, but i think it’s a bit tricky to create and I stopped using hooks so It wasn’t worth my investment at the time.

If we can’t make it portable, we could at least make a generator cli that sets it all up and installs the required deps and some files for rules.

1 Like
38

I don’t see any reason this couldn’t be made portable, and even a core part of the rescript experience.

Just read this on the ast-grep docs too, so extensibility is actually already a solved problem.

Pro tip

We can also use directories in node_modules to reuse preconfigured rules published on npm!

More broadly speaking, any git hosted projects can be imported as rule sets by using git submodule.