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

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

Open
IndrajeetPatil opened this issue Jul 20, 2023 · 7 comments

Comments

@IndrajeetPatil
Copy link
Collaborator

IndrajeetPatil commented Jul 20, 2023

This is reported in r-lib/styler#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
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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

Another one that looks wrong for {lintr}:

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

@jabenninghoff
Copy link

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
Copy link
Collaborator

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

Some suggestions:

  • File some PRs on label:indentation issues (obv 😄)
  • Possibly close some label:indentation issues if there's nothing for {lintr} to do
  • Help point out which issues you think are highest priority, possibly adding them to milestone:3.1.1 to make sure they're fixed before we bundle up a release
  • Help point out which issues you think are relatively easy to fix -- I've been staying away on the assumption it's relatively much easier for you to handle fixes, but there may be some that won't require deep understanding of the indentation_linter() internals you could identify & I could try my hand at.

Thanks!

@AshesITR
Copy link
Collaborator

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.

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

4 participants