Support astgrep

[mathstuf]

---

Support ast-grep. Note that it isn't really associated with a specific language, so I made a new "generic" group for it.

[mathstuf]

Looks like to get a fixer, some more work is needed. But this is a good start at least.

[mathstuf]

astgrep is quite grumpy when it tries to get used when there's not a configuration around. Is there a way to disable it if ale_linters#generic#astgrep#GetProjectRoot comes back with an empty string?

[mathstuf]

Another thing is that I have a local hack to append generic as a filetype to inject it into all buffers:

diff --git i/autoload/ale.vim w/autoload/ale.vim
index 2f46bce5c..9f9a2048f 100644
--- i/autoload/ale.vim
+++ w/autoload/ale.vim
@@ -96,7 +96,7 @@ endfunction
 function! s:Lint(buffer, should_lint_file, timer_id) abort
     " Use the filetype from the buffer
     let l:filetype = getbufvar(a:buffer, '&filetype')
-    let l:linters = ale#linter#Get(l:filetype)
+    let l:linters = ale#linter#Get(l:filetype . '.generic')
 
     let l:ignore_config = ale#Var(a:buffer, 'linters_ignore')
     let l:disable_lsp = ale#Var(a:buffer, 'disable_lsp')

I don't imagine this is the most acceptable way to do this, but some way to have filetype-independent checkers seems useful (e.g., I can imagine spell checkers joining the ranks here; I see some special handling for cspell in ALE today?).

[mathstuf]

Updated the GetProjectRoot to accurately detect the path based on logic in other linters. It now works without complaint when no configuration file exists.

[mathstuf]

Hmm. I'm seeing a lot of ast-grep lsp instances hanging off of my nvim where it is used. Is there something I might be missing here?

[rymdbar]

There are a couple of other tools (e.g. cspell, write-good) which are more or less generic already. I have not really thought about this, but it seems like an appropriate question to ask is whether this would better be handled like them? Wouldn't it make sense to be able to use astgrep with some filetypes, but not with others?

I believe the tool ought to be added to supported-tools.md too.

[mathstuf]

Wouldn't it make sense to be able to use astgrep with some filetypes, but not with others?

I suppose it could be limited to the languages that ast-grep actually compiles in tree-sitter support for. I don't see a way to coax it from a binary though (completions doesn't list them anywhere either).

[rymdbar]

I suppose it could be limited to the languages that ast-grep actually compiles in tree-sitter support for.

My intention was to question whether adding a new "generic" group is the right design, or if keeping granular control by making ast-grep configurable per filetype as the already existing (e.g. cspell, writegood) general tools are.

I'm not saying this pull-request necessarily does it wrong. Just stating that I noticed it being done differently. Thus wishing to understand whether you have some reasoning behind adding a globally enabled tool rather than mimicking how similar things appear to have been done in the past.

If doing like with pre-existing generic tools, I envision going through the language crate once and adding a linter definition per language. I assume new languages are added infrequently enough that updating ale would be a reasonable chore.

Untested, and mainly to illustrate my thinking. A question for all interested parties: Would transforming the suggested linter into a handler, and doing something like this be desirable?

for lang in $( sed -n 's/^mod \(.*\);/\1/p' < ../ast-grep/crates/language/src/lib.rs ); do
cat > ale_linters/$lang/astgrep.vim << EOF
" Author: Ben Boeckel <redacted>
" Description: astgrep for $lang files

call ale#linter#Define('$lang', {
\   'name': 'astgrep',
\   'lsp': 'stdio',
\   'executable': function('ale#handlers#astgrep#GetExecutable'),
\   'project_root': function('ale#handlers#astgrep#GetProjectRoot'),
\   'command': function('ale#handlers#astgrep#GetCommand'),         
\})                                               
EOF 
done

From the nitpicking department, a possible confusion point is with tree-sitter. I understand ast-grep is using it internally. Thus both vim and neovim interface that tool through ale in the same way. At the same time I believe it is a well-known fact that only one out of the two editors itself uses tree-sitter, which might lead to misconceptions. If you're the master of words who is able to add a single word or two to make ast-grep's description express this subtle detail more clearly, then feel welcome to add those to the documentation. If not, please ignore this comment. The description is correct as is, and ale is targeting users who can question their own misunderstandings.

[mathstuf]

I don't know what the answer is. I don't like the idea of duplicating the block umpteen times. Maybe we make an ale#linter#astgrep#Define('$lang') call that at least shares all the other stuff?

I made the generic bit since it seemed like a way to get things that can handle "any" filetype working. If it is preferred to do per-language files, I can do that (though I'd like to add a helper API for it).

[mathstuf]

I pulled the typo fix out into #5021.

As for the change here, I figured out what needed to be done for a fixer via #5022. I also refactored out the "generic" support into a module that can enable it for a given language. This is plumbed through via the g:ale_astgrep_linter_languages list variable that creates a linter for ast-grep on startup for the listed languages.

[mathstuf]

Gah, the fixer just replaces the contents with the tool output. I'll need to investigate more.

[rymdbar]

Apologies for not replying immediately.

Implementation discussion

I don't know what the answer is.

Neither do I, but mimicing how things have been done in the past might be a relatively easy way towards a merge.

I could get the previous version of this working when patching l:linters to always contain '.generic'. This version with g:ale_astgrep_linter_languages I couldn't get to work at all. Not even with a fresh clean ~/.vim/ folder. Is it working for you?

I made the generic bit since it seemed like a way to get things that can handle "any" filetype working.

As far as I understand astgrep doesn't handle any filetype. According to List of Languages with Built-in Support its about twenty-four or so. While the ale_linters/ folder has 156 entries and there are 414 different filetypes in …vim/vim91/ftplugin/. Still I frequently edit several other filetypes which require ftplugins from other sources than vim-runtime.

If it is preferred to do per-language files, I can do that (though I'd like to add a helper API for it).

I have pushed the branch pr-4975-classic. How does that one look to you?

Unfortunately there is a mixup between sh and bash in ale, but treating them as the same should be good enough. Previously tsx didn't exist at all, so I created a new folder for it.

I don't like the idea of duplicating the block umpteen times.

The definition on the above branch is one line per filetype, since you added the helper. Not too bad, is it? Given that it saves unnecessarily searching for that astgrep-file for 95% of filetypes. It would be great with a way of applying DRY to the still missing documentation. As far as I know nothing in the code base exists to help with that.

Actual functionality

How can one use this? When merely reading I got the impression ast-grep brought in some quite impressive ast-aware code rewriting functionality, but when trying it out it seems that is not actually exposed through the LSP? At least to my eyes the LSP Server section seems to say only diagnostics and the fixer are there. That's great if it helps some people, of course.

The response to initialize is:

{
    "jsonrpc": "2.0",
    "result": {
        "capabilities": {
            "codeActionProvider": {
                "codeActionKinds": [
                    "quickfix.ast-grep",
                    "source.fixAll.ast-grep"
                ],
                "resolveProvider": true
            },
            "executeCommandProvider": {
                "commands": [
                    "ast-grep.applyAllFixes"
                ]
            },
            "hoverProvider": true,
            "textDocumentSync": 1
        },
        "serverInfo": {
            "name": "ast-grep language server"
        }
    },
    "id": 2
}

Attempting :ALECodeAction anywhere only seems to result in:

{
    "jsonrpc": "2.0",
    "result": null,
    "id": 3
}

As for the fixer config, shouldn't it actually better be interfaced using the LSP instead?

[mathstuf]

Found read_temporary_file for the fixer. That makes it work as I'd expect.

I could get the previous version of this working when patching l:linters to always contain '.generic'. This version with g:ale_astgrep_linter_languages I couldn't get to work at all. Not even with a fresh clean ~/.vim/ folder. Is it working for you?

Not that easily…the load order is not correct I think. It doesn't work if I set it in .nvimrc or a .lvimrc file. But if I call ale#astgrep#ApplyLanguages() after startup, it does work.

As far as I understand astgrep doesn't handle any filetype.

That is true…I think the generic thing is not suitable (especially for a tool that actually needs dedicated support for each language in order to properly support it).

I have pushed the branch pr-4975-classic. How does that one look to you?

Yeah, that looks good. It also avoids the awkwardness of ale_astgrep_linter_languages. I'll squash that in and remove the linter_languages cruft.

Given that it saves unnecessarily searching for that astgrep-file for 95% of filetypes.

That is a good point.

How can one use this?

You have to write some rules that you want to work on. A simple example for the CMake codebase is in this MR. Though these rules are disabled by default (severity: 'off'), This MR adds more complex rules, but ones that are always active. With the LSP, I get inline diagnostics about things ast-grep finds (with default-enabled rules at least).

As for the fixer config, shouldn't it actually better be interfaced using the LSP instead?

Ideally, yes. But AFAICT, the LSP-as-linter shows inline diagnostic notes but I don't know how to act on them. Is there some way to access "fix" behavior of linters? On that note, when a compilation generates fixits, I'd love to be able to filter that back into the same ALE workflow (almost like a build-log-based-LSP I guess?). If that could be hooked up to SARIF as well…ooh that'd be quite the pipeline.

[rymdbar]

How can one use this?

A simple example for the CMake codebase is in [this MR]

These are for whatever language CMake is written in, not the CMake language itself?

You have to write some rules that you want to work on.

I might need to get this explained to me like I'm five. Would be glad if you could walk me through it.

The ast-guide has a Lint Rule chapter, which appears to give an example for javascript. Thus I try creating this hello.js in an empty folder and run ast-grep new, pressing Enter all through the options given.

#!/usr/bin/env node

console.log("World");

Then I configure .vimrc to enable astgrep for ft=javascript and create a rules/hello-rule.yaml.

---
id: hello-rule
language: javascript
rule:
  pattern: console.log($GREET)
fix: console.log('Hello ' + $GREET)
message: Greet properly, please
severity: error
note: |
  Care should be taken to be correct when greeting someone, or something.

Running ast-grep scan then results in a seemingly successful invocation with the fix suggested:

hello.js
help[hello-rule]: 
@@ -0,3 +0,3 @@
1 1│ #!/usr/bin/env node
2 2│ 
3  │-console.log("World");
  3│+console.log('Hello ' + "World");

However I can't get the severity, message or note showing up from ast-grep scan. No matter what I set --report-style to. Presumably the rule file isn't fully correct?

Opening up hello.js with vim + ALE and saving LSP communication with :call ch_logfile(…) gives communication, but no diagnostics:

initialize, ALE → ast-grep

{
    "method": "initialize",
    "jsonrpc": "2.0",
    "id": 2,
    "params": {
        "initializationOptions": {},
        "rootUri": "file:///tmp/ale_fjMAeK",
        "capabilities": {
            "workspace": {
                "workspaceFolders": false,
                "configuration": false,
                "symbol": {
                    "dynamicRegistration": false
                },
                "applyEdit": false,
                "didChangeConfiguration": {
                    "dynamicRegistration": false
                }
            },
            "textDocument": {
                "documentSymbol": {
                    "dynamicRegistration": false,
                    "hierarchicalDocumentSymbolSupport": false
                },
                "references": {
                    "dynamicRegistration": false
                },
                "publishDiagnostics": {
                    "relatedInformation": true
                },
                "rename": {
                    "dynamicRegistration": false
                },
                "completion": {
                    "completionItem": {
                        "snippetSupport": false,
                        "commitCharactersSupport": false,
                        "preselectSupport": false,
                        "deprecatedSupport": false,
                        "documentationFormat": [
                            "plaintext",
                            "markdown"
                        ]
                    },
                    "contextSupport": false,
                    "dynamicRegistration": false
                },
                "synchronization": {
                    "didSave": true,
                    "willSaveWaitUntil": false,
                    "willSave": false,
                    "dynamicRegistration": false
                },
                "codeAction": {
                    "codeActionLiteralSupport": {
                        "codeActionKind": {
                            "valueSet": []
                        }
                    },
                    "dynamicRegistration": false
                },
                "diagnostic": {
                    "dynamicRegistration": true,
                    "relatedDocumentSupport": true
                },
                "typeDefinition": {
                    "dynamicRegistration": false
                },
                "hover": {
                    "dynamicRegistration": false,
                    "contentFormat": [
                        "plaintext",
                        "markdown"
                    ]
                },
                "implementation": {
                    "dynamicRegistration": false,
                    "linkSupport": false
                },
                "definition": {
                    "dynamicRegistration": false,
                    "linkSupport": false
                }
            }
        },
        "rootPath": "/tmp/ale_fjMAeK",
        "processId": 2628
    }
}

initialize, ALE ← ast-grep

{
    "jsonrpc": "2.0",
    "result": {
        "capabilities": {
            "codeActionProvider": {
                "codeActionKinds": [
                    "quickfix.ast-grep",
                    "source.fixAll.ast-grep"
                ],
                "resolveProvider": true
            },
            "executeCommandProvider": {
                "commands": [
                    "ast-grep.applyAllFixes"
                ]
            },
            "hoverProvider": true,
            "textDocumentSync": 1
        },
        "serverInfo": {
            "name": "ast-grep language server"
        }
    },
    "id": 2
}

initialized, ALE → ast-grep

{
    "method": "initialized",
    "jsonrpc": "2.0",
    "params": {}
}

textDocument/didOpen, ALE → ast-grep

{
    "method": "textDocument/didOpen",
    "jsonrpc": "2.0",
    "params": {
        "textDocument": {
            "uri": "file:///tmp/ale_fjMAeK/hello.js",
            "version": 2,
            "languageId": "javascript",
            "text": "#!/usr/bin/env node\n\nconsole.log(\"World\");\n"
        }
    }
}

window/logMessage, ALE ← ast-grep

{
    "jsonrpc": "2.0",
    "method": "window/logMessage",
    "params": {
        "message": "server initialized!",
        "type": 3
    }
}

window/logMessage, ALE ← ast-grep

{
    "jsonrpc": "2.0",
    "method": "window/logMessage",
    "params": {
        "message": "file opened!",
        "type": 3
    }
}

workspace/workspaceFolders, ALE ← ast-grep

{
    "jsonrpc": "2.0",
    "method": "workspace/workspaceFolders",
    "params": null,
    "id": 0
}

textDocument/hover, ALE → ast-grep

{
    "method": "textDocument/hover",
    "jsonrpc": "2.0",
    "id": 3,
    "params": {
        "textDocument": {
            "uri": "file:///tmp/ale_fjMAeK/hello.js"
        },
        "position": {
            "character": 0,
            "line": 2
        }
    }
}

textDocument/hover, ALE ← ast-grep

{
    "jsonrpc": "2.0",
    "result": null,
    "id": 3
}

window/logMessage, ALE ← ast-grep

{
    "jsonrpc": "2.0",
    "method": "window/logMessage",
    "params": {
        "message": "Get Hover Notes",
        "type": 4
    }
}

You do get diagnostics, right? Would it be easy for you to make this simple scenario with hello.js and hello-rule.yaml work?

As for the fixer config, shouldn't it actually better be interfaced using the LSP instead?

Ideally, yes. But AFAICT, the LSP-as-linter shows inline diagnostic notes but I don't know how to act on them. Is there some way to access "fix" behavior of linters?

LSP Server states:

Server capability

• publish diagnostics
• Fix diagnostic code action

My expectation is that a fix action ought to show up if running :ALECodeAction on a line with a diagnostic. Does it do that for you?

[mathstuf]

These are for whatever language CMake is written in, not the CMake language itself?

Yes. These are rules for CMake's own C++ code.

You do get diagnostics, right? Would it be easy for you to make this simple scenario with hello.js and hello-rule.yaml work?

Yes.

Alas, I'm not good at ast-grep yet. I fumbled my way through the rules CMake has to get where these are.

My expectation is that a fix action ought to show up if running :ALECodeAction on a line with a diagnostic. Does it do that for you?

I'm not seeing it :/ . :ALEFix shows ast-grep. :ALECodeAction shows "No code actions received from server".

[rymdbar]

I see the screenshot, yet wonder how it works... Could you perhaps have a non-lsp setup to use ast-grep linger in your configs? Maybe you could post your full output from :ALEInfo?

Alas, I'm not good at ast-grep yet. I fumbled my way through the rules CMake has to get where these are.

My results with CMake sources are as with javascript. Running ast-grep scan in the terminal works, but through ale it is all silent.

I brought this question to ast-grep's discussions, and then looked enough at the sources to be very surprised the lsp config appears to be working for you. Could you please have a look at my linked answer and double check on your side?

Depending on whether the lsp communication is broken or not, can be made to work, this pull request might need another path.

[mathstuf]

 Current Filetype: cpp
Available Linters: ['astgrep', 'cc', 'ccls', 'clangcheck', 'clangd', 'clangtidy', 'clazy', 'cppcheck', 'cpplint', 'cquery', 'cspell', 'flawfinder']
   Linter Aliases:
'cc' -> ['gcc', 'clang', 'g++', 'clang++']
  Enabled Linters: ['astgrep', 'cc', 'ccls', 'clangcheck', 'clangd', 'clangtidy', 'clazy', 'cppcheck', 'cpplint', 'cquery', 'cspell', 'flawfinder']
  Ignored Linters: []
 Suggested Fixers:
  'ast-grep' - Apply ast-grep rules.
  'astyle' - Fix C/C++ with astyle.
  'clang-format' - Fix C, C++, C#, CUDA, Java, JavaScript, JSON, ObjectiveC and Protobuf files with clang-format.
  'clangtidy' - Fix C/C++ and ObjectiveC files with clang-tidy.
  'remove_trailing_lines' - Remove all blank lines at the end of a file.
  'trim_whitespace' - Remove all trailing whitespace characters at the end of every line.
  'uncrustify' - Fix C, C++, C#, ObjectiveC, ObjectiveC++, D, Java, Pawn, and VALA files with uncrustify.

[rymdbar]

Update: We've seen fixes in ast-grep triggered by the linked discussion, as well as have been gifted #5044 which I haven't had the chance to look in detail at yet. I only saw that it likely needs added test cases.

[rymdbar]

Given that this PR effectively depends on #5044, maybe you'd want to transform this into a Draft until it gets unblocked @mathstuf?