Unnecessary brackets inside substitute expressions

[kpagacz]

This PR https://github.com/r-lib/styler/pull/876 supposedly fixes the issue of styler putting additional, unnecessary brackets inside substitute expressions. But having installed the latest version of styler from main, I am still experiencing the issue described in https://github.com/r-lib/styler/issues/873

Here is my reprex:

library(magrittr)
expr <- substitute(
  expr = {
    iris %>%
      exprA %>%
      exprB
  },
  env = list(exprA = call("summary"), exprB = call("print"))
)

Styler styles it to:

library(magrittr)
expr <- substitute(
  expr = {
    iris %>%
      exprA() %>%
      exprB()
  },
  env = list(exprA = call("summary"), exprB = call("print"))
)

Here's my sessionInfo:

[lorenzwalthert]

Thanks @kpagacz. Indeed it was fixed in #876, but only for code where the expression of concern was a direct child of substitute(), also see the unit test added there. You are right, we should fix it in general, which will probably require to set an attribute of the nest of substitute() and propagate it downwards. Fortunately, this seems to work for rules that change tokens, beacuse they use pre_visit(), i.e. go from outside inwards (i.e. style substitute(...), then its content. Not sure this also works for post_visit(). I can't fix it immediately, so I advise you to be just turn {styler} off for that section. This is also the safest option since even once we fix the problem and release a new version, other people using your code might still use an old version of {styler} and break the code.

library(magrittr)
expr <- substitute(
  expr = {
  # styler: off
    iris %>%
      exprA %>%
      exprB
  },
  # styler: on
  env = list(exprA = call("summary"), exprB = call("print"))
)

[lorenzwalthert]

Actually this does not work either because of another bug 😮‍💨... I ~filed~ closed https://github.com/r-lib/styler/issues/890. Should work with styler dev now.

[lorenzwalthert]

@kpagacz with #891, you should now at least be able to turn styler off for that sequence.

[kpagacz]

Having had a look at the unit tests and the styler code, I think I now understand why #876 does not work as I suspected.

Thanks a lot for the feedback. I actually filed this issue because styler: off and styler: on threw errors for me when I tried using it with substitute, so I felt like I was trapped to suffer manually reverting styler changes for every substitute occurrence.

I have tested on the latest main branch and I can now use # styler: off to turn styler off for particular lines as well as whole expressions in substitute. This is good enough for me and my team I think.