Skip to content

conflict between lintr and styler about curly brackets #438

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
IndrajeetPatil opened this issue Nov 12, 2018 · 8 comments
Closed

conflict between lintr and styler about curly brackets #438

IndrajeetPatil opened this issue Nov 12, 2018 · 8 comments

Comments

@IndrajeetPatil
Copy link
Collaborator

If I have a code like the following-

  iris %>% 
  {if (x != 2) {
    dplyr::select(.data = ., -Species) %>%
      tibble::as.tibble(.)
  } else {
    tibble::as.tibble(.)
  }}

And style it using styler, it changes to-

  iris %>%
  {
    if (x != 2) {
      dplyr::select(.data = ., -Species) %>%
        tibble::as.tibble(.)
    } else {
      tibble::as.tibble(.)
    }
  }

But this produces the following marker from lintr:

Opening curly braces should never go on their own line and should always be followed by a new line.
@lorenzwalthert
Copy link
Collaborator

Thanks. I can't reproduce it on Windows 10 with the CRAN version of styler (1.0.2) using style_text or the Addin. Which styler version are you using?

@IndrajeetPatil
Copy link
Collaborator Author

Here is a live demo of this:

styler

The lines of code in question are the following:
https://github.com/IndrajeetPatil/ggstatsplot/blob/615242696d72f25da96659796cc6c72388af3409/R/ggscatterstats.R#L237-L248

Here is my session info. As you can see, I am working with development version of both styler and R:

options(width = 200)
devtools::session_info(include_base = TRUE)
#> - Session info ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
#>  setting  value                                             
#>  version  R Under development (unstable) (2018-10-20 r75474)
#>  os       Windows 10 x64                                    
#>  system   x86_64, mingw32                                   
#>  ui       RTerm                                             
#>  language (EN)                                              
#>  collate  English_United States.1252                        
#>  ctype    English_United States.1252                        
#>  tz       America/New_York                                  
#>  date     2018-11-12                                        
#> 
#> - Packages -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
#>  ! package     * version    date       lib source                            
#>    assertthat    0.2.0      2017-04-11 [1] CRAN (R 3.5.1)                    
#>    backports     1.1.2      2017-12-13 [1] CRAN (R 3.5.0)                    
#>    base        * 3.6.0      2018-10-21 [?] local                             
#>    base64enc     0.1-3      2015-07-28 [1] CRAN (R 3.5.0)                    
#>    callr         3.0.0.9001 2018-10-30 [1] Github (r-lib/callr@942ba24)      
#>    cli           1.0.1.9000 2018-10-30 [1] Github (r-lib/cli@56538e3)        
#>  P compiler      3.6.0      2018-10-21 [2] local                             
#>    crayon        1.3.4      2017-09-16 [1] CRAN (R 3.5.1)                    
#>  P datasets    * 3.6.0      2018-10-21 [2] local                             
#>    debugme       1.1.0      2017-10-22 [1] CRAN (R 3.5.1)                    
#>    desc          1.2.0      2018-10-30 [1] Github (r-lib/desc@7c12d36)       
#>    devtools      2.0.1      2018-10-26 [1] CRAN (R 3.5.1)                    
#>    digest        0.6.18     2018-10-10 [1] CRAN (R 3.5.1)                    
#>    evaluate      0.12       2018-10-09 [1] CRAN (R 3.5.1)                    
#>    fs            1.2.6      2018-08-23 [1] CRAN (R 3.5.1)                    
#>    glue          1.3.0      2018-07-17 [1] CRAN (R 3.5.1)                    
#>  P graphics    * 3.6.0      2018-10-21 [2] local                             
#>  P grDevices   * 3.6.0      2018-10-21 [2] local                             
#>    htmltools     0.3.6      2017-04-28 [1] CRAN (R 3.5.1)                    
#>    knitr         1.20       2018-02-20 [1] CRAN (R 3.5.1)                    
#>    magrittr      1.5        2014-11-22 [1] CRAN (R 3.5.1)                    
#>    memoise       1.1.0      2017-04-21 [1] CRAN (R 3.5.1)                    
#>  P methods     * 3.6.0      2018-10-21 [2] local                             
#>    pkgbuild      1.0.2      2018-10-16 [1] CRAN (R 3.5.1)                    
#>    pkgload       1.0.2      2018-10-29 [1] CRAN (R 3.6.0)                    
#>    prettyunits   1.0.2      2015-07-13 [1] CRAN (R 3.5.1)                    
#>    processx      3.2.0      2018-08-16 [1] CRAN (R 3.5.1)                    
#>    ps            1.2.1      2018-11-06 [1] CRAN (R 3.6.0)                    
#>    R6            2.3.0      2018-10-04 [1] CRAN (R 3.5.1)                    
#>    Rcpp          1.0.0      2018-11-07 [1] CRAN (R 3.6.0)                    
#>    remotes       2.0.2      2018-10-30 [1] CRAN (R 3.6.0)                    
#>    rlang         0.3.0.1    2018-10-25 [1] CRAN (R 3.5.1)                    
#>    rmarkdown     1.10.15    2018-11-08 [1] Github (rstudio/rmarkdown@9875136)
#>    rprojroot     1.3-2      2018-01-03 [1] CRAN (R 3.5.1)                    
#>    sessioninfo   1.1.1      2018-11-05 [1] CRAN (R 3.6.0)                    
#>  P stats       * 3.6.0      2018-10-21 [2] local                             
#>    stringi       1.2.4      2018-07-20 [1] CRAN (R 3.6.0)                    
#>    stringr       1.3.1      2018-05-10 [1] CRAN (R 3.5.1)                    
#>    testthat      2.0.1      2018-10-13 [1] CRAN (R 3.5.1)                    
#>  P tools         3.6.0      2018-10-21 [2] local                             
#>    usethis       1.4.0.9000 2018-10-28 [1] Github (r-lib/usethis@44dd72c)    
#>  P utils       * 3.6.0      2018-10-21 [2] local                             
#>    withr         2.1.2      2018-03-15 [1] CRAN (R 3.5.1)                    
#>    yaml          2.2.0      2018-07-25 [1] CRAN (R 3.5.1)                    
#> 
#> [1] C:/Users/inp099/Documents/R/win-library/3.6
#> [2] C:/Program Files/R/R-devel/library
#> 
#>  P -- Loaded and on-disk path mismatch.

Created on 2018-11-12 by the reprex package (v0.2.1)

@lorenzwalthert
Copy link
Collaborator

Thanks. Will look into this later this week but as a warning, R devel and styler are currently not guaranteed to work well together because of #419.

@nlarusstone
Copy link

I'm encountering the same issue in R 4.0.3. Package versions:

> packageVersion("lintr")
[1] ‘2.0.1’
> packageVersion("styler")
[1] ‘1.3.2’

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Feb 24, 2021

Thanks. I refer to https://style.tidyverse.org/syntax.html#indenting, in particular 2.4.1:

Curly braces, {}, define the most important hierarchy of R code. To make this hierarchy easy to see:

  • { should be the last character on the line. Related code (e.g., an if clause, a function declaration, a trailing comma, …) must be on the same line as the opening brace.
  • The contents should be indented by two spaces.
  • } should be the first character on the line.
# good
tryCatch(
  {
    x <- scan()
    cat("Total: ", sum(x), "\n", sep = "")
  },
  interrupt = function(e) {
    message("Aborted by user")
  }
)

For that reason, I believe {styler} formatting matches the tidyverse style guide. Note that Jim Hester already disagreed on that edge case before in #549 (comment).

In addition, I think irrelevant of formatting, code like

iris %>% 
  {if (x != 2) {
    dplyr::select(.data = ., -Species) %>%
      tibble::as.tibble(.)
  } else {
    tibble::as.tibble(.)
  }}

is not very readable. You can use purrr::when() to make it more functional, see my StackOverflow answer.

@nlarusstone
Copy link

nlarusstone commented Feb 25, 2021

Sorry -- my message wasn't clear. I don't have strong opinions about the right way to do this in R and I don't have this exact code, I simply had the issue where styler would auto-correct code and then lintr would fail.

It looks like the recommended solution is:

tryCatch(
  expr = {
    ...
  }
)

It's kind of difficult to figure that -- perhaps this could be a docs issue?

@lorenzwalthert
Copy link
Collaborator

Not sure why we should document it in styler. Once you run styler, it will result in correct formatting. I believe the solution to this is that lintr does stop complaining about this formatting, because it is Tidyverse style compliant. For that, you'd have to open an issue in lintr (or simply deactivate this particular rule). So what I am trying to say is that this is not a styler issue when lintr complains.

@IndrajeetPatil
Copy link
Collaborator Author

You can have a .lintr like the following to avoid this issue:

linters: with_defaults(open_curly_linter = NULL, closed_curly_linter = NULL)

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

No branches or pull requests

3 participants