Skip to content

Commit

Permalink
Emphasize S3-S4 difference in recommendation for how to fix class_equ…
Browse files Browse the repository at this point in the history
…als_linter() (#2688)

* Emphasize S3-S4 difference in recommendation

* Update in tests

* One more
  • Loading branch information
MichaelChirico authored Nov 26, 2024
1 parent 9d58027 commit 5c72a1c
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 8 deletions.
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
10 changes: 5 additions & 5 deletions tests/testthat/test-class_equals_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -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.<class> or is(x, 'class')")
lint_msg <- rex::rex("Use inherits(x, 'class-name'), is.<class> 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)
Expand All @@ -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.<class> or is(x, 'class')")
lint_msg <- rex::rex("Use inherits(x, 'class-name'), is.<class> 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)
Expand All @@ -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.<class> or is(x, 'class')"),
rex::rex("Use inherits(x, 'class-name'), is.<class> for S3 classes, or is(x, 'S4Class') for S4 classes"),
class_equals_linter()
)
})
Expand All @@ -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.<class> or is(x, 'class')"),
rex::rex("Use inherits(x, 'class-name'), is.<class> 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.<class> or is(x, 'class')")
lint_msg <- rex::rex("Use inherits(x, 'class-name'), is.<class> for S3 classes, or is(x, 'S4Class') for S4 classes")

expect_lint(
trim_some("{
Expand Down

0 comments on commit 5c72a1c

Please sign in to comment.