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

object_usage_linter doesn't match R CMD check #941

Closed
MichaelChirico opened this issue Mar 14, 2022 · 2 comments
Closed

object_usage_linter doesn't match R CMD check #941

MichaelChirico opened this issue Mar 14, 2022 · 2 comments

Comments

@MichaelChirico
Copy link
Collaborator

As seen here: #931 (comment)

Basically our tools::checkUsage() call doesn't match that of R CMD check. In this case, it's because the default checkUsage() doesn't set skipWith=TRUE, so objects seen inside with() calls can trip the linter.

My understanding is the main goal of object_usage_linter is to help match R CMD check, so we should adjust our code to match.

But maybe we think of this as a bit more general, and could expose an option instead (e.g. match_r_check, I would set TRUE by default).

@MichaelChirico
Copy link
Collaborator Author

PS I think this linter should be tagged as package_development

@AshesITR
Copy link
Collaborator

Is there a legitimate true positive caused by skipWith = FALSE?
I reckon most with() calls are using columns from a data frame and thus always trip checkUsage() unless the used columns are declared outside of the data frame as well.

BTW:

checkUsage(fun, name = "<anonymous>", report = cat, all = FALSE, 
           suppressLocal = FALSE, suppressParamAssigns = !all,
           suppressParamUnused = !all, suppressFundefMismatch = FALSE,
           suppressLocalUnused = FALSE, suppressNoLocalFun = !all,
           skipWith = FALSE, suppressUndefined = dfltSuppressUndefined,
           suppressPartialMatchArgs = TRUE) 

suppressPartialMatchArgs = FALSE sounds interesting.
Also, suppressLocalUnused = symbols_in_glue_calls seems like the way to support glue::glue().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants