Skip to content

Details for improving multiple_dots_linter() #182

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

Closed
fangly opened this issue Oct 24, 2016 · 3 comments
Closed

Details for improving multiple_dots_linter() #182

fangly opened this issue Oct 24, 2016 · 3 comments

Comments

@fangly
Copy link
Contributor

fangly commented Oct 24, 2016

Hi,

I would like to use some sort of linter against object names that contain a dot, but a few things about the multiple_dots_linter make me raise an eyebrow...

1/ Why multiple dots? I would also like to flag objects that contain a single dot.
2/ I would like to make an exception for S3 generics, so perhaps have an "allow_S3" flag?
3/ How to handle "private" functions, i.e. functions that are prefixed with a "." in some coding conventions (e.g. https://www.bioconductor.org/developers/how-to/coding-style/)? This point may be related to #136. Does ".private.fun" count as a 2-dot name? In my case, I would like to allow such private names. Perhaps it would be good to have an option "allow_prefix".

Let me know what you think of this and I may have the opportunity to work on implementing the changes needed. Cheers,

Florent

@jimhester
Copy link
Member

jimhester commented Oct 24, 2016

  1. S3 methods contain a single dot and should be allowed.
  2. It is not possible to determine if a given function is an S3 method without loading all possible S3 generics.
  3. This seems reasonable, the current linters implement hadley's style guide and he does not use prefixed . for internal functions, relying instead on the NAMESPACE file. prefixed . convention mostly predates introduction of package namespaces in R. I think only counting internal . should work unconditionally though.

@fangly
Copy link
Contributor Author

fangly commented Oct 25, 2016

Great, thanks Jim!
Regarding 1/ & 2/, it seems like the "ftype()" function from the pryr package would discriminate single-dotted objects:

> ftype(t.test)
[1] "s3"      "generic"
> ftype(data.frame)
[1] "function"

Is that not an option?

@jimhester
Copy link
Member

The problem is not figuring out if a function is a S3 generic, you just search for a UseMethod call, which is what pryr:::is_s3_generic() does, but you cannot determine if a function is an S3 method without searching for generics in the calling environment, which for a package would require loading the package and for scripts would require loading all of the library(), require() calls etc.

It is certainly possible to do if you require running all the prior code in a file or package before linting, but that is not a road I want to go down.

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