rhino
rhino copied to clipboard
Topic: Linting and formatting R
We have faced situations where changes introduced by styler are flagged by lintr, or are simply undesirable. Due to its flaws, we typically don't use styler in regular development workflow, but perhaps as a one-off at the beginning of a project, when inheriting a lot of ill-formatted legacy code.
Ideas
- Linter: Check for
...
used inbox::use()
statements (box::use(shiny[...])
). - Styler: Provide a default
paths = c("app", "tests")
argument. This could encourage users to runformat_r()
more often and give us more feedback - in what situations it's too aggressive / inconsistent withlint_r()
. See #109 for discussion. - Styler: In addition to (2) - add a prompt to confirm the styling.
@kamilzyla maintainer of {styler} here. Consistency with {lintr} is certainly a goal we are working towards and we are open to concrete reporting. Also, there are different levels of invasiveness for styler, e.g. you can only format token, indention, linebreaks or spaces, see https://styler.r-lib.org/articles/styler.html#scope-what-to-style.
@kamilzyla I'd like to add ideas for a linter rules based on my previous experiences:
- Linter: flagging
reactive
declarations inside theobservers
as warning/error. Declaringreactive
's inobservers
doesn't yield an immediate error but it can cause a serious problems (chain reactions) further down the line (unnecessary performance drops, strange errors with values etc.). It's a bad practice to do it and it would be nice if system automatically detected it - Linter: checking if functions declared in the
box::use()
are actually used in the file. If we usebox
properly and only explicitly declare functions we need, there's still a risk that we will keep something unused in thebox
even if we get rid of it in the code. This may later "invisibly" increase number of dependencies. I imagine it may be more tricky to do with loading scripts instead of packages though - (this may be very complicated to implement) Linter: if we dabble in JS in
shiny
there's a chance we communicate JS withshiny
throughShiny.setInputValue('app-moduleName-inputName', value);
. This works nicely, but it also means we create inputs in two places: R scripts and JS scripts which makes managing them harder. A rule that would detect the inputs declared in JS and check if those are actually used anywhere in R would be nice to ease the pain of managing them