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

find variables which reuse names from functions or variables in base or recommended packages #376

Closed
jackwasey opened this issue Mar 2, 2019 · 5 comments
Labels
feature a feature request or enhancement google-linters new-linter

Comments

@jackwasey
Copy link

jackwasey commented Mar 2, 2019

Although the code in R itself doesn't follow its own recommendations, it would be nice to lint for times when a variable is defined which overrides the name of something in a base or recommended package. I imagine it would be easy to string match against the contents of those namespaces.

E.g., I would love a lints for:

version <- "version is a 'simple.list' in the base package"
names <- c("alice", "bob") # oh no, names is a function in base

This can lead to problems, e.g. when editing the variable declaration, but not updating subsequent uses, since the symbol will still be found, but will often be for a closure, giving opaque errors.

This would apply to both variables defined within functions, and function arguments, so the following would also be good to lint:

my_fun <- function(data.frame = cars) head(data.frame)
@jimhester
Copy link
Member

This is a good suggestion, thanks

@jimhester jimhester added the feature a feature request or enhancement label Sep 23, 2019
@MichaelChirico
Copy link
Collaborator

I wrote something similar to this, the issue is there are several thousand names to check which would make the linter relatively heavy

@AshesITR
Copy link
Collaborator

Maybe reusing logic from the object_usage_linter and checking for collisions of all of the found symbols within the base & recommended packages would be a lightweight way to implement this.

That way we wouldn't need a list of all bad object names, but instead dynamically check the (presumably fewer) used symbols against a bunch of loaded namespaces.

@MichaelChirico
Copy link
Collaborator

See #2307. It's not as slow as you might expect (though noticeably slower than the fastest+simplest linters)

@MichaelChirico
Copy link
Collaborator

Marking this as closed by #2307. please file any residual FR as a follow-up

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 google-linters new-linter
Projects
None yet
Development

No branches or pull requests

4 participants