From 88ad5ea431e471c7f3356b150cd004d285e51341 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Fri, 28 Feb 2025 23:32:40 +0000 Subject: [PATCH 1/6] adjustments to read settings up front --- R/lint.R | 11 ++++++----- tests/testthat/test-methods.R | 6 +----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/R/lint.R b/R/lint.R index e3200b6ba..f45dcb107 100644 --- a/R/lint.R +++ b/R/lint.R @@ -45,6 +45,12 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings = needs_tempfile <- missing(filename) || re_matches(filename, rex(newline)) inline_data <- !is.null(text) || needs_tempfile + + if (!inline_data && isTRUE(parse_settings)) { + read_settings(filename) + on.exit(reset_settings(), add = TRUE) + } + lines <- get_lines(filename, text) if (needs_tempfile) { @@ -58,11 +64,6 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings = filename <- normalize_path(filename, mustWork = !inline_data) # to ensure a unique file in cache source_expressions <- get_source_expressions(filename, lines) - if (isTRUE(parse_settings)) { - read_settings(filename) - on.exit(reset_settings(), add = TRUE) - } - linters <- define_linters(linters) linters <- Map(validate_linter_object, linters, names(linters)) diff --git a/tests/testthat/test-methods.R b/tests/testthat/test-methods.R index 91a04cafc..ddc79dc90 100644 --- a/tests/testthat/test-methods.R +++ b/tests/testthat/test-methods.R @@ -178,9 +178,6 @@ local({ lints <- lint(text = "a", linters = test_linter()) lint <- lints[[1L]] - widths <- c(10L, 20L, 40L, 80L) - test_names <- paste0(": width = ", widths) - patrick::with_parameters_test_that( "print.lint, print.lints support optional message wrapping", { @@ -190,8 +187,7 @@ local({ expect_snapshot(print(lints)) }) }, - .test_name = test_names, - width = widths + width = c(10L, 20L, 40L, 80L) ) wrapped_strings <- c( From 40cff570a5170ff8db6f468ddeef80d74c32a08e Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 3 Mar 2025 20:27:00 +0000 Subject: [PATCH 2/6] fix --- tests/testthat/test-methods.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-methods.R b/tests/testthat/test-methods.R index ddc79dc90..24624a450 100644 --- a/tests/testthat/test-methods.R +++ b/tests/testthat/test-methods.R @@ -177,6 +177,7 @@ local({ lints <- lint(text = "a", linters = test_linter()) lint <- lints[[1L]] + widths <- c(10L, 20L, 40L, 80L) patrick::with_parameters_test_that( "print.lint, print.lints support optional message wrapping", @@ -187,7 +188,7 @@ local({ expect_snapshot(print(lints)) }) }, - width = c(10L, 20L, 40L, 80L) + width = widths ) wrapped_strings <- c( From 1f4b628a1eb49976838f9a023670133f7e28a98f Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 3 Mar 2025 20:28:47 +0000 Subject: [PATCH 3/6] avoid tinkering with snapshots --- tests/testthat/test-methods.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/testthat/test-methods.R b/tests/testthat/test-methods.R index 24624a450..91a04cafc 100644 --- a/tests/testthat/test-methods.R +++ b/tests/testthat/test-methods.R @@ -177,7 +177,9 @@ local({ lints <- lint(text = "a", linters = test_linter()) lint <- lints[[1L]] + widths <- c(10L, 20L, 40L, 80L) + test_names <- paste0(": width = ", widths) patrick::with_parameters_test_that( "print.lint, print.lints support optional message wrapping", @@ -188,6 +190,7 @@ local({ expect_snapshot(print(lints)) }) }, + .test_name = test_names, width = widths ) From ca4b49a889cc29f0124cc5bc6b5766337feb7305 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 3 Mar 2025 20:41:24 +0000 Subject: [PATCH 4/6] tested: remove ':::' from tests --- tests/testthat/test-get_source_expressions.R | 41 ++++++++++---------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/tests/testthat/test-get_source_expressions.R b/tests/testthat/test-get_source_expressions.R index aaf1c8f7c..5bc564547 100644 --- a/tests/testthat/test-get_source_expressions.R +++ b/tests/testthat/test-get_source_expressions.R @@ -104,17 +104,14 @@ test_that("Multi-byte character truncated by parser is ignored", { }) test_that("Can read non UTF-8 file", { - withr::local_options(lintr.linter_file = tempfile()) - file <- test_path("dummy_projects", "project", "cp1252.R") - lintr:::read_settings(file) - expect_null(get_source_expressions(file)$error) + proj_dir <- test_path("dummy_projects", "project") + withr::local_dir(proj_dir) + expect_no_lint(file = "cp1252.R", linters = list()) }) -test_that("Warns if encoding is misspecified", { - file <- test_path("dummy_projects", "project", "cp1252.R") - lintr:::read_settings(NULL) - the_lint <- lint(filename = file, parse_settings = FALSE)[[1L]] - expect_s3_class(the_lint, "lint") +test_that("Warns if encoding is misspecified, Pt. 1", { + proj_dir <- test_path("dummy_projects", "project") + withr::local_dir(proj_dir) lint_msg <- "Invalid multibyte character in parser. Is the encoding correct?" if (!isTRUE(l10n_info()[["UTF-8"]])) { @@ -124,17 +121,21 @@ test_that("Warns if encoding is misspecified", { lint_msg <- "unexpected '<'" } - expect_identical(the_lint$linter, "error") - expect_identical(the_lint$message, lint_msg) - expect_identical(the_lint$line_number, 4L) - - file <- test_path("dummy_projects", "project", "cp1252_parseable.R") - lintr:::read_settings(NULL) - the_lint <- lint(filename = file, parse_settings = FALSE)[[1L]] - expect_s3_class(the_lint, "lint") - expect_identical(the_lint$linter, "error") - expect_identical(the_lint$message, "Invalid multibyte string. Is the encoding correct?") - expect_identical(the_lint$line_number, 1L) + expect_lint( + file = "cp1252.R", + parse_settings = FALSE, + checks = list(rex::rex(lint_msg), linter = "error", line_number = 4L) + ) + + expect_lint( + file = "cp1252_parseable.R", + parse_settings = FALSE, + checks = list( + rex::rex("Invalid multibyte string. Is the encoding correct?"), + linter = "error", + line_number = 1L + ) + ) }) test_that("Can extract line number from parser errors", { From 026255476044c6f43ab0f484424dfd4cb05a90b5 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 3 Mar 2025 20:50:52 +0000 Subject: [PATCH 5/6] cyclocomp massage --- R/lint.R | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/R/lint.R b/R/lint.R index f45dcb107..f40f4069c 100644 --- a/R/lint.R +++ b/R/lint.R @@ -45,8 +45,9 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings = needs_tempfile <- missing(filename) || re_matches(filename, rex(newline)) inline_data <- !is.null(text) || needs_tempfile + parse_settings <- !inline_data && isTRUE(parse_settings) - if (!inline_data && isTRUE(parse_settings)) { + if (parse_settings) { read_settings(filename) on.exit(reset_settings(), add = TRUE) } @@ -82,12 +83,7 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings = lints <- list() if (!is_tainted(source_expressions$lines)) { for (expr in source_expressions$expressions) { - if (is_lint_level(expr, "expression")) { - necessary_linters <- expression_linter_names - } else { - necessary_linters <- file_linter_names - } - for (linter in necessary_linters) { + for (linter in necessary_linters(expr, expression_linter_names, file_linter_names)) { # use withCallingHandlers for friendlier failures on unexpected linter errors lints[[length(lints) + 1L]] <- withCallingHandlers( get_lints(expr, linter, linters[[linter]], lint_cache, source_expressions$lines), @@ -701,3 +697,11 @@ zap_temp_filename <- function(res, needs_tempfile) { } res } + +necessary_linters <- function(expr, expression_linter_names, file_linter_names) { + if (is_lint_level(expr, "expression")) { + expression_linter_names + } else { + file_linter_names + } +} From e2931f51477f2cd39346116552eb99d3092eeddc Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 3 Mar 2025 22:41:22 +0000 Subject: [PATCH 6/6] Add NEWS --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 24053f883..9bfb8fd62 100644 --- a/NEWS.md +++ b/NEWS.md @@ -14,6 +14,7 @@ ## Bug fixes * `Lint()`, and thus all linters, ensures that the returned object's `message` attribute is consistently a simple character string (and not, for example, an object of class `"glue"`; #2740, @MichaelChirico). +* Files with encoding inferred from settings read more robustly under `lint(parse_settings = TRUE)` (#2803, @MichaelChirico). ## New and improved features