Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflicting lintr rules with alist and explicitly missing arguments #540

Closed
richfitz opened this issue Oct 23, 2020 · 6 comments · Fixed by #1643
Closed

Conflicting lintr rules with alist and explicitly missing arguments #540

richfitz opened this issue Oct 23, 2020 · 6 comments · Fixed by #1643
Labels
false-positive code that shouldn't lint, but does

Comments

@richfitz
Copy link

This is pretty niche but shows up a conflicting pair of rules:

> lintr::lint("alist(a =)\n")
<text>:1:9: style: Put spaces around all infix operators.
alist(a =)
        ^~
> lintr::lint("alist(a = )\n")
<text>:1:10: style: Do not place spaces around code in parentheses or square brackets.
alist(a = )
         ^
> lintr::lint("alist(a =, b = NULL)\n")
<text>:1:9: style: Put spaces around all infix operators.
alist(a =, b = NULL)
        ^~
> lintr::lint("alist(a = , b = NULL)\n")
<text>:1:11: style: Commas should never have a space before.
alist(a = , b = NULL)

So for arguments that are "explicitly missing" there is no way of satisfying the default set of linter rules - we must and cannot have a space following the =, for cases that are within or at the end of an argument list.

The above behaviour is triggered in both the cran version (2.0.1) and current github (2.0.1.9000, eece3c1)

@AshesITR
Copy link
Collaborator

This is related to #532

@richfitz
Copy link
Author

Ah, I'd looked though for issues about commas but managed to miss that!

@AshesITR
Copy link
Collaborator

Your examples show that the commas_linter must also be fixed, not only the spaces_inside_linter so thanks 👍

@MichaelChirico
Copy link
Collaborator

Is this in fact the same as #532?

@AshesITR
Copy link
Collaborator

#532 is a special case of this. Closing #532

@MichaelChirico
Copy link
Collaborator

This is basically the same as #708 -- there, we now allow your second usage, but not your first:

lintr::lint("switch(a = , b = 2)\n") # doesn't lint
lintr::lint("switch(a =, b = 2)\n")
# <text>:1:10: style: [infix_spaces_linter] Put spaces around all infix operators.
# switch(a =, b = 2)
#          ^

This aligns with {styler}:

styler::style_text("switch(a = , b = 2)\nswitch(a =, b = 2)\nalist(a = )\nalist(a =)")
# switch(a = ,
#   b = 2
# )
# switch(a = ,
#   b = 2
# )
# alist(a = )
# alist(a = )

@MichaelChirico MichaelChirico added false-positive code that shouldn't lint, but does and removed bug an unexpected problem or unintended behavior labels Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false-positive code that shouldn't lint, but does
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants