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

Fix parsing usage warnings from codetools without location info #1993

Merged
merged 7 commits into from
Jul 2, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
* `assignment_linter()` no longer lints assignments in braces that include comments when `allow_trailing = FALSE` (#1701, @ashbaldry)

* `object_usage_linter()`
+ No longer silently ignores usage warnings that don't contain a quoted name (#1714, @AshesITR)
+ No longer fails on code with comments inside a multi-line call to `glue::glue()` (#1919, @MichaelChirico)
+ No longer silently ignores usage warnings that don't contain a quoted name (#1714, @AshesITR)
MichaelChirico marked this conversation as resolved.
Show resolved Hide resolved
+ No longer fails on code with comments inside a multi-line call to `glue::glue()` (#1919, @MichaelChirico)

* `namespace_linter()` correctly recognizes backticked operators to be exported from respective namespaces (like `` rlang::`%||%` ``) (#1752, @IndrajeetPatil)

Expand All @@ -40,7 +40,9 @@

* `object_name_linter()` allows all S3 group Generics (see `?base::groupGeneric`) and S3 generics defined in a different file in the same package (#1808, #1841, @AshesITR)

* `object_usage_linter()` improves identification of the exact source of a lint for undefined variables in expressions with where the variable is used as a symbol in a usual way, for example in a formula or in an extraction with `$` (#1914, @MichaelChirico).
* `object_usage_linter()` improves identification of the exact source of a lint
+ for undefined variables in expressions with where the variable is used as a symbol in a usual way, for example in a formula or in an extraction with `$` (#1914, @MichaelChirico).
MichaelChirico marked this conversation as resolved.
Show resolved Hide resolved
+ for general usage warnings without location info (#1986, @AshesITR)

* `function_left_parentheses_linter()` produces a more specific lint (and no longer fails) when the opening parenthesis is on a different line than `function` or the call name (#1953, @MichaelChirico). Thanks also to @IndrajeetPatil and @lorenzwalthert for identifying a regression in the initial fix, #1963.

Expand Down
10 changes: 7 additions & 3 deletions R/object_usage_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,10 @@ parse_check_usage <- function(expression,
"'",
capture(name = "name", anything),
"'",
anything
zero_or_more(any, type = "lazy")
)
),
line_info
or(line_info, end)
)
)

Expand All @@ -301,7 +301,11 @@ parse_check_usage <- function(expression,
# nocov end
res <- res[!missing, ]

res$line1 <- as.integer(res$line1) + start_line - 1L
res$line1 <- ifelse(
nzchar(res$line1),
as.integer(res$line1) + start_line - 1L,
start_line
)
res$line2 <- ifelse(
nzchar(res$line2),
as.integer(res$line2) + start_line - 1L,
Expand Down
39 changes: 39 additions & 0 deletions tests/testthat/test-object_usage_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -765,3 +765,42 @@ test_that("functional lambda definitions are also caught", {
object_usage_linter()
)
})

test_that("messages without location info are repaired", {
AshesITR marked this conversation as resolved.
Show resolved Hide resolved
# regression test for #1986
expect_lint(
trim_some("
foo <- function() no_fun()
"),
list(
message = rex::rex("no visible global function definition for", anything),
line_number = 1L,
column_number = 19L
),
object_usage_linter()
)

expect_lint(
trim_some("
foo <- function() no_global
"),
list(
message = rex::rex("no visible binding for global variable", anything),
line_number = 1L,
column_number = 19L
),
object_usage_linter()
)

expect_lint(
trim_some("
foo <- function() unused_local <- 42L
"),
list(
message = rex::rex("local variable", anything, "assigned but may not be used"),
line_number = 1L,
column_number = 19L
),
object_usage_linter()
)
})