https://pantsbuild.org/ logo
Join Slack
Powered by
Hello! I’ve been trying to create a plugin for a l...
# plugins
a
Hello! I’ve been trying to create a plugin for a linter/formatter for sql using SQLFluff. I’m able to implement both a linter and a formatter (thanks to the very helpful docs!). However when I now run 
./pants lint path/to/file.sql
both the formatter and the linter run, while I would like to only run the linter when using the 
lint
goal. Any tips on how to achieve this?
b
Does the 
lint
implementation do anything extra above formatting?
All formatters are also run in 
lint
but in a "see if the file changed" mode. If the tool also has extra ability to 
lint
, that's something we're aware of, but looking for some ideas/feedback on good UI/UX on how to best support that.
a
Sqlfluff has a 
lint
and a 
fix
command and 
lint
behaves differently than running 
fix
in this mode that you describe, so therefore my preference to separate these two.
b
Yeah that's the right thing. Then it's just a matter of figuring out the right UI/UX. Is there a reason you don't want the lint goal to run the formatting check?
(try running it with 
--no-formatters
by the way... I think that's the flag. Depending on the 
name
attribute, that may/may not work)
a
Mostly because there is no added value for the formatter to run when using 
lint
, it just takes extra processing and generates confusing output for the user
let me try that --no-formatters flag
b
FWIW in almost every case, it actually isn't adding any overhead when you do 
lint
after 
fmt
, because under-the-hood, Pants runs the formatter the exact same way in both goals (and therefore the results are memoized/cached). Just in 
fmt
we write results back, and in 
lint
we simply report.
👍 2
a
If I understand correctly this then would also skip black and docformatter for my Python code when using 
./pants lint :: --skip-formatters
b
Correct
Ohhhh I misunderstood maybe? Are you saying that the 
SQLfluff
lint
command also checks for formatting?
a
nope, but I do think we have some misunderstanding. Let me try to explain more clearly
🙏 1
sqlfluff has multiple commands, of which two are 
sqlfluff lint path/to/file.sql
and 
sqlfluff fix path/to/file.sql
I’ve now implemented two rules 
sqlfluff_lint
and 
sqlfluff_fmt
according to the documentation for adding a linter and a formatter
in both rules I basically use the same 
VenvPexProcess
but with different arguments used in 
argv
.
👍 1
but both rules are run/executed (however you call it) when using 
./pants lint …
while I would be happier with only rule 
sqlfluff_lint
. sorry lots of messages but hope this was clearer.
b
(Lots of messages is fine 😅 I do that all the time 😛) 1. So I'm still not seeing why the user wouldn't want to know when linting that there are issues that can automatically be fixed? 2. (In the future, hopefully Pants 2.14, we'll have a 
fix
goal which acts similar to 
fmt
, but is for semantic changes. It'll still be running in 
lint
, so just more of an FYI)
What you want is possible, it's just not easy so I'm trying to understand where the pain really is to see if/how to relieve it
a
good question, what is the real pain? Maybe there isn’t really a problem and it’s fine to run both. It just seems a bit superfluous is all.
it is definitely not a blocking issue, but maybe I was just hoping for an easy way to imlement this that I’m currently not aware of
anyway thanks for thinking along!
b
I think for things like this the UI can certainly be improved, but in general I think the intent is good. "Here is what happens when you run 
lint
on your code. Also there's some things that can be fixed with 
fix
." Also, the fact that we cache and re-use the results of running the process between 
fmt
and 
lint
. Means we shouldn't really be imposing much of a cost between running 
fmt
-then-
lint
, or between 
lint
-then`-fmt` IMO 
lint
really shines in CI, where we can check a bunch of stuff in parallel with one command.
h
There's prior art here: Buf for Protobufs, which also has a dedicated fmt command and lint command. Here's how we modeled it: https://github.com/pantsbuild/pants/blob/main/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py#L33-L36 https://github.com/pantsbuild/pants/blob/a3f6f04d12f8ee5863888e10e6c619b449d67828/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py#L42-L44 https://github.com/pantsbuild/pants/blob/a3f6f04d12f8ee5863888e10e6c619b449d67828/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py#L37-L39 Imo it's highly desirable to be able to run SQLFuff's fix/fmt in "check only" mode. It allows you to see in CI if the formatting is already good or not. You probably don't want to use 
fmt
in your CI
4 Views
Open in Slack
PreviousNext
Powered by