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

lint_package('lintr') #47

Closed
ttriche opened this issue Feb 4, 2015 · 3 comments
Closed

lint_package('lintr') #47

ttriche opened this issue Feb 4, 2015 · 3 comments

Comments

@ttriche
Copy link
Contributor

ttriche commented Feb 4, 2015

So since I haven't the foggiest idea what I'm doing (at the moment I'm trying to enforce camelCase instead of underscores_between_variable_name_bits, blah blah), I figured I'd start by linting lintr.

library(lintr)
lintrCheckResults <- lint_package('lintr')
## ...time passes...
## There were 50 or more warnings (use warnings() to see the first 50)
library(dplyr)
warnings() %>% unique
## Warning messages:
## 1: In source_file$parse : partial match of 'parse' to 'parsed_content'
lintrCheckResults %>% tail(5) %>% head(2)
## [[1]]
## ./tests/testthat/test-rstudio_markers.R:74:26: style: Variable and function names should be all lowercase.
##    expect_equal(marker3$basePath, "test"),
##                         ^~~~~~~~
##
## [[2]]
## ./tests/testthat/test-rstudio_markers.R:76:1: style: lines should not be more than 80 characters.
##    expect_equal(marker3$markers[[1]]$file, file.path("test", lint3[[1]]$filename)),
## ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
## ...

So some thoughts (which will turn into a pull request momentarily):

  1. lintr could stand to pass its own linting (modulo bad.R, which should probably be in inst/example)
  2. it would be cool if results of linting a package could be organized in a hierarchy, like the package.
@jimhester
Copy link
Member

Tim,

First I agree with you about lintr passing its own linting. One of the reasons you are getting more lints than I would be is I have my default line length linter set to 120 rather than 80 characters. I am currently in the process of supporting per-project lint settings, which will clear up that issue. However there are still other lints which I need to fix in lintr.

However these warnings you mentioned are disturbing to me. I have options(warnPartialMatchings = TRUE) and I do not get this. Are you using the CRAN version of linter or the latest development version? Running devtools::session_info() will tell us exactly what commit you are using.

@ttriche
Copy link
Contributor Author

ttriche commented Feb 6, 2015

after a bit of futzing, and reading your note regarding 80 vs. 120 char lines, I'm reasonably confident that the non-line-length complaints are due to things (like .onLoad) that are baked into R. That said, here's my session info, and the pull request will be coming shortly (mostly just a bit of tidying and addition of names.lints / split.lints to ease exploration of output from lint_package).

Session info -------------------------------------------------------------------
 setting  value                       
 version  R version 3.1.2 (2014-10-31)
 system   x86_64, linux-gnu           
 ui       X11                         
 language en_US                       
 collate  en_US.UTF-8                 
 tz       <NA>                        

Packages -----------------------------------------------------------------------
 package       * version    date       source                           
 assertthat    * 0.1        2013-12-06 CRAN (R 3.1.2)                   
 bigrquery       0.1        2015-01-03 Github (hadley/bigrquery@51069ea)
 BiocInstaller   1.16.1     2015-01-02 Bioconductor                     
 codetools     * 0.2-10     2015-01-17 CRAN (R 3.1.2)                   
 crayon        * 1.1.0      2014-10-15 CRAN (R 3.1.2)                   
 DBI           * 0.3.1      2014-09-24 CRAN (R 3.1.2)                   
 devtools      * 1.7.0      2015-01-17 CRAN (R 3.1.2)                   
 dplyr           0.4.1      2015-01-14 CRAN (R 3.1.2)                   
 gtools          3.4.1      2014-05-28 CRAN (R 3.1.2)                   
 httr          * 0.6.1      2015-01-01 CRAN (R 3.1.2)                   
 igraph        * 0.7.1      2014-04-22 CRAN (R 3.1.2)                   
 lazyeval      * 0.1.10     2015-01-02 CRAN (R 3.1.2)                   
 lintr           0.2.1.9000 2015-02-05 local                            
 magrittr      * 1.5        2014-11-22 CRAN (R 3.1.2)                   
 Rcpp          * 0.11.4     2015-01-24 CRAN (R 3.1.2)                   
 rex           * 0.2.0      2014-11-26 CRAN (R 3.1.2)                   
 rstudioapi    * 0.2        2014-12-31 CRAN (R 3.1.2)                   
 stringdist    * 0.9.0      2015-01-10 CRAN (R 3.1.2)                   
 stringr       * 0.6.2      2012-12-06 CRAN (R 3.1.2)                   

@jimhester
Copy link
Member

Also Tim in terms of enforcing camelCase instead of snake_case the easiest way to do it is with with_defaults(). You turn off a given linter by using a value of NULL for it.

lint_package(".", 
  linters = with_defaults(
    object_camel_case_linter = NULL,
    object_snake_case_linter
  )
)

Or to also only lint for lines over 120

lint_package(".", 
  linters = with_defaults(
    object_camel_case_linter = NULL,
    object_snake_case_linter,
    line_length_linter(120)
  )
)

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