Skip to content

Parse settings prior to reading file to ensure Encoding is set #2805

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

Merged
merged 13 commits into from
Mar 4, 2025
Merged
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,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

Expand Down
27 changes: 16 additions & 11 deletions R/lint.R
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ 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 (parse_settings) {
read_settings(filename)
on.exit(reset_settings(), add = TRUE)
}

lines <- get_lines(filename, text)

if (needs_tempfile) {
Expand All @@ -58,11 +65,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))

Expand All @@ -81,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),
Expand Down Expand Up @@ -709,3 +706,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
}
}
41 changes: 21 additions & 20 deletions tests/testthat/test-get_source_expressions.R
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,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"]])) {
Expand All @@ -125,17 +122,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", {
Expand Down
Loading