lintr icon indicating copy to clipboard operation
lintr copied to clipboard

brace_lintr: exception for "detached" braces?

Open MichaelChirico opened this issue 2 years ago • 3 comments

Sometimes it's useful in scripts to use a "detached" brace to group a bunch of expressions together so they all execute together, and can be collapsed together, e.g.:

{
  do_a()
  do_b()
}

Currently brace_linter() doesn't allow this because the opening { is alone on a line.

Should this be another exception?

MichaelChirico avatar Jun 30 '22 18:06 MichaelChirico

Should be optional, or at least constrained to top-level if it is. Otherwise, no more lints would occur, right?

AshesITR avatar Jun 30 '22 18:06 AshesITR

Not really -- intended logic is to make sure { is attached to function(), while (), repeat, if (), else, and a few other cases.

Constraining to the top level is probably the right approach, anyway. All of the other cases I'm seeing should really be local({ ... }), not { ... }.

So how about:

  • Allow { ... } at the top level by default when { is alone on a line
  • Disallow { ... } elsewhere, maybe adding logic to the message that mentions local({ ... }) if "detached" { is found elsewhere

MichaelChirico avatar Jun 30 '22 19:06 MichaelChirico

In particular, the top-level { ... } usage is not a violation of the style guide as I read it:

https://style.tidyverse.org/syntax.html#control-flow

{ should be the last character on the line. Related code (e.g., an if clause, a function declaration, a trailing comma, …) must be on the same line as the opening brace.

In this case, there is no "related code", so { is only constrained to be the last character on the line.

MichaelChirico avatar Jun 30 '22 19:06 MichaelChirico