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

New lint to not use {} in generic #1968

Open
donyunardi opened this issue May 23, 2023 · 3 comments
Open

New lint to not use {} in generic #1968

donyunardi opened this issue May 23, 2023 · 3 comments
Labels
feature a feature request or enhancement

Comments

@donyunardi
Copy link

donyunardi commented May 23, 2023

Based on Hadley's suggestion:
https://github.com/hadley/adv-r/blob/dc49c3872c3530ac08716fd4f4c235b01266a4ce/S4.Rmd#L345-L352

It is bad practice to use {} in the generic as it triggers a special case that is more expensive, and generally best avoided.

# Don't do this!
 setGeneric("myGeneric", function(x) {
 standardGeneric("myGeneric")
})

Would it be a good idea to create a lint for this rule?

Linking with:
insightsengineering/teal.code#109

@MichaelChirico
Copy link
Collaborator

MichaelChirico commented May 23, 2023

SGTM. It will be part of brace_linter(). Note that there are two things:

  • Throw a lint if { is used in the def= (2nd positional) argument of setGeneric, either \(...) { ... } or function(...) { ... }
  • Don't throw a lint if { is excluded from a multi-line function in the def= argument of setGeneric

The latter can come up for long definitions like

setGeneric(
  "myLongGeneric",
  function(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p)
    standardGeneric("myLongGeneric")
)

where bumping the line is used to avoid line_length_linter() hits.

@MichaelChirico
Copy link
Collaborator

MichaelChirico commented May 23, 2023

Hmm actually in the multi-line case, {styler} will auto-include the { as well. So might need a companion issue, WDYT @lorenzwalthert?

cc also @IndrajeetPatil as our designated liaison :)

@MichaelChirico
Copy link
Collaborator

Linking {styler} tracker: r-lib/styler#1141

@MichaelChirico MichaelChirico added the feature a feature request or enhancement label Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants