search

r-lib / lintr

Static Code Analysis for R
https://lintr.r-lib.org
Other
1.19k stars 185 forks source link

Disagreement between `{styler}` and `indentation_linter()` about multiline conditional expressions #2007

Open IndrajeetPatil opened 1 year ago

IndrajeetPatil commented 1 year ago

This is reported in https://github.com/r-lib/styler/issues/1065, but I'd also like to track this in {lintr}:

# code styled with {styler}
"foo <- function(info) {
  if (info$is_dispersion ||
    info$is_zero_inflated ||
    info$is_zeroinf) {
    NULL
  }
}" -> code

# produces a lint
lintr::lint(text = code, linters = lintr::indentation_linter())
#> <text>:3:4: style: [indentation_linter] Indentation should be 8 spaces but is 4 spaces.
#>     info$is_zero_inflated ||
#>    ^~~~~

Created on 2023-07-20 with reprex v2.0.2

Session info ``` r sessioninfo::session_info() #> ─ Session info ─────────────────────────────────────────────────────────────── #> setting value #> version R version 4.3.0 (2023-04-21) #> os macOS Ventura 13.4.1 #> system aarch64, darwin20 #> ui X11 #> language (EN) #> collate en_US.UTF-8 #> ctype en_US.UTF-8 #> tz Europe/Berlin #> date 2023-07-20 #> pandoc 3.1.3 @ /usr/local/bin/ (via rmarkdown) #> #> ─ Packages ─────────────────────────────────────────────────────────────────── #> package * version date (UTC) lib source #> backports 1.4.1 2021-12-13 [1] CRAN (R 4.3.0) #> callr 3.7.3 2022-11-02 [1] CRAN (R 4.3.0) #> cli 3.6.1 2023-03-23 [1] CRAN (R 4.3.0) #> crayon 1.5.2 2022-09-29 [1] CRAN (R 4.3.0) #> cyclocomp 1.1.0 2016-09-10 [1] CRAN (R 4.3.0) #> data.table 1.14.8 2023-02-17 [1] CRAN (R 4.3.0) #> desc 1.4.2 2022-09-08 [1] CRAN (R 4.3.0) #> digest 0.6.33 2023-07-07 [1] CRAN (R 4.3.0) #> evaluate 0.21 2023-05-05 [1] CRAN (R 4.3.0) #> fansi 1.0.4 2023-01-22 [1] CRAN (R 4.3.0) #> fastmap 1.1.1 2023-02-24 [1] CRAN (R 4.3.0) #> fs 1.6.3 2023-07-20 [1] CRAN (R 4.3.0) #> glue 1.6.2 2022-02-24 [1] CRAN (R 4.3.0) #> htmltools 0.5.5 2023-03-23 [1] CRAN (R 4.3.0) #> knitr 1.43 2023-05-25 [1] CRAN (R 4.3.0) #> lazyeval 0.2.2 2019-03-15 [1] CRAN (R 4.3.0) #> lifecycle 1.0.3 2022-10-07 [1] CRAN (R 4.3.0) #> lintr 3.1.0 2023-07-19 [1] CRAN (R 4.3.0) #> magrittr 2.0.3 2022-03-30 [1] CRAN (R 4.3.0) #> pillar 1.9.0 2023-03-22 [1] CRAN (R 4.3.0) #> pkgconfig 2.0.3 2019-09-22 [1] CRAN (R 4.3.0) #> processx 3.8.2 2023-06-30 [1] CRAN (R 4.3.0) #> ps 1.7.5 2023-04-18 [1] CRAN (R 4.3.0) #> purrr 1.0.1 2023-01-10 [1] CRAN (R 4.3.0) #> R.cache 0.16.0 2022-07-21 [1] CRAN (R 4.3.0) #> R.methodsS3 1.8.2 2022-06-13 [1] CRAN (R 4.3.0) #> R.oo 1.25.0 2022-06-12 [1] CRAN (R 4.3.0) #> R.utils 2.12.2 2022-11-11 [1] CRAN (R 4.3.0) #> R6 2.5.1 2021-08-19 [1] CRAN (R 4.3.0) #> remotes 2.4.2.1 2023-07-18 [1] CRAN (R 4.3.0) #> reprex 2.0.2 2022-08-17 [1] CRAN (R 4.3.0) #> rex 1.2.1 2021-11-26 [1] CRAN (R 4.3.0) #> rlang 1.1.1 2023-04-28 [1] CRAN (R 4.3.0) #> rmarkdown 2.23 2023-07-01 [1] CRAN (R 4.3.0) #> rprojroot 2.0.3 2022-04-02 [1] CRAN (R 4.3.0) #> rstudioapi 0.15.0 2023-07-07 [1] CRAN (R 4.3.0) #> sessioninfo 1.2.2 2021-12-06 [1] CRAN (R 4.3.0) #> styler 1.10.1 2023-06-12 [1] Github (r-lib/styler@acd0b46) #> tibble 3.2.1 2023-03-20 [1] CRAN (R 4.3.0) #> utf8 1.2.3 2023-01-31 [1] CRAN (R 4.3.0) #> vctrs 0.6.3 2023-06-14 [1] CRAN (R 4.3.0) #> withr 2.5.0 2022-03-03 [1] CRAN (R 4.3.0) #> xfun 0.39 2023-04-20 [1] CRAN (R 4.3.0) #> xml2 1.3.5 2023-07-06 [1] CRAN (R 4.3.0) #> xmlparsedata 1.0.5 2021-03-06 [1] CRAN (R 4.3.0) #> yaml 2.3.7 2023-01-23 [1] CRAN (R 4.3.0) #> #> [1] /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/library #> #> ────────────────────────────────────────────────────────────────────────────── ```
MichaelChirico commented 1 year ago

I'm also seeing many such examples. I haven't had a chance to understand if they all fit the same rule as this issue. I'll post more examples if they look different enough.

FWIW I think indentation_linter gets it right here. if if has function call rules, at least the ) should move to the subsequent line.

MichaelChirico commented 1 year ago

Here's one where I think {lintr} is wrong:

stopifnot(
  `A very long test failure description` =
    inherits(found, "error") &&
      grepl("A regex to be found in the.*parent", found$parent$message)
)
MichaelChirico commented 1 year ago

Here's another one I think {lintr} is wrong:

FOO <- function(opt = c(
                  "opt1", "opt2", "opt3", "opt4", ...
                ),
                ...) {
  ...
}

This might be better using the double-indent declaration style, but not always so.

MichaelChirico commented 1 year ago

Another one that looks wrong for {lintr}:

foo <- function() {
  return(unique(subset(geo,
    RegionCode == "US",
    select = c("AdminLevel1Code", "AdminLevel2Name")
  )))
}
jabenninghoff commented 1 year ago

I have another example that appears to be related. This is styled according to styler but indentation_linter complains.

txsamp <- subset(txhousing, city %in% c("Houston", "Fort Worth", "San Antonio", "Dallas", "Austin"))

(d <- ggplot(data = txsamp, aes(x = sales, y = median)) +
  geom_point(aes(colour = city)))
(p <- ggplot(txsamp, aes(x = median, fill = city)) +
  geom_histogram(position = "dodge", binwidth = 15000))
(v <- ggplot(faithfuld) +
  geom_tile(aes(waiting, eruptions, fill = density)))
MichaelChirico commented 1 year ago

Hey @AshesITR it would be nice to have some progress on indentation_linter() in the 3.1.1 release.

Some suggestions:

Thanks!

AshesITR commented 1 year ago

Hi, I've cleaned up some issues. The styler consistency related ones are still open with the tidyverse styleguide so we can't really move unless we want a "styler" mode for indentation linter which bends twoards styler where possible.

Unfortunately, none of the indentation issues that are still open seem particularly easy to tackle. Not sure I'll be able to create a PR in due time so feel free to try.