diff --git a/.Rbuildignore b/.Rbuildignore index 257a72963..23f41e938 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -23,6 +23,7 @@ ^bench$ ^tests/testthat/dummy_packages/package/[.]Rbuildignore$ ^tests/testthat/dummy_packages/cp1252/[.]Rbuildignore$ +testthat-problems[.]rds$ ^_pkgdown\.yaml$ ^docs$ ^pkgdown$ diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md new file mode 100644 index 000000000..10289cd35 --- /dev/null +++ b/.github/CONTRIBUTING.md @@ -0,0 +1,39 @@ +# Contributing to `{lintr}` + +This outlines how to propose a change to `{lintr}`. For a detailed discussion on contributing to this, r-lib, and other tidyverse packages, please see the [development contributing guide](https://rstd.io/tidy-contrib) and our [code review principles](https://code-review.tidyverse.org/). + +## Fixing typos + +You can fix typos, spelling mistakes, or grammatical errors in the documentation directly using the GitHub web interface, as long as the changes are made in the _source_ file. This generally means you'll need to edit [roxygen2 comments](https://roxygen2.r-lib.org/articles/roxygen2.html) in an `.R`, not a `.Rd` file. You can find the `.R` file that generates the `.Rd` by reading the comment in the first line. + +## Bigger changes + +If you want to make a bigger change, it's a good idea to first file an issue and make sure someone from the team agrees that it’s needed. If you’ve found a bug, please file an issue that illustrates the bug with a minimal [reprex](https://www.tidyverse.org/help/#reprex) (this will also help you write a unit test, if needed). See the tidyverse guide on [how to create a great issue](https://code-review.tidyverse.org/issues/) for more advice. + +### Adding a new linter + +If you wish to contribute a new linter, the [Creating new linters](https://lintr.r-lib.org/articles/creating_linters.html) article serves as a comprehensive guide. + +### Pull request process + +* Fork the package and clone onto your computer. If you haven't done this before, we recommend using `usethis::create_from_github("r-lib/lintr", fork = TRUE)`. + +* Install all development dependencies with `devtools::install_dev_deps()`, and then make sure the package passes R CMD check by running `devtools::check()`. If R CMD check doesn't pass cleanly, it's a good idea to ask for help before continuing. + +* Create a Git branch for your pull request (PR). We recommend using `usethis::pr_init("brief-description-of-change")`. At a minimum, please avoid submitting PRs from your fork's `main` branch` as this can make the review process more complicated. + +* Make your changes, commit them to Git, and create a PR using `usethis::pr_push()`. Follow the prompts in your browser to complete the process. Use a concise title for your PR that summarizes the change, and include `Fixes #issue-number` in the PR _description_. Doing so will automatically close the linked issue when the PR is merged. For complicated changes, add a textual overview of what your PR does in the description. Consider breaking up large PRs into a chain of more digestible+focused smaller PRs. + +* For user-facing changes, add a bullet appropriately in the top section of `NEWS.md` (i.e. below the first header). Follow the style described in . Most importantly, your audience for NEWS items is a package user, i.e., _not_ a package developer. + +### Code style + +* New code should follow the tidyverse [style guide](https://style.tidyverse.org). You can use the [styler](https://CRAN.R-project.org/package=styler) package to apply these styles. + +* We use [roxygen2](https://cran.r-project.org/package=roxygen2), with [Markdown syntax](https://cran.r-project.org/web/packages/roxygen2/vignettes/rd-formatting.html), for documentation. + +* We use [testthat](https://cran.r-project.org/package=testthat) for unit tests. Contributions with test cases included are easier to accept. + +## Code of Conduct + +Please note that the lintr project is released with a [Contributor Code of Conduct](CODE_OF_CONDUCT.md). By contributing to this project you agree to abide by its terms. diff --git a/.github/workflows/pkgdown.yaml b/.github/workflows/pkgdown.yaml index 3d75fb082..91711d445 100644 --- a/.github/workflows/pkgdown.yaml +++ b/.github/workflows/pkgdown.yaml @@ -39,7 +39,7 @@ jobs: - name: Deploy to GitHub pages 🚀 if: github.event_name != 'pull_request' - uses: JamesIves/github-pages-deploy-action@v4.6.3 + uses: JamesIves/github-pages-deploy-action@v4.7.2 with: clean: false branch: gh-pages diff --git a/DESCRIPTION b/DESCRIPTION index ff326321a..87ec42de2 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -23,7 +23,7 @@ BugReports: https://github.com/r-lib/lintr/issues Depends: R (>= 4.0) Imports: - backports (>= 1.1.7), + backports (>= 1.4.0), cli (>= 3.4.0), codetools, digest, diff --git a/NAMESPACE b/NAMESPACE index 6b5c46937..d7089a304 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -2,6 +2,7 @@ S3method("[",lints) S3method(as.data.frame,lints) +S3method(data.table::as.data.table,lints) S3method(format,lint) S3method(format,lints) S3method(names,lints) @@ -9,6 +10,7 @@ S3method(print,lint) S3method(print,lints) S3method(split,lints) S3method(summary,lints) +S3method(tibble::as_tibble,lints) export(Lint) export(Linter) export(T_and_F_symbol_linter) @@ -176,17 +178,15 @@ importFrom(rex,re_matches) importFrom(rex,re_substitutes) importFrom(rex,regex) importFrom(rex,rex) +importFrom(stats,complete.cases) importFrom(stats,na.omit) importFrom(tools,R_user_dir) importFrom(utils,capture.output) importFrom(utils,getParseData) -importFrom(utils,getTxtProgressBar) importFrom(utils,globalVariables) importFrom(utils,head) importFrom(utils,relist) -importFrom(utils,setTxtProgressBar) importFrom(utils,tail) -importFrom(utils,txtProgressBar) importFrom(xml2,as_list) importFrom(xml2,xml_attr) importFrom(xml2,xml_children) diff --git a/NEWS.md b/NEWS.md index d017c606a..688c18fab 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,7 +6,6 @@ + `source_file=` argument to `ids_with_token()` and `with_id()`. + Passing linters by name or as non-`"linter"`-classed functions. + `linter=` argument of `Lint()`. - + Linters `closed_curly_linter()`, `open_curly_linter()`, `paren_brace_linter()`, and `semicolon_terminator_linter()`.. + `with_defaults()`. + Linters `closed_curly_linter()`, `open_curly_linter()`, `paren_brace_linter()`, and `semicolon_terminator_linter()`. + Helper `with_defaults()`. @@ -19,7 +18,6 @@ * `scalar_in_linter` is now configurable to allow other `%in%` like operators to be linted. The data.table operator `%chin%` is no longer linted by default; use `in_operators = "%chin%"` to continue linting it. (@F-Noelle) * `lint()` and friends now normalize paths to forward slashes on Windows (@olivroy, #2613). * `undesirable_function_linter()`, `undesirable_operator_linter()`, and `list_comparison_linter()` were removed from the tag `efficiency` (@IndrajeetPatil, #2655). If you use `linters_with_tags("efficiency")` to include these linters, you'll need to adjust your config to keep linting your code against them. We did not find any such users on GitHub. - ## Bug fixes @@ -29,6 +27,7 @@ * `.lintr` configs set by option `lintr.linter_file` or environment variable `R_LINTR_LINTER_FILE` can point to subdirectories (#2512, @MichaelChirico). * `indentation_linter()` returns `ranges[1L]==1L` when the offending line has 0 spaces (#2550, @MichaelChirico). * `literal_coercion_linter()` doesn't surface a warning about NAs during coercion for code like `as.integer("a")` (#2566, @MichaelChirico). +* `commented_code_linter()` can detect commented code that ends with a pipe (#2671, @jcken95) ## Changes to default linters @@ -61,6 +60,7 @@ * `paste_linter()` is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico). * `expect_no_lint()` was added as new function to cover the typical use case of expecting no lint message, akin to the recent {testthat} functions like `expect_no_warning()` (#2580, @F-Noelle). * `lint()` and friends emit a message if no lints are found (#2643, @IndrajeetPatil). +* `{lintr}` now has a hex sticker (https://github.com/rstudio/hex-stickers/pull/110). Thank you, @gregswinehart! ### New linters @@ -90,9 +90,10 @@ ## Notes -* All user-facing messages are now prepared using the `{cli}` package (#2418, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent. +* All user-facing messages (including progress bars) are now prepared using the `{cli}` package (#2418 and #2641, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent. * File locations in lints and error messages contain clickable hyperlinks to improve code navigation (#2645, #2588, @olivroy). * {lintr} now depends on R version 4.0.0. It already does so implicitly due to recursive upstream dependencies requiring this version; we've simply made that dependency explicit and up-front (#2569, @MichaelChirico). +* Some code with parameters accepting regular expressions is less strict about whether there are capture groups (#2678, @MichaelChirico). In particular, this affects `unreachable_code_linter(allow_comment_regex=)` and `expect_lint(checks=)`. # lintr 3.1.2 diff --git a/R/class_equals_linter.R b/R/class_equals_linter.R index 2dd24b83d..1f3483e9b 100644 --- a/R/class_equals_linter.R +++ b/R/class_equals_linter.R @@ -48,9 +48,10 @@ class_equals_linter <- function() { bad_expr <- xml_find_all(xml_calls, xpath) operator <- xml_find_chr(bad_expr, "string(*[2])") - lint_message <- sprintf( - "Use inherits(x, 'class-name'), is. or is(x, 'class') instead of comparing class(x) with %s.", - operator + lint_message <- paste0( + "Use inherits(x, 'class-name'), is. for S3 classes, ", + "or is(x, 'S4Class') for S4 classes, ", + "instead of comparing class(x) with ", operator, "." ) xml_nodes_to_lints( bad_expr, diff --git a/R/commented_code_linter.R b/R/commented_code_linter.R index fcc4af9ef..97f9c8d8e 100644 --- a/R/commented_code_linter.R +++ b/R/commented_code_linter.R @@ -83,9 +83,10 @@ commented_code_linter <- function() { all_comments <- xml_text(all_comment_nodes) code_candidates <- re_matches(all_comments, code_candidate_regex, global = FALSE, locations = TRUE) extracted_code <- code_candidates[, "code"] - # ignore trailing ',' when testing for parsability - extracted_code <- re_substitutes(extracted_code, rex(",", any_spaces, end), "") + # ignore trailing ',' or pipes ('|>', '%>%') when testing for parsability + extracted_code <- re_substitutes(extracted_code, rex(or(",", "|>", "%>%"), any_spaces, end), "") extracted_code <- re_substitutes(extracted_code, rex(start, any_spaces, ","), "") + is_parsable <- which(vapply(extracted_code, parsable, logical(1L))) lint_list <- xml_nodes_to_lints( diff --git a/R/expect_lint.R b/R/expect_lint.R index 7bc1b65fd..78658ffc0 100644 --- a/R/expect_lint.R +++ b/R/expect_lint.R @@ -105,16 +105,10 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") { ) # deparse ensures that NULL, list(), etc are handled gracefully ok <- if (field == "message") { - re_matches(value, check) + re_matches_logical(value, check) } else { isTRUE(all.equal(value, check)) } - if (!is.logical(ok)) { - cli_abort(c( - x = "Invalid regex result. Did you mistakenly have a capture group in the regex?", - i = "You can match parentheses with a character class, i.e. inside `[]`." - )) - } testthat::expect(ok, msg) }) }, diff --git a/R/extract.R b/R/extract.R index c9aa57b70..edd12a456 100644 --- a/R/extract.R +++ b/R/extract.R @@ -9,7 +9,7 @@ extract_r_source <- function(filename, lines, error = identity) { output <- rep.int(NA_character_, length(lines)) chunks <- tryCatch(get_chunk_positions(pattern = pattern, lines = lines), error = error) - if (inherits(chunks, "error") || inherits(chunks, "lint")) { + if (is_error(chunks) || is_lint(chunks)) { assign("e", chunks, envir = parent.frame()) # error, so return empty code return(output) diff --git a/R/get_source_expressions.R b/R/get_source_expressions.R index ec8a4d406..3f1817023 100644 --- a/R/get_source_expressions.R +++ b/R/get_source_expressions.R @@ -86,7 +86,7 @@ get_source_expressions <- function(filename, lines = NULL) { source_expression$content <- get_content(source_expression$lines) parsed_content <- get_source_expression(source_expression, error = function(e) lint_parse_error(e, source_expression)) - if (inherits(e, "lint") && (is.na(e$line) || !nzchar(e$line) || e$message == "unexpected end of input")) { + if (is_lint(e) && (is.na(e$line) || !nzchar(e$line) || e$message == "unexpected end of input")) { # Don't create expression list if it's unreliable (invalid encoding or unhandled parse error) expressions <- list() } else { @@ -502,7 +502,7 @@ get_source_expression <- function(source_expression, error = identity) { error = error ) - if (inherits(parsed_content, c("error", "lint"))) { + if (is_error(parsed_content) || is_lint(parsed_content)) { assign("e", parsed_content, envir = parent.frame()) parse_error <- TRUE } @@ -513,7 +513,7 @@ get_source_expression <- function(source_expression, error = identity) { error = error ) - if (inherits(parsed_content, c("error", "lint"))) { + if (is_error(parsed_content) || is_lint(parsed_content)) { # Let parse errors take precedence over encoding problems if (!parse_error) assign("e", parsed_content, envir = parent.frame()) return() # parsed_content is unreliable if encoding is invalid diff --git a/R/indentation_linter.R b/R/indentation_linter.R index a89f3da7c..de4a7c777 100644 --- a/R/indentation_linter.R +++ b/R/indentation_linter.R @@ -257,9 +257,12 @@ indentation_linter <- function(indent = 2L, hanging_indent_style = c("tidy", "al } # Only lint non-empty lines if the indentation level doesn't match. + # TODO: remove styler ignore directives once tidyverse/style/issues/197 is resolved + # styler: off bad_lines <- which(indent_levels != expected_indent_levels & nzchar(trimws(source_expression$file_lines)) & !in_str_const) + # styler: on if (length(bad_lines) > 0L) { # Suppress consecutive lints with the same indentation difference, to not generate an excessive number of lints is_consecutive_lint <- c(FALSE, diff(bad_lines) == 1L) diff --git a/R/lint.R b/R/lint.R index 042fbeaca..36faf36de 100644 --- a/R/lint.R +++ b/R/lint.R @@ -27,11 +27,14 @@ #' @return An object of class `c("lints", "list")`, each element of which is a `"list"` object. #' #' @examples +#' # linting inline-code +#' lint("a = 123\n") +#' lint(text = "a = 123") +#' +#' # linting a file #' f <- tempfile() #' writeLines("a=1", f) -#' lint(f) # linting a file -#' lint("a = 123\n") # linting inline-code -#' lint(text = "a = 123") # linting inline-code +#' lint(f) #' unlink(f) #' #' @export @@ -182,20 +185,24 @@ lint_dir <- function(path = ".", ..., return(lints) } - pb <- if (isTRUE(show_progress)) { - txtProgressBar(max = length(files), style = 3L) + if (isTRUE(show_progress)) { + lints <- lapply( + # NB: This cli API is experimental (https://github.com/r-lib/cli/issues/709) + cli::cli_progress_along(files, name = "Running linters"), + function(idx) { + lint(files[idx], ..., parse_settings = FALSE, exclusions = exclusions) + } + ) + } else { + lints <- lapply( + files, + function(file) { # nolint: unnecessary_lambda_linter. + lint(file, ..., parse_settings = FALSE, exclusions = exclusions) + } + ) } - lints <- flatten_lints(lapply( - files, - function(file) { - maybe_report_progress(pb) - lint(file, ..., parse_settings = FALSE, exclusions = exclusions) - } - )) - - if (!is.null(pb)) close(pb) - + lints <- flatten_lints(lints) lints <- reorder_lints(lints) if (relative_path) { @@ -688,15 +695,8 @@ has_positional_logical <- function(dots) { !nzchar(names2(dots)[1L]) } -maybe_report_progress <- function(pb) { - if (is.null(pb)) { - return(invisible()) - } - setTxtProgressBar(pb, getTxtProgressBar(pb) + 1L) -} - maybe_append_error_lint <- function(lints, error, lint_cache, filename) { - if (inherits(error, "lint")) { + if (is_lint(error)) { error$linter <- "error" lints[[length(lints) + 1L]] <- error diff --git a/R/lintr-package.R b/R/lintr-package.R index 70f8d0d11..0e7f558b4 100644 --- a/R/lintr-package.R +++ b/R/lintr-package.R @@ -11,10 +11,9 @@ #' @importFrom cli cli_inform cli_abort cli_warn #' @importFrom glue glue glue_collapse #' @importFrom rex rex regex re_matches re_substitutes character_class -#' @importFrom stats na.omit +#' @importFrom stats complete.cases na.omit #' @importFrom tools R_user_dir -#' @importFrom utils capture.output getParseData getTxtProgressBar globalVariables head relist -#' setTxtProgressBar tail txtProgressBar +#' @importFrom utils capture.output getParseData globalVariables head relist tail #' @importFrom xml2 as_list #' xml_attr xml_children xml_find_all xml_find_chr xml_find_lgl xml_find_num xml_find_first xml_name xml_text ## lintr namespace: end diff --git a/R/methods.R b/R/methods.R index 7a8fe3cc1..211222a58 100644 --- a/R/methods.R +++ b/R/methods.R @@ -173,6 +173,7 @@ as.data.frame.lints <- function(x, row.names = NULL, optional = FALSE, ...) { # ) } +#' @exportS3Method tibble::as_tibble as_tibble.lints <- function(x, ..., # nolint: object_name_linter. .rows = NULL, .name_repair = c("check_unique", "unique", "universal", "minimal"), @@ -181,6 +182,7 @@ as_tibble.lints <- function(x, ..., # nolint: object_name_linter. tibble::as_tibble(as.data.frame(x), ..., .rows = .rows, .name_repair = .name_repair, rownames = rownames) } +#' @exportS3Method data.table::as.data.table as.data.table.lints <- function(x, keep.rownames = FALSE, ...) { # nolint: object_name_linter. stopifnot(requireNamespace("data.table", quietly = TRUE)) data.table::setDT(as.data.frame(x), keep.rownames = keep.rownames, ...) diff --git a/R/nested_pipe_linter.R b/R/nested_pipe_linter.R index fd595b233..adc0d5089 100644 --- a/R/nested_pipe_linter.R +++ b/R/nested_pipe_linter.R @@ -51,8 +51,9 @@ #' @seealso [linters] for a complete list of linters available in lintr. #' @export nested_pipe_linter <- function( - allow_inline = TRUE, - allow_outer_calls = c("try", "tryCatch", "withCallingHandlers")) { + allow_inline = TRUE, + allow_outer_calls = c("try", "tryCatch", "withCallingHandlers") +) { multiline_and <- if (allow_inline) "@line1 != @line2 and" else "" xpath <- glue(" (//PIPE | //SPECIAL[{ xp_text_in_table(magrittr_pipes) }]) diff --git a/R/object_name_linter.R b/R/object_name_linter.R index 54cbb523d..ba9583039 100644 --- a/R/object_name_linter.R +++ b/R/object_name_linter.R @@ -146,13 +146,8 @@ object_name_linter <- function(styles = c("snake_case", "symbols"), regexes = ch } check_style <- function(nms, style, generics = character()) { - conforming <- re_matches(nms, style) + conforming <- re_matches_logical(nms, style) - # style has capture group(s) - if (is.data.frame(conforming)) { - # if any group is missing, all groups are missing, so just check the first column - conforming <- !is.na(conforming[[1L]]) - } # mark empty or NA names as conforming conforming <- is.na(nms) | !nzchar(nms) | conforming diff --git a/R/object_overwrite_linter.R b/R/object_overwrite_linter.R index 099129ed5..acee4c2ea 100644 --- a/R/object_overwrite_linter.R +++ b/R/object_overwrite_linter.R @@ -52,8 +52,9 @@ #' - #' @export object_overwrite_linter <- function( - packages = c("base", "stats", "utils", "tools", "methods", "graphics", "grDevices"), - allow_names = character()) { + packages = c("base", "stats", "utils", "tools", "methods", "graphics", "grDevices"), + allow_names = character() +) { for (package in packages) { if (!requireNamespace(package, quietly = TRUE)) { cli_abort("Package {.pkg {package}} is required, but not available.") diff --git a/R/return_linter.R b/R/return_linter.R index a0e1c245f..bb2383b2d 100644 --- a/R/return_linter.R +++ b/R/return_linter.R @@ -72,11 +72,12 @@ #' - #' @export return_linter <- function( - return_style = c("implicit", "explicit"), - allow_implicit_else = TRUE, - return_functions = NULL, - except = NULL, - except_regex = NULL) { + return_style = c("implicit", "explicit"), + allow_implicit_else = TRUE, + return_functions = NULL, + except = NULL, + except_regex = NULL +) { return_style <- match.arg(return_style) check_except <- !allow_implicit_else || return_style == "explicit" @@ -147,7 +148,8 @@ return_linter <- function( xml <- source_expression$xml_parsed_content if (defer_except) { assigned_functions <- xml_text(xml_find_all(xml, function_name_xpath)) - except <- union(except, assigned_functions[re_matches(assigned_functions, except_regex)]) + except <- + union(except, assigned_functions[re_matches_logical(assigned_functions, except_regex)]) except_xpath <- glue(except_xpath_fmt, except = except) body_xpath <- glue(body_xpath_fmt, except_xpath = except_xpath) } diff --git a/R/todo_comment_linter.R b/R/todo_comment_linter.R index 16e1de05f..5d6d94282 100644 --- a/R/todo_comment_linter.R +++ b/R/todo_comment_linter.R @@ -57,9 +57,9 @@ todo_comment_linter <- function(todo = c("todo", "fixme"), except_regex = NULL) comment_expr <- xml_find_all(xml, "//COMMENT") comment_text <- xml_text(comment_expr) - invalid_todo <- re_matches(comment_text, todo_comment_regex, ignore.case = TRUE) + invalid_todo <- re_matches_logical(comment_text, todo_comment_regex, ignore.case = TRUE) if (!is.null(valid_todo_regex)) { - invalid_todo <- invalid_todo & !re_matches(comment_text, valid_todo_regex) + invalid_todo <- invalid_todo & !re_matches_logical(comment_text, valid_todo_regex) } xml_nodes_to_lints( diff --git a/R/unnecessary_nesting_linter.R b/R/unnecessary_nesting_linter.R index 653406881..b198326bf 100644 --- a/R/unnecessary_nesting_linter.R +++ b/R/unnecessary_nesting_linter.R @@ -95,16 +95,17 @@ #' - [linters] for a complete list of linters available in lintr. #' @export unnecessary_nesting_linter <- function( - allow_assignment = TRUE, - allow_functions = c( - "switch", - "try", "tryCatch", "withCallingHandlers", - "quote", "expression", "bquote", "substitute", - "with_parameters_test_that", - "reactive", "observe", "observeEvent", - "renderCachedPlot", "renderDataTable", "renderImage", "renderPlot", - "renderPrint", "renderTable", "renderText", "renderUI" - )) { + allow_assignment = TRUE, + allow_functions = c( + "switch", + "try", "tryCatch", "withCallingHandlers", + "quote", "expression", "bquote", "substitute", + "with_parameters_test_that", + "reactive", "observe", "observeEvent", + "renderCachedPlot", "renderDataTable", "renderImage", "renderPlot", + "renderPrint", "renderTable", "renderText", "renderUI" + ) +) { exit_calls <- c("stop", "return", "abort", "quit", "q") exit_call_expr <- glue(" expr[SYMBOL_FUNCTION_CALL[{xp_text_in_table(exit_calls)}]] diff --git a/R/unreachable_code_linter.R b/R/unreachable_code_linter.R index 124b5a12f..f2e9f8d56 100644 --- a/R/unreachable_code_linter.R +++ b/R/unreachable_code_linter.R @@ -130,7 +130,7 @@ unreachable_code_linter <- function(allow_comment_regex = getOption("covr.exclud drop_valid_comments <- function(expr, valid_comment_re) { is_valid_comment <- xml2::xml_name(expr) == "COMMENT" & - re_matches(xml_text(expr), valid_comment_re) + re_matches_logical(xml_text(expr), valid_comment_re) expr[!is_valid_comment] } diff --git a/R/utils.R b/R/utils.R index d23fc8902..1cdfca15c 100644 --- a/R/utils.R +++ b/R/utils.R @@ -186,8 +186,7 @@ read_lines <- function(file, encoding = settings$encoding, ...) { # nocov start # support for usethis::use_release_issue(). Make sure to use devtools::load_all() beforehand! -release_bullets <- function() { -} +release_bullets <- function() {} # nocov end # see issue #923, PR #2455 -- some locales ignore _ when running sort(), others don't. @@ -195,6 +194,17 @@ release_bullets <- function() { platform_independent_order <- function(x) order(tolower(x), method = "radix") platform_independent_sort <- function(x) x[platform_independent_order(x)] +#' re_matches with type-stable logical output +#' TODO(r-lib/rex#94): Use re_matches() option directly & deprecate this. +#' @noRd +re_matches_logical <- function(x, regex, ...) { + res <- re_matches(x, regex, ...) + if (is.data.frame(res)) { + res <- complete.cases(res) + } + res +} + #' Extract text from `STR_CONST` nodes #' #' Convert `STR_CONST` `text()` values into R strings. This is useful to account for arbitrary @@ -245,9 +255,12 @@ get_r_string <- function(s, xpath = NULL) { } is_linter <- function(x) inherits(x, "linter") +is_lint <- function(x) inherits(x, "lint") + +is_error <- function(x) inherits(x, "error") is_tainted <- function(lines) { - inherits(tryCatch(nchar(lines), error = identity), "error") + is_error(tryCatch(nchar(lines), error = identity)) } #' Check that the entries in ... are valid diff --git a/R/zzz.R b/R/zzz.R index e281353db..5fd5dd74e 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -329,12 +329,5 @@ settings <- new.env(parent = emptyenv()) )) reset_settings() - - if (requireNamespace("tibble", quietly = TRUE)) { - registerS3method("as_tibble", "lints", as_tibble.lints, asNamespace("tibble")) - } - if (requireNamespace("data.table", quietly = TRUE)) { - registerS3method("as.data.table", "lints", as.data.table.lints, asNamespace("data.table")) - } } # nocov end diff --git a/README.md b/README.md index 14bafb124..dc7c993d5 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# lintr +# lintr [![R build status](https://github.com/r-lib/lintr/workflows/R-CMD-check/badge.svg)](https://github.com/r-lib/lintr/actions) [![codecov.io](https://codecov.io/gh/r-lib/lintr/branch/main/graphs/badge.svg)](https://app.codecov.io/gh/r-lib/lintr?branch=main) diff --git a/man/figures/logo.png b/man/figures/logo.png new file mode 100644 index 000000000..7d9602ab9 Binary files /dev/null and b/man/figures/logo.png differ diff --git a/man/lint.Rd b/man/lint.Rd index a165a9343..da16dafea 100644 --- a/man/lint.Rd +++ b/man/lint.Rd @@ -89,11 +89,14 @@ Note that if files contain unparseable encoding problems, only the encoding prob unintelligible error messages from other linters. } \examples{ +# linting inline-code +lint("a = 123\n") +lint(text = "a = 123") + +# linting a file f <- tempfile() writeLines("a=1", f) -lint(f) # linting a file -lint("a = 123\n") # linting inline-code -lint(text = "a = 123") # linting inline-code +lint(f) unlink(f) if (FALSE) { diff --git a/paper/.gitignore b/paper/.gitignore new file mode 100644 index 000000000..075b2542a --- /dev/null +++ b/paper/.gitignore @@ -0,0 +1 @@ +/.quarto/ diff --git a/paper/paper.Rmd b/paper/paper.Rmd index 05e3e3f61..072cb1ef5 100644 --- a/paper/paper.Rmd +++ b/paper/paper.Rmd @@ -46,7 +46,7 @@ output: standalone: true bibliography: paper.bib csl: apa.csl -link-citations: yes +link-citations: true --- ```{r setup, warning=FALSE, message=FALSE, echo=FALSE} @@ -59,16 +59,19 @@ knitr::opts_chunk$set( library(lintr) withr::local_options(list( - lintr.format_width = 60L + lintr.format_width = 60L, + cli.condition_unicode_bullets = FALSE )) ``` # Statement of Need -R is an interpreted, dynamically-typed programming language [@base2023]. It is a popular choice for statistical analysis and visualization, and is used by a wide range of researchers and data scientists. The `{lintr}` package is an open-source R package that provides static code analysis [@enwiki:1218663830] to check for a variety of common problems related to readability, efficiency, consistency, style, etc. In particular, by default it enforces the tidyverse style guide [@Wickham2023]. It is designed to be easy to use and integrate into existing workflows, and can be used as part of an automated build or continuous integration process. `{lintr}` also integrates with a number of popular IDEs and text editors, such as RStudio and Visual Studio Code, making it convenient for users to run `{lintr}` checks on their code as they work. +In computer programming, "linting" is the process of analyzing the source code to identify possible programming and stylistic problems [@enwiki:1260589258] and a linter is a tool used for linting. A linter analyzes code to identify potential errors, stylistic issues, or deviations from coding standards. It helps ensure consistency, readability, and best practices by flagging common mistakes, such as syntax errors, unused variables, or improper formatting. Linters are essential for improving code quality, preventing bugs, and maintaining a clean codebase, especially in collaborative development environments [@enwiki:1218663830]. `{lintr}` is an open-source package that provides linters for the R programming language, which is an interpreted, dynamically-typed programming language [@base2023], and is used by a wide range of researchers and data scientists. `{lintr}` can thus act as a valuable tool for R users to help improve the quality and reliability of their code. # Features +By default, `{lintr}` enforces the tidyverse style guide [@Wickham2023,@Müller2024]. In this respect, it differs from other static code analysis tools in R (like `{codetools}` [@Tierney2024]), which are not opinionated and don't enforce any particular style of writing code, but, rather, check R code for possible problems (incidentally, `{lintr}` uses `{codetools}` as a backend for object usage linters). Additionally, `{lintr}` is concerned only with R code, so code-adjacent text like inline `{roxygen2}` comments [@Wickham2024roxy] will not be covered (cf. `{roxylint}` [@Kelkhoff2024]). + As of this writing, `{lintr}` offers `r length(all_linters())` linters. ```{r all_linters} @@ -77,7 +80,7 @@ library(lintr) length(all_linters()) ``` -Naturally, we can't discuss all of them here. To see details about all available linters, we encourage readers to see . +Naturally, we can't discuss all of them here. To see the most up-to-date details about all the available linters, we encourage readers to visit . We will showcase one linter for each kind of common problem found in R code. @@ -103,7 +106,7 @@ lint( - **Efficiency** -Sometimes the users might not be aware of a more efficient way offered by R for carrying out a computation. `{lintr}` offers linters to improve code efficiency by avoiding common inefficient patterns. +Sometimes users might not be aware of a more efficient way offered by R for carrying out a computation. `{lintr}` offers linters to improve code efficiency by avoiding common inefficient patterns. For example, the `any_is_na_linter()` linter detects usages of `any(is.na(x))` and suggests `anyNA(x)` as a more efficient alternative to detect presence of *any* missing values. @@ -217,11 +220,13 @@ There are two main ways to customize it: - Create new linters (by leveraging functions like `lintr::make_linter_from_xpath()`) tailored to match project- or organization-specific coding standards. +Indeed, `{goodpractice}` [@Padgham2024] bundles a set of custom linters that are not part of the default set of `{lintr}` linters, while `{box.linters}` [@Basa2024] extends `{lintr}` to support `{box}` modules [@Rudolph2024] and `{checklist}` includes linters as one of the strict checks for R packages [@Onkelinx2024]. `{flint}` [@Bacher2024] is a Rust-backed analogue inspired by `{lintr}` that also provides support for fixing lints. + # Benefits of using `{lintr}` There are several benefits to using `{lintr}` to analyze and improve R code. One of the most obvious is that it can help users identify and fix problems in their code, which can save time and effort during the development process. By catching issues early on, `{lintr}` can help prevent bugs and other issues from creeping into code, which can save time and effort when it comes to debugging and testing. -Another benefit of `{lintr}` is that it can help users write more readable and maintainable code. By enforcing a consistent style and highlighting potential issues, `{lintr}` can help users write code that is easier to understand and work with. This is especially important for larger projects or teams, where multiple contributors may be working on the same codebase and it is important to ensure that code is easy to follow and understand, particularly when frequently switching context among code primarily authored by different people. +Another benefit of `{lintr}` is that it can help users write more readable and maintainable code. By enforcing a consistent style and highlighting potential issues, `{lintr}` can help users write code that is easier to understand and work with. This is especially important for larger projects or teams, where multiple contributors may be working on the same codebase and it is important to ensure that code is easy to follow and understand, particularly when frequently switching context among code primarily authored by different people. `{lintr}` is designed to be easy to use and integrate into existing workflows, and can be used as part of an automated build or continuous integration process. `{lintr}` also integrates with a number of popular IDEs and text editors, such as RStudio and Visual Studio Code, making it convenient for users to run `{lintr}` checks on their code as they work. It can also be a useful tool for teaching and learning R. By providing feedback on code style and potential issues, it can help users learn good coding practices and improve their skills over time. This can be especially useful for beginners, who may not yet be familiar with all of the best practices for writing R code. diff --git a/paper/paper.bib b/paper/paper.bib index f9578f0c9..5b8e8db9e 100644 --- a/paper/paper.bib +++ b/paper/paper.bib @@ -32,10 +32,94 @@ @book{mcconnell2004code publisher={Pearson Education} } - @misc{ enwiki:1218663830, +@misc{enwiki:1218663830, author = "{Wikipedia contributors}", title = "Static program analysis --- {Wikipedia}{,} The Free Encyclopedia", year = "2024", url = "https://en.wikipedia.org/w/index.php?title=Static_program_analysis&oldid=1218663830", - note = "[Online; accessed 7-May-2024]" + note = "[Online; accessed 2-December-2024]" + } + +@misc{enwiki:1260589258, + author = "{Wikipedia contributors}", + title = "Lint (software) --- {Wikipedia}{,} The Free Encyclopedia", + year = "2024", + url = "https://en.wikipedia.org/w/index.php?title=Lint_(software)&oldid=1260589258", + note = "[Online; accessed 2-December-2024]" + } + +@Manual{Tierney2024, + title = {codetools: Code Analysis Tools for R}, + author = {Luke Tierney}, + year = {2024}, + note = {R package version 0.2-20}, + url = {https://CRAN.R-project.org/package=codetools}, + } + +@Manual{Bacher2024, + title = {flint: Find and Fix Lints in {R} Code}, + author = {Etienne Bacher}, + year = {2024}, + note = {R package version 0.0.2, +https://github.com/etiennebacher/flint}, + url = {https://flint.etiennebacher.com}, + } + +@Manual{Müller2024, + title = {styler: Non-Invasive Pretty Printing of R Code}, + author = {Kirill Müller and Lorenz Walthert and Indrajeet Patil}, + year = {2024}, + note = {R package version 1.10.3.9000, commit 6d2f0b34245b6bc712bf2fcabf240d9ca814f0ef}, + url = {https://github.com/r-lib/styler}, + } + +@Manual{Padgham2024, + title = {goodpractice: Advice on R Package Building}, + author = {Mark Padgham and Karina Marks and Daniel {de Bortoli} and Gabor Csardi and Hannah Frick and Owen Jones and Hannah Alexander}, + year = {2024}, + note = {R package version 1.0.5, +https://github.com/ropensci-review-tools/goodpractice}, + url = {https://docs.ropensci.org/goodpractice/}, + } + +@Manual{Rudolph2024, + title = {box: Write Reusable, Composable and Modular {R} Code}, + author = {Konrad Rudolph}, + year = {2024}, + note = {R package version 1.2.0}, + url = {https://CRAN.R-project.org/package=box}, + } + +@Manual{Basa2024, + title = {box.linters: Linters for 'box' Modules}, + author = {Ricardo Rodrigo Basa and Jakub Nowicki}, + year = {2024}, + note = {R package version 0.10.5}, + url = {https://CRAN.R-project.org/package=box.linters}, + } + +@Manual{Kelkhoff2024, + title = {roxylint: Lint 'roxygen2'-Generated Documentation}, + author = {Doug Kelkhoff}, + year = {2024}, + note = {R package version 0.1.0}, + url = {https://CRAN.R-project.org/package=roxylint}, + } + +@Manual{Wickham2024roxy, + title = {roxygen2: In-Line Documentation for {R}}, + author = {Hadley Wickham and Peter Danenberg and Gábor Csárdi and Manuel Eugster}, + year = {2024}, + note = {R package version 7.3.2}, + url = {https://CRAN.R-project.org/package=roxygen2}, + } + +@Manual{Onkelinx2024, + title = {checklist: A Thorough and Strict Set of Checks for {R} Packages and Source Code. Version 0.4.0}, + author = {Thierry Onkelinx}, + year = {2024}, + url = {https://inbo.github.io/checklist/}, + abstract = {An opinionated set of rules for R packages and R source code projects.}, + keywords = {quality control; documentation; publication}, + doi = {10.5281/zenodo.4028303}, } diff --git a/paper/paper.md b/paper/paper.md index ebdc1677d..c7a35d658 100644 --- a/paper/paper.md +++ b/paper/paper.md @@ -1,6 +1,6 @@ --- title: "Static Code Analysis for R" -date: "2024-09-05" +date: "2024-12-02" tags: ["R", "linter", "tidyverse"] authors: - name: Jim Hester @@ -46,27 +46,40 @@ output: standalone: true bibliography: paper.bib csl: apa.csl -link-citations: yes +link-citations: true --- # Statement of Need -R is an interpreted, dynamically-typed programming language [@base2023]. -It is a popular choice for statistical analysis and visualization, and -is used by a wide range of researchers and data scientists. The -`{lintr}` package is an open-source R package that provides static code -analysis [@enwiki:1218663830] to check for a variety of common problems -related to readability, efficiency, consistency, style, etc. In -particular, by default it enforces the tidyverse style guide -[@Wickham2023]. It is designed to be easy to use and integrate into -existing workflows, and can be used as part of an automated build or -continuous integration process. `{lintr}` also integrates with a number -of popular IDEs and text editors, such as RStudio and Visual Studio -Code, making it convenient for users to run `{lintr}` checks on their -code as they work. +In computer programming, "linting" is the process of analyzing the +source code to identify possible programming and stylistic problems +[@enwiki:1260589258] and a linter is a tool used for linting. A linter +analyzes code to identify potential errors, stylistic issues, or +deviations from coding standards. It helps ensure consistency, +readability, and best practices by flagging common mistakes, such as +syntax errors, unused variables, or improper formatting. Linters are +essential for improving code quality, preventing bugs, and maintaining a +clean codebase, especially in collaborative development environments +[@enwiki:1218663830]. `{lintr}` is an open-source package that provides +linters for the R programming language, which is an interpreted, +dynamically-typed programming language [@base2023], and is used by a +wide range of researchers and data scientists. `{lintr}` can thus act as +a valuable tool for R users to help improve the quality and reliability +of their code. # Features +By default, `{lintr}` enforces the tidyverse style guide +[@Wickham2023,@Müller2024]. In this respect, it differs from other +static code analysis tools in R (like `{codetools}` [@Tierney2024]), +which are not opinionated and don't enforce any particular style of +writing code, but, rather, check R code for possible problems +(incidentally, `{lintr}` uses `{codetools}` as a backend for object +usage linters). Additionally, `{lintr}` is concerned only with R code, +so code-adjacent text like inline `{roxygen2}` comments +[@Wickham2024roxy] will not be covered (cf. `{roxylint}` +[@Kelkhoff2024]). + As of this writing, `{lintr}` offers 113 linters. ``` r @@ -76,8 +89,8 @@ length(all_linters()) #> [1] 113 ``` -Naturally, we can't discuss all of them here. To see details about all -available linters, we encourage readers to see +Naturally, we can't discuss all of them here. To see the most up-to-date +details about all the available linters, we encourage readers to visit . We will showcase one linter for each kind of common problem found in R @@ -110,14 +123,13 @@ lint( text = "x >= 2.5", linters = redundant_ifelse_linter() ) -#> ℹ No lints found. ``` - **Efficiency** -Sometimes the users might not be aware of a more efficient way offered -by R for carrying out a computation. `{lintr}` offers linters to improve -code efficiency by avoiding common inefficient patterns. +Sometimes users might not be aware of a more efficient way offered by R +for carrying out a computation. `{lintr}` offers linters to improve code +efficiency by avoiding common inefficient patterns. For example, the `any_is_na_linter()` linter detects usages of `any(is.na(x))` and suggests `anyNA(x)` as a more efficient alternative @@ -143,7 +155,6 @@ lint( text = "anyNA(x)", linters = any_is_na_linter() ) -#> ℹ No lints found. ``` - **Readability** @@ -176,7 +187,6 @@ lint( text = "x != 2", linters = comparison_negation_linter() ) -#> ℹ No lints found. ``` - **Tidyverse style** @@ -204,7 +214,6 @@ lint( text = "my_var <- 1L", linters = object_name_linter() ) -#> ℹ No lints found. ``` - **Common mistakes** @@ -278,13 +287,20 @@ lint( text = "my.var <- 1L", linters = object_name_linter(styles = "dotted.case") ) -#> ℹ No lints found. ``` - Create new linters (by leveraging functions like `lintr::make_linter_from_xpath()`) tailored to match project- or organization-specific coding standards. +Indeed, `{goodpractice}` [@Padgham2024] bundles a set of custom linters +that are not part of the default set of `{lintr}` linters, while +`{box.linters}` [@Basa2024] extends `{lintr}` to support `{box}` modules +[@Rudolph2024] and `{checklist}` includes linters as one of the strict +checks for R packages [@Onkelinx2024]. `{flint}` [@Bacher2024] is a +Rust-backed analogue inspired by `{lintr}` that also provides support +for fixing lints. + # Benefits of using `{lintr}` There are several benefits to using `{lintr}` to analyze and improve R @@ -301,7 +317,12 @@ is easier to understand and work with. This is especially important for larger projects or teams, where multiple contributors may be working on the same codebase and it is important to ensure that code is easy to follow and understand, particularly when frequently switching context -among code primarily authored by different people. +among code primarily authored by different people. `{lintr}` is designed +to be easy to use and integrate into existing workflows, and can be used +as part of an automated build or continuous integration process. +`{lintr}` also integrates with a number of popular IDEs and text +editors, such as RStudio and Visual Studio Code, making it convenient +for users to run `{lintr}` checks on their code as they work. It can also be a useful tool for teaching and learning R. By providing feedback on code style and potential issues, it can help users learn diff --git a/_pkgdown.yaml b/pkgdown/_pkgdown.yaml similarity index 100% rename from _pkgdown.yaml rename to pkgdown/_pkgdown.yaml diff --git a/pkgdown/favicon/apple-touch-icon.png b/pkgdown/favicon/apple-touch-icon.png new file mode 100644 index 000000000..de8ec0236 Binary files /dev/null and b/pkgdown/favicon/apple-touch-icon.png differ diff --git a/pkgdown/favicon/favicon-96x96.png b/pkgdown/favicon/favicon-96x96.png new file mode 100644 index 000000000..e01781803 Binary files /dev/null and b/pkgdown/favicon/favicon-96x96.png differ diff --git a/pkgdown/favicon/favicon.ico b/pkgdown/favicon/favicon.ico new file mode 100644 index 000000000..d4b5a379b Binary files /dev/null and b/pkgdown/favicon/favicon.ico differ diff --git a/pkgdown/favicon/favicon.svg b/pkgdown/favicon/favicon.svg new file mode 100644 index 000000000..a02d2dcae --- /dev/null +++ b/pkgdown/favicon/favicon.svg @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/pkgdown/favicon/site.webmanifest b/pkgdown/favicon/site.webmanifest new file mode 100644 index 000000000..4ebda26b5 --- /dev/null +++ b/pkgdown/favicon/site.webmanifest @@ -0,0 +1,21 @@ +{ + "name": "", + "short_name": "", + "icons": [ + { + "src": "/web-app-manifest-192x192.png", + "sizes": "192x192", + "type": "image/png", + "purpose": "maskable" + }, + { + "src": "/web-app-manifest-512x512.png", + "sizes": "512x512", + "type": "image/png", + "purpose": "maskable" + } + ], + "theme_color": "#ffffff", + "background_color": "#ffffff", + "display": "standalone" +} \ No newline at end of file diff --git a/pkgdown/favicon/web-app-manifest-192x192.png b/pkgdown/favicon/web-app-manifest-192x192.png new file mode 100644 index 000000000..8fef3b1d4 Binary files /dev/null and b/pkgdown/favicon/web-app-manifest-192x192.png differ diff --git a/pkgdown/favicon/web-app-manifest-512x512.png b/pkgdown/favicon/web-app-manifest-512x512.png new file mode 100644 index 000000000..558dc0f77 Binary files /dev/null and b/pkgdown/favicon/web-app-manifest-512x512.png differ diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R index b66210eb6..d504070e1 100644 --- a/tests/testthat/helper.R +++ b/tests/testthat/helper.R @@ -62,6 +62,16 @@ skip_if_not_utf8_locale <- function() { testthat::skip_if_not(l10n_info()[["UTF-8"]], "Not a UTF-8 locale") } +safe_load_help_db <- function() { + help_db <- tryCatch(tools::Rd_db("lintr"), error = function(e) NULL) + # e.g. in dev under pkgload::load_all() + if (length(help_db) == 0L) { + help_db <- tryCatch(tools::Rd_db(dir = testthat::test_path("..", "..")), error = function(e) NULL) + testthat::skip_if_not(length(help_db) > 0L, message = "Package help corrupted or not installed") + } + help_db +} + pipes <- function(exclude = NULL) { if (getRversion() < "4.1.0") exclude <- unique(c(exclude, "|>")) all_pipes <- c( diff --git a/tests/testthat/test-class_equals_linter.R b/tests/testthat/test-class_equals_linter.R index fb640e448..cc4495ec3 100644 --- a/tests/testthat/test-class_equals_linter.R +++ b/tests/testthat/test-class_equals_linter.R @@ -11,7 +11,7 @@ test_that("class_equals_linter skips allowed usages", { test_that("class_equals_linter blocks simple disallowed usages", { linter <- class_equals_linter() - lint_msg <- rex::rex("Use inherits(x, 'class-name'), is. or is(x, 'class')") + lint_msg <- rex::rex("Use inherits(x, 'class-name'), is. for S3 classes, or is(x, 'S4Class') for S4 classes") expect_lint("if (class(x) == 'character') stop('no')", lint_msg, linter) expect_lint("is_regression <- class(x) == 'lm'", lint_msg, linter) @@ -20,7 +20,7 @@ test_that("class_equals_linter blocks simple disallowed usages", { test_that("class_equals_linter blocks usage of %in% for checking class", { linter <- class_equals_linter() - lint_msg <- rex::rex("Use inherits(x, 'class-name'), is. or is(x, 'class')") + lint_msg <- rex::rex("Use inherits(x, 'class-name'), is. for S3 classes, or is(x, 'S4Class') for S4 classes") expect_lint("if ('character' %in% class(x)) stop('no')", lint_msg, linter) expect_lint("if (class(x) %in% 'character') stop('no')", lint_msg, linter) @@ -29,7 +29,7 @@ test_that("class_equals_linter blocks usage of %in% for checking class", { test_that("class_equals_linter blocks class(x) != 'klass'", { expect_lint( "if (class(x) != 'character') TRUE", - rex::rex("Use inherits(x, 'class-name'), is. or is(x, 'class')"), + rex::rex("Use inherits(x, 'class-name'), is. for S3 classes, or is(x, 'S4Class') for S4 classes"), class_equals_linter() ) }) @@ -43,13 +43,13 @@ test_that("class_equals_linter skips usage for subsetting", { # but not further nesting expect_lint( "x[if (class(x) == 'foo') 1 else 2]", - rex::rex("Use inherits(x, 'class-name'), is. or is(x, 'class')"), + rex::rex("Use inherits(x, 'class-name'), is. for S3 classes, or is(x, 'S4Class') for S4 classes"), linter ) }) test_that("lints vectorize", { - lint_msg <- rex::rex("Use inherits(x, 'class-name'), is. or is(x, 'class')") + lint_msg <- rex::rex("Use inherits(x, 'class-name'), is. for S3 classes, or is(x, 'S4Class') for S4 classes") expect_lint( trim_some("{ diff --git a/tests/testthat/test-commented_code_linter.R b/tests/testthat/test-commented_code_linter.R index fadc98d5e..5154a5a94 100644 --- a/tests/testthat/test-commented_code_linter.R +++ b/tests/testthat/test-commented_code_linter.R @@ -103,3 +103,13 @@ test_that("commented_code_linter can detect operators in comments and lint corre commented_code_linter() ) }) + +test_that("commented_code_linter can detect commented code ending with pipes", { + linter <- commented_code_linter() + lint_msg <- rex::rex("Remove commented code.") + + expect_lint("# f() %>%", lint_msg, linter) + + skip_if_not_r_version("4.1.0") + expect_lint("# f() |>", lint_msg, linter) +}) diff --git a/tests/testthat/test-expect_lint.R b/tests/testthat/test-expect_lint.R index ea23dce34..f03d7e7a3 100644 --- a/tests/testthat/test-expect_lint.R +++ b/tests/testthat/test-expect_lint.R @@ -30,7 +30,7 @@ test_that("single check", { expect_success(expect_lint("a=1", list(message = lint_msg, line_number = 1L), linter)) expect_failure(expect_lint("a=1", list(2L, lint_msg), linter)) - expect_error(expect_lint("1:nrow(x)", "(group)", seq_linter()), "Invalid regex result", fixed = TRUE) + expect_success(expect_lint("1:nrow(x)", "(nrow)", seq_linter())) }) test_that("multiple checks", { diff --git a/tests/testthat/test-fixed_regex_linter.R b/tests/testthat/test-fixed_regex_linter.R index 98a279a02..83a00c141 100644 --- a/tests/testthat/test-fixed_regex_linter.R +++ b/tests/testthat/test-fixed_regex_linter.R @@ -42,7 +42,7 @@ test_that("fixed_regex_linter blocks simple disallowed usages", { }) patrick::with_parameters_test_that( - "fixed_regex_linter is robust to unrecognized escapes error", + "fixed_regex_linter is robust to unrecognized escapes error for {char}", { expect_lint( sprintf(R"{grep('\\%s', x)}", char), @@ -56,14 +56,11 @@ patrick::with_parameters_test_that( fixed_regex_linter() ) }, - .cases = local({ - char <- c( - "^", "$", "{", "}", "(", ")", ".", "*", "+", "?", - "|", "[", "]", R"(\\)", "<", ">", "=", ":", ";", "/", - "_", "-", "!", "@", "#", "%", "&", "~" - ) - data.frame(char = char, .test_name = char) - }) + char = c( + "^", "$", "{", "}", "(", ")", ".", "*", "+", "?", + "|", "[", "]", R"(\\)", "<", ">", "=", ":", ";", "/", + "_", "-", "!", "@", "#", "%", "&", "~" + ) ) test_that("fixed_regex_linter catches regex like [.] or [$]", { @@ -326,7 +323,7 @@ local({ skip_cases <- character() } patrick::with_parameters_test_that( - "fixed replacements are correct", + "fixed replacements of {regex_expr} with {fixed_expr} is correct", { # TODO(google/patrick#19): handle this more cleanly by skipping up-front skip_if( diff --git a/tests/testthat/test-lint_dir.R b/tests/testthat/test-lint_dir.R index 6da4b81bb..da6dd824c 100644 --- a/tests/testthat/test-lint_dir.R +++ b/tests/testthat/test-lint_dir.R @@ -10,12 +10,6 @@ test_that("lint all files in a directory", { expect_s3_class(lints, "lints") expect_identical(sort(linted_files), sort(files)) - - expect_output( - lint_dir(the_dir, parse_settings = FALSE, show_progress = TRUE), - "======", - fixed = TRUE - ) }) test_that("lint all relevant directories in a package", { diff --git a/tests/testthat/test-linter_tags.R b/tests/testthat/test-linter_tags.R index 2d7f53810..800831fef 100644 --- a/tests/testthat/test-linter_tags.R +++ b/tests/testthat/test-linter_tags.R @@ -103,12 +103,7 @@ test_that("rownames for available_linters data frame doesn't have missing entrie # See the roxygen helpers in R/linter_tags.R for the code used to generate the docs. # This test helps ensure the documentation is up to date with the available_linters() database test_that("lintr help files are up to date", { - help_db <- tools::Rd_db("lintr") - # e.g. in dev under pkgload::load_all() - if (length(help_db) == 0L) { - help_db <- tools::Rd_db(dir = test_path("..", "..")) - skip_if_not(length(help_db) > 0L, message = "Package help not installed or corrupted") - } + help_db <- safe_load_help_db() lintr_db <- available_linters(exclude_tags = NULL) lintr_db$package <- NULL @@ -234,3 +229,7 @@ test_that("available_linters gives precedence to included tags", { available_linters(tags = "deprecated", exclude_tags = NULL) ) }) + +test_that("all linters have at least one tag", { + expect_true(all(lengths(available_linters()$tags) > 0L)) +}) diff --git a/tests/testthat/test-lintr-package.R b/tests/testthat/test-lintr-package.R index e7be3c484..80d668b69 100644 --- a/tests/testthat/test-lintr-package.R +++ b/tests/testthat/test-lintr-package.R @@ -1,5 +1,5 @@ test_that("All linter help files have examples", { - help_db <- tools::Rd_db("lintr") + help_db <- safe_load_help_db() linter_db <- help_db[endsWith(names(help_db), "_linter.Rd")] rd_has_examples <- function(rd) any(vapply(rd, attr, "Rd_tag", FUN.VALUE = character(1L)) == "\\examples") linter_has_examples <- vapply(linter_db, rd_has_examples, logical(1L)) diff --git a/tests/testthat/test-pipe_continuation_linter.R b/tests/testthat/test-pipe_continuation_linter.R index 528f0c011..0d3350bca 100644 --- a/tests/testthat/test-pipe_continuation_linter.R +++ b/tests/testthat/test-pipe_continuation_linter.R @@ -138,46 +138,42 @@ test_that("pipe-continuation linter handles native pipe", { local({ linter <- pipe_continuation_linter() - valid_code <- c( - # all on one line + .cases <- tibble::tribble( + ~code_string, ~.test_name, trim_some(" my_fun <- function() { a %>% b() } - "), + "), "two on one line", trim_some(" my_fun <- function() { a %>% b() %>% c() } - "), + "), "three on one line", trim_some(" with( diamonds, x %>% head(10) %>% tail(5) ) - "), + "), "three inside with()", trim_some(" test_that('blah', { test_data <- diamonds %>% head(10) %>% tail(5) }) - "), - - # two different single-line pipelines + "), "three inside test_that()", trim_some(" { x <- a %>% b %>% c y <- c %>% b %>% a } - "), - - # at most one pipe-character per line + "), "two different single-line pipelines", trim_some(" my_fun <- function() { a %>% b() %>% c() } - ") + "), "at most one pipe-character per line" ) patrick::with_parameters_test_that( "valid nesting is handled", @@ -185,8 +181,7 @@ local({ { expect_lint(code_string, NULL, linter) }, - .test_name = valid_code, - code_string = valid_code + .cases = .cases ) }) diff --git a/tests/testthat/test-return_linter.R b/tests/testthat/test-return_linter.R index b05b3abd3..58a5d1cf9 100644 --- a/tests/testthat/test-return_linter.R +++ b/tests/testthat/test-return_linter.R @@ -691,6 +691,20 @@ test_that("except_regex= argument works", { list(rex::rex("All functions must have an explicit return()."), line_number = 5L), linter ) + + # capture group doesn't cause issues, #2678 + expect_lint( + trim_some(" + TestFun <- function() { + non_return() + } + AssertFun <- function() { + non_return() + } + "), + NULL, + return_linter(return_style = "explicit", except_regex = "^(Test|Assert)") + ) }) test_that("except= and except_regex= combination works", { diff --git a/tests/testthat/test-todo_comment_linter.R b/tests/testthat/test-todo_comment_linter.R index 103f9c8fc..b7793d1d8 100644 --- a/tests/testthat/test-todo_comment_linter.R +++ b/tests/testthat/test-todo_comment_linter.R @@ -57,4 +57,11 @@ test_that("except_regex= excludes valid TODO", { NULL, todo_comment_linter(except_regex = c("TODO\\(#[0-9]+\\):", "fixme\\(#[0-9]+\\):")) ) + + # ignore captured groups + expect_lint( + "# TODO(a)", + NULL, + todo_comment_linter(except_regex = "TODO\\((a|abc)\\)") + ) }) diff --git a/tests/testthat/test-unreachable_code_linter.R b/tests/testthat/test-unreachable_code_linter.R index 167909d50..b54d3b11e 100644 --- a/tests/testthat/test-unreachable_code_linter.R +++ b/tests/testthat/test-unreachable_code_linter.R @@ -687,6 +687,19 @@ test_that("allow_comment_regex= works", { NULL, linter_x1x2 ) + + # might contain capture groups, #2678 + expect_lint( + trim_some(" + function() { + stop('a') + # a + # ab + } + "), + NULL, + unreachable_code_linter(allow_comment_regex = "#\\s*(a|ab|abc)") + ) }) test_that("allow_comment_regex= obeys covr's custom exclusion when set", {