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

Suggestion: ignore linters due to imported functions #191

Closed
ThierryO opened this issue Dec 28, 2016 · 7 comments
Closed

Suggestion: ignore linters due to imported functions #191

ThierryO opened this issue Dec 28, 2016 · 7 comments

Comments

@ThierryO
Copy link

Since there is no common coding style in R, importing functions from other packages often results in a clash of coding styles. Currently we have to #nolint the offending functions, which is a bit tedious. All imported functions are listed in NAMESPACE so that would be nice place to look for the exceptions.

@jimhester
Copy link
Member

An alternative is to use :: or ::: to refer to functions they will automatically excluded from the object linters. This style is the recommended one in Hadley's R Packages book.

Excluding functions imported into the package NAMESPACE is possible of course, but lintr currently works on a file a time and does not handle package structure at all, so it would require some changes in how things work.

@fangly
Copy link
Contributor

fangly commented Jan 11, 2017

@ThierryO I like your suggestion!
@jimhester It would make sense if the lint_package function automatically dealt with the NAMESPACE and somehow added imported functions to the exclude function (although exclude currently only works on entire files and lines).

@ThierryO
Copy link
Author

@jimhester Note that in the second paragraph Hadley writes:

If you are using functions repeatedly, you can avoid :: by importing the function with @importFrom pgk fun.

Would you accept a pull request which takes into account the imports from NAMESPACE? I'm willing to try implementing that.

@jimhester
Copy link
Member

I think what @fangly Suggested makes sense, if the lint_package() parses the NAMESPACE file once they can then be added to the exclusion objects.

@fangly
Copy link
Contributor

fangly commented Jan 13, 2017

Come to think of it, if that's of the objects_linter.R (name length, camel case, etc) that we are talking about, there may be a better strategy. I do also get a lot of lints from function calls, dataframe accesses, etc that I did not create.
It seems like it would be preferable to lint only objects that are assigned, not just merely used or called. To illustrate this, in this example, we would only check the names of f, a, b and x:
f <- function(a, b) {x <- 1}
Likely, the "make_object_linter" function could be adapted for this purpose.

@fangly
Copy link
Contributor

fangly commented Jan 17, 2017

I updated my object_name_linter() to only lint assigned object, as described in my previous comment.

@fangly
Copy link
Contributor

fangly commented Mar 13, 2017

Fixed in #214

@fangly fangly closed this as completed Mar 13, 2017
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

3 participants