lintr
lintr copied to clipboard
Error when linting fails
Two ideas here:
- Instead of quiting R, error — this will have the same impact when running R non-interactively (e.g. in GHA), but is less intrusive when working interactively.
- Default to erroring when linting "fails"
Fixes #1126
If this seems like a worthwhile approach I'll update the docs, news, etc.
Hmm I like the idea of stopping with an error in non-interactive mode, but for interactive mode, I don't see the benefit over what print.lints()
has to offer - e.g. opening the source markers pane with a list of lints.
Also, before making this breaking change, we should probably check what on-the-fly linting plugins do and expect in terms of output.
@AshesITR this doesn't change the user experience of the existing interactive method. The source markers pane still opens, and the only difference is that when you change back to the console, you'll see more clearly that linting was not successful.
I suspect this will have relatively limited downstream implications because most lintr usage is interactive. It may lead to more GitHub lintr workflows failing, but I suspect most folks would be surprised that they weren't failing before.
Ooooh I mis-read the very long if block; let me put the error in the right place.
I'm more worried about what RLanguageServer & co. do when R signals an error. Not using these plugins so I can't test unfortunately.
@renkun-ken IIRC you were using vscode-r, right?
Do you have an idea what RLanguageServer would do if lintr starts stop()
ing if it finds a lint?
Agree that q()
is the wrong behavior, but have my doubts about the changed default.
It's not uncommon for me to do something like
lint_dir(dir, linter())
# flood of lints
# oh, darn, I forgot to assign that
lints <- .Last.value
# analyze lints with as.data.frame() etc
My main concern is erroring by default makes this workflow more onerous.
At first I thought it would make this workflow impossible, but .Last.value
does still capture the right output of lint_dir()
, i.e. a "lints"
-classed object. But it will keep throwing an error every time we try and look at it.
Is there another example you can think of where a print()
method throws an error by default?
It'd feel weird for print()
to error, but maybe that's the least worst solution?
Another option is to have a specific last_lint()
helper.
last_lints()
seems like a nice addition regardless of what we do here.