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

Add buf linter and fixer #4128

Merged
merged 6 commits into from
Apr 6, 2022
Merged

Conversation

amckinney
Copy link
Contributor

This adds the buf lint command as a linter and the buf format command as a fixer. The buf lint linter was previously published from our own vim-buf repository here, whereas the buf format fixer is completely new. We'd like both of them to exist in ale now that buf has a stable v1 release.

I've included a description and test for each (pattern matching from what previously exists, e.g. clang-format, dartfmt, and gofmt). I've also verified that both work locally by pointing vim-plug to my local copy of ale.

I've used the following .vimrc configuration to test locally against .proto files:

let g:ale_linters = {
\   'proto': ['buf-lint'],
\}
let g:ale_lint_on_text_changed = 'never'
let g:ale_linters_explicit = 1

let g:ale_fixers = {
\   'proto': ['buf-format'],
\}
let g:ale_fix_on_save = 1

test/linter/test_buf_lint.vader Outdated Show resolved Hide resolved
test/linter/test_buf_lint.vader Outdated Show resolved Hide resolved
test/fixers/test_buf_format_fixer_callback.vader Outdated Show resolved Hide resolved
@amckinney
Copy link
Contributor Author

Thanks for the reviews @w0rp and @hsanson! This should be ready for another look.

@amckinney
Copy link
Contributor Author

It looks like we're seeing a lint failure for the name of the buf-lint linter:

========================================
Running custom linting rules
========================================
Custom warnings/errors follow:

ale_linters/proto/buf_lint.vim: Use snake_case names for linters

Is there any way we can make an exception for this one? Given that we're migrating the linter from vim-buf, this would be a breaking change for anyone that removes the dependency on vim-buf and instead uses ale directly - they would need to update the following configuration:

let g:ale_linters = {
\   'proto': ['buf-lint'],
\}

Plus, I noticed that there is already a protoc-gen-lint linter in here and it appears that kebab-case is allowed for it (here). I searched around for an exception list (one that would include protoc-gen-lint), but I can't seem to find it anywhere.

@hsanson
Copy link
Contributor

hsanson commented Apr 5, 2022

@amckinney what you can do is set the name using snake case and add an alias. See "cabal_ghc" for example:

call ale#linter#Define('haskell', {
\   'name': 'cabal_ghc',
\   'aliases': ['cabal-ghc'],
\   'output_stream': 'stderr',
\   'executable': 'cabal',
\   'cwd': '%s:h',
\   'command': function('ale_linters#haskell#cabal_ghc#GetCommand'),
\   'callback': 'ale#handlers#haskell#HandleGHCFormat',
\})

@amckinney
Copy link
Contributor Author

Thanks for the tip @hsanson, I added the alias and everything works on my end.

@hsanson hsanson merged commit 607f33a into dense-analysis:master Apr 6, 2022
@amckinney amckinney deleted the amckinney/buf branch April 6, 2022 16:07
cyyever pushed a commit to cyyever/ale that referenced this pull request Jul 11, 2022
* Add buf lint to linters

* Add buf format to fixers

* Fix test/linter/test_buf_lint.vader

* Fix test/fixers/test_buf_format_fixer_callback.vader

* Simplify test/test-files/proto/testfile.proto

* Add buf-lint alias and rename linter
cyyever pushed a commit to cyyever/ale that referenced this pull request Jul 11, 2022
* Add buf lint to linters

* Add buf format to fixers

* Fix test/linter/test_buf_lint.vader

* Fix test/fixers/test_buf_format_fixer_callback.vader

* Simplify test/test-files/proto/testfile.proto

* Add buf-lint alias and rename linter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants