Skip to content

Commit

Permalink
Merge branch 'main' into f2599-cli-progress-bar
Browse files Browse the repository at this point in the history
  • Loading branch information
IndrajeetPatil committed Dec 2, 2024
2 parents f1b8b30 + b7cd0f6 commit b1d8083
Show file tree
Hide file tree
Showing 31 changed files with 2,839 additions and 112 deletions.
2 changes: 2 additions & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@
^bench$
^tests/testthat/dummy_packages/package/[.]Rbuildignore$
^tests/testthat/dummy_packages/cp1252/[.]Rbuildignore$
testthat-problems[.]rds$
^_pkgdown\.yaml$
^docs$
^pkgdown$
^vignettes/[^-]+.gif$
^CRAN-SUBMISSION$
^CODE_OF_CONDUCT\.md$
^paper$
30 changes: 30 additions & 0 deletions .github/workflows/draft-pdf.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# TODO: delete this file once the paper is published
on:
push:
branches: [main]
pull_request:
branches: [main]

jobs:
paper:
runs-on: ubuntu-latest
name: Paper Draft
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Build draft PDF
uses: openjournals/openjournals-draft-action@master
with:
journal: joss
# This should be the path to the paper within your repo.
paper-path: paper/paper.md

- name: Upload
uses: actions/upload-artifact@v4
with:
name: paper
# This is the output path where Pandoc will write the compiled
# PDF. Note, this should be the same directory as the input
# paper.md
path: paper/paper.pdf
2 changes: 1 addition & 1 deletion .github/workflows/pkgdown.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.1
with:
clean: false
branch: gh-pages
Expand Down
6 changes: 6 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
* `cyclocomp_linter()` is no longer part of the default linters (#2555, @IndrajeetPatil) because the tidyverse style guide doesn't contain any guidelines on meeting certain complexity requirements. Note that users with `cyclocomp_linter()` in their configs may now need to install {cyclocomp} intentionally, in particular in CI/CD pipelines.
* `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

Expand All @@ -27,6 +29,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

Expand Down Expand Up @@ -84,10 +87,13 @@
### Lint accuracy fixes: removing false positives

* `object_name_linter()` and `object_length_linter()` ignore {rlang} name injection like `x |> mutate("{new_name}" := foo(col))` (#1926, @MichaelChirico). No checking is applied in such cases. {data.table} in-place assignments like `DT[, "sPoNGeBob" := "friend"]` are still eligible for lints.
* `object_usage_linter()` finds global variables assigned with `=` or `->`, which avoids some issues around "undefined global variables" in scripts (#2654, @MichaelChirico).

## Notes

* All user-facing messages (including progress bar) are now prepared using the `{cli}` package (#2418 and #2641, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent.
* 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.
* 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).

# lintr 3.1.2
Expand Down
7 changes: 4 additions & 3 deletions R/class_equals_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -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.<class> or is(x, 'class') instead of comparing class(x) with %s.",
operator
lint_message <- paste0(
"Use inherits(x, 'class-name'), is.<class> for S3 classes, ",
"or is(x, 'S4Class') for S4 classes, ",
"instead of comparing class(x) with ", operator, "."
)
xml_nodes_to_lints(
bad_expr,
Expand Down
5 changes: 3 additions & 2 deletions R/commented_code_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion R/extract.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions R/get_source_expressions.R
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion R/lint.R
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ has_positional_logical <- function(dots) {
}

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

Expand Down
42 changes: 22 additions & 20 deletions R/methods.R
Original file line number Diff line number Diff line change
@@ -1,27 +1,16 @@
#' @export
format.lint <- function(x, ..., width = getOption("lintr.format_width")) {
if (requireNamespace("cli", quietly = TRUE)) {
color <- switch(x$type,
warning = cli::col_magenta,
error = cli::col_red,
style = cli::col_blue,
cli::style_bold
)
emph <- cli::style_bold
} else {
# nocov start
color <- identity
emph <- identity
# nocov end
}
color <- switch(x$type,
warning = cli::col_magenta,
error = cli::col_red,
style = cli::col_blue,
cli::style_bold
)
emph <- cli::style_bold

line_ref <- build_line_ref(x)
annotated_msg <- paste0(
emph(
x$filename, ":",
as.character(x$line_number), ":",
as.character(x$column_number), ": ",
sep = ""
),
emph(line_ref, ": "),
color(x$type, ": ", sep = ""),
"[", x$linter, "] ",
emph(x$message)
Expand All @@ -40,6 +29,19 @@ format.lint <- function(x, ..., width = getOption("lintr.format_width")) {
)
}

build_line_ref <- function(x) {
line_ref <- paste0(
x$filename, ":",
as.character(x$line_number), ":",
as.character(x$column_number)
)

if (!cli::ansi_has_hyperlink_support()) {
return(line_ref)
}
cli::format_inline("{.path {line_ref}}")
}

#' @export
print.lint <- function(x, ...) {
cat(format(x, ...))
Expand Down
4 changes: 3 additions & 1 deletion R/object_usage_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,10 @@ get_assignment_symbols <- function(xml) {
get_r_string(xml_find_all(
xml,
"
expr[LEFT_ASSIGN]/expr[1]/SYMBOL[1] |
expr[LEFT_ASSIGN or EQ_ASSIGN]/expr[1]/SYMBOL[1] |
expr[RIGHT_ASSIGN]/expr[2]/SYMBOL[1] |
equal_assign/expr[1]/SYMBOL[1] |
expr_or_assign_or_help/expr[1]/SYMBOL[1] |
expr[expr[1][SYMBOL_FUNCTION_CALL/text()='assign']]/expr[2]/* |
expr[expr[1][SYMBOL_FUNCTION_CALL/text()='setMethod']]/expr[2]/*
"
Expand Down
11 changes: 11 additions & 0 deletions R/redundant_equals_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
#' linters = redundant_equals_linter()
#' )
#'
#' lint(
#' text = "dt[is_tall == FALSE, y]",
#' linters = redundant_equals_linter()
#' )
#'
#' # okay
#' lint(
#' text = "if (any(x)) 1",
Expand All @@ -30,6 +35,12 @@
#' linters = redundant_equals_linter()
#' )
#'
#' # in `{data.table}` semantics, `dt[x]` is a join, `dt[(x)]` is a subset
#' lint(
#' text = "dt[(!is_tall), y]",
#' linters = redundant_equals_linter()
#' )
#'
#' @evalRd rd_tags("redundant_equals_linter")
#' @seealso
#' - [linters] for a complete list of linters available in lintr.
Expand Down
3 changes: 2 additions & 1 deletion R/seq_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
#' Additionally, it checks for `1:n()` (from `{dplyr}`) and `1:.N` (from `{data.table}`).
#'
#' These often cause bugs when the right-hand side is zero.
#' It is safer to use [base::seq_len()] or [base::seq_along()] instead.
#' Instead, it is safer to use [base::seq_len()] (to create a sequence of a specified *length*) or
#' [base::seq_along()] (to create a sequence *along* an object).
#'
#' @examples
#' # will produce lints
Expand Down
5 changes: 4 additions & 1 deletion R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,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
Expand Down
6 changes: 3 additions & 3 deletions inst/lintr/linters.csv
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ length_test_linter,common_mistakes efficiency
lengths_linter,efficiency readability best_practices
library_call_linter,style best_practices readability configurable
line_length_linter,style readability default configurable
list_comparison_linter,best_practices common_mistakes efficiency
list_comparison_linter,best_practices common_mistakes
literal_coercion_linter,best_practices consistency efficiency
matrix_apply_linter,readability efficiency
missing_argument_linter,correctness common_mistakes configurable
Expand Down Expand Up @@ -108,8 +108,8 @@ terminal_close_linter,best_practices robustness
todo_comment_linter,style configurable
trailing_blank_lines_linter,style default
trailing_whitespace_linter,style default configurable
undesirable_function_linter,style efficiency configurable robustness best_practices
undesirable_operator_linter,style efficiency configurable robustness best_practices
undesirable_function_linter,style configurable robustness best_practices
undesirable_operator_linter,style configurable robustness best_practices
unnecessary_concatenation_linter,style readability efficiency configurable
unnecessary_lambda_linter,best_practices efficiency readability configurable
unnecessary_nested_if_linter,readability best_practices deprecated
Expand Down
3 changes: 0 additions & 3 deletions man/efficiency_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions man/linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/list_comparison_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions man/redundant_equals_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion man/seq_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/undesirable_function_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/undesirable_operator_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions paper/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/.quarto/
Loading

0 comments on commit b1d8083

Please sign in to comment.