a near-complete pass

tests/testthat/test-commas_linter.R

@@ -1,7 +1,7 @@
 test_that("returns the correct linting (with default parameters)", {
   linter <- commas_linter()
-  msg_after <- rex::rex("Commas should always have a space after.")
-  msg_before <- rex::rex("Commas should never have a space before.")
+  msg_after <- rex::rex("Commas should always precede a space.")
+  msg_before <- rex::rex("Commas should never follow a space.")
 
   expect_lint("blah", NULL, linter)
   expect_lint("fun(1, 1)", NULL, linter)
@@ -52,8 +52,8 @@ test_that("returns the correct linting (with default parameters)", {
 
 test_that("returns the correct linting (with 'allow_trailing' set)", {
   linter <- commas_linter(allow_trailing = TRUE)
-  msg_after <- rex::rex("Commas should always have a space after.")
-  msg_before <- rex::rex("Commas should never have a space before.")
+  msg_after <- rex::rex("Commas should always precede a space.")
+  msg_before <- rex::rex("Commas should never follow a space.")
 
   expect_lint("blah", NULL, linter)
   expect_lint("fun(1, 1)", NULL, linter)

tests/testthat/test-commented_code_linter.R

@@ -20,7 +20,7 @@ test_that("commented_code_linter skips allowed usages", {
 })
 
 test_that("commented_code_linter blocks disallowed usages", {
-  lint_msg <- rex::rex("Commented code should be removed.")
+  lint_msg <- rex::rex("Remove commented code.")
   linter <- commented_code_linter()
 
   expect_lint("# blah <- 1", lint_msg, linter)
@@ -80,7 +80,7 @@ test_that("commented_code_linter blocks disallowed usages", {
 
 test_that("commented_code_linter can detect operators in comments and lint correctly", {
   linter <- commented_code_linter()
-  lint_msg <- rex::rex("Commented code should be removed.")
+  lint_msg <- rex::rex("Remove commented code.")
 
   test_ops <- c(
     "+", "=", "==", "!=", "<=", ">=", "<-", "<<-", "<", ">", "->",
@@ -100,7 +100,7 @@ test_that("commented_code_linter can detect operators in comments and lint corre
 
   expect_lint(
     "# 1:3 |> sum()",
-    rex::rex("Commented code should be removed."),
+    rex::rex("Remove commented code."),
     commented_code_linter()
   )
 })

tests/testthat/test-condition_call_linter.R

@@ -20,13 +20,13 @@ patrick::with_parameters_test_that(
   "condition_call_linter blocks disallowed usages",
   {
     linter <- condition_call_linter()
-    lint_message <- rex::rex(call_name, anything, "to not display call")
+    lint_message <- rex::rex(call_name, anything, "not to display the call")
 
     expect_lint(paste0(call_name, "('test')"), lint_message, linter)
     expect_lint(paste0(call_name, "('test', call. = TRUE)"), lint_message, linter)
 
     linter <- condition_call_linter(display_call = TRUE)
-    lint_message <- rex::rex(call_name, anything, "to display call")
+    lint_message <- rex::rex(call_name, anything, "to display the call")
 
     expect_lint(paste0(call_name, "('test', call. = FALSE)"), lint_message, linter)
 

tests/testthat/test-conjunct_test_linter.R

@@ -19,22 +19,16 @@ test_that("conjunct_test_linter skips allowed usages of expect_true", {
 })
 
 test_that("conjunct_test_linter blocks && conditions with expect_true()", {
-  expect_lint(
-    "expect_true(x && y)",
-    rex::rex("Instead of expect_true(A && B), write multiple expectations"),
-    conjunct_test_linter()
-  )
+  linter <- conjunct_test_linter()
+  lint_msg <- rex::rex("Write multiple expectations like expect_true(A) and expect_true(B) instead of expect_true(A && B)")
 
-  expect_lint(
-    "expect_true(x && y && z)",
-    rex::rex("Instead of expect_true(A && B), write multiple expectations"),
-    conjunct_test_linter()
-  )
+  expect_lint("expect_true(x && y)", lint_msg, linter)
+  expect_lint("expect_true(x && y && z)", lint_msg, linter)
 })
 
 test_that("conjunct_test_linter blocks || conditions with expect_false()", {
   linter <- conjunct_test_linter()
-  lint_msg <- rex::rex("Instead of expect_false(A || B), write multiple expectations")
+  lint_msg <- rex::rex("Write multiple expectations like expect_false(A) and expect_false(B) instead of expect_false(A || B)")
 
   expect_lint("expect_false(x || y)", lint_msg, linter)
   expect_lint("expect_false(x || y || z)", lint_msg, linter)
@@ -59,7 +53,7 @@ test_that("conjunct_test_linter skips allowed stopifnot() and assert_that() usag
 
 test_that("conjunct_test_linter blocks simple disallowed usages of stopifnot() and assert_that()", {
   linter <- conjunct_test_linter()
-  lint_msg <- function(fun) rex::rex("Instead of ", fun, "(A && B), write multiple conditions")
+  lint_msg <- function(fun) rex::rex("Write multiple conditions like ", fun, "(A, B) instead of ", fun, "(A && B)")
 
   expect_lint("stopifnot(x && y)", lint_msg("stopifnot"), linter)
   expect_lint("stopifnot(x && y && z)", lint_msg("stopifnot"), linter)
@@ -78,7 +72,7 @@ test_that("conjunct_test_linter's allow_named_stopifnot argument works", {
   )
   expect_lint(
     "stopifnot('x is a logical scalar' = length(x) == 1 && is.logical(x) && !is.na(x))",
-    rex::rex("Instead of stopifnot(A && B), write multiple conditions"),
+    rex::rex("Write multiple conditions like stopifnot(A, B)"),
     conjunct_test_linter(allow_named_stopifnot = FALSE)
   )
 })

tests/testthat/test-cyclocomp_linter.R

@@ -1,7 +1,7 @@
 test_that("returns the correct linting", {
   cc_linter_1 <- cyclocomp_linter(1L)
   cc_linter_2 <- cyclocomp_linter(2L)
-  lint_msg <- rex::rex("Functions should have cyclomatic complexity")
+  lint_msg <- rex::rex("Reduce the cyclomatic complexity of this function")
 
   expect_lint("if (TRUE) 1 else 2", NULL, cc_linter_2)
   expect_lint("if (TRUE) 1 else 2", lint_msg, cc_linter_1)
@@ -40,7 +40,7 @@ test_that("returns the correct linting", {
   expect_lint(complex_lines, lint_msg, cc_linter_2)
   expect_lint(
     complex_lines,
-    "should have cyclomatic complexity of less than 2, this has 10",
+    rex::rex("Reduce the cyclomatic complexity of this function from 10 to at most 2."),
     cc_linter_2
   )
   expect_lint(complex_lines, NULL, cyclocomp_linter(10L))

tests/testthat/test-duplicate_argument_linter.R

@@ -11,7 +11,7 @@ test_that("duplicate_argument_linter doesn't block allowed usages", {
 
 test_that("duplicate_argument_linter blocks disallowed usages", {
   linter <- duplicate_argument_linter()
-  lint_msg <- rex::rex("Duplicate arguments in function call.")
+  lint_msg <- rex::rex("Remove duplicate arguments in function call.")
 
   expect_lint("fun(arg = 1, arg = 2)", lint_msg, linter)
   expect_lint("fun(arg = 1, 'arg' = 2)", lint_msg, linter)
@@ -51,13 +51,13 @@ test_that("duplicate_argument_linter respects except argument", {
     "fun(`
 ` = 1, `
 ` = 2)",
-    list(message = rex::rex("Duplicate arguments in function call.")),
+    rex::rex("Remove duplicate arguments in function call."),
     duplicate_argument_linter(except = character())
   )
 
   expect_lint(
     "function(arg = 1, arg = 1) {}",
-    list(message = rex::rex("Repeated formal argument 'arg'.")),
+    rex::rex("Repeated formal argument 'arg'."),
     duplicate_argument_linter(except = character())
   )
 })

tests/testthat/test-fixed_regex_linter.R

@@ -245,16 +245,16 @@ test_that("fixed replacements vectorize and recognize str_detect", {
       )
     "),
     list(
-      rex::rex('Here, you can use "abcdefg" with fixed = TRUE'),
-      rex::rex('Here, you can use "a..b\\n" with fixed = TRUE')
+      rex::rex('Use "abcdefg" with fixed = TRUE'),
+      rex::rex('Use "a..b\\n" with fixed = TRUE')
     ),
     linter
   )
 
   # stringr hint works
   expect_lint(
     "str_detect(x, 'abc')",
-    rex::rex('Here, you can use stringr::fixed("abc") as the pattern'),
+    rex::rex('Use stringr::fixed("abc") as the pattern'),
     linter
   )
 })
@@ -267,7 +267,7 @@ test_that("fixed replacement is correct with UTF-8", {
 
   expect_lint(
     "grepl('[\\U{1D4D7}]', x)",
-    rex::rex('Here, you can use "\U1D4D7" with fixed = TRUE'),
+    rex::rex('Use "\U1D4D7" with fixed = TRUE'),
     fixed_regex_linter()
   )
 })
@@ -311,7 +311,7 @@ patrick::with_parameters_test_that("fixed replacements are correct", {
   )
   expect_lint(
     sprintf("grepl('%s', x)", regex_expr),
-    rex::rex(sprintf('Here, you can use "%s" with fixed = TRUE', fixed_expr)),
+    rex::rex(sprintf('Use "%s" with fixed = TRUE', fixed_expr)),
     fixed_regex_linter()
   )
 }, .cases = tibble::tribble(
@@ -354,7 +354,7 @@ test_that("'unescaped' regex can optionally be skipped", {
 
   expect_lint("grepl('a', x)", NULL, linter)
   expect_lint("str_detect(x, 'a')", NULL, linter)
-  expect_lint("grepl('[$]', x)", rex::rex('Here, you can use "$"'), linter)
+  expect_lint("grepl('[$]', x)", rex::rex('Use "$" with fixed = TRUE'), linter)
 })
 
 local({

tests/testthat/test-if_not_else_linter.R

@@ -20,7 +20,7 @@ test_that("if_not_else_linter skips allowed usages", {
 
 test_that("if_not_else_linter blocks simple disallowed usages", {
   linter <- if_not_else_linter()
-  lint_msg <- rex::rex("In a simple if/else statement, prefer `if (A) x else y`")
+  lint_msg <- rex::rex("Prefer `if (A) x else y`")
 
   expect_lint("if (!A) x else y", lint_msg, linter)
   expect_lint("if (!A) x else if (!B) y else z", lint_msg, linter)
@@ -65,7 +65,7 @@ test_that("multiple lints are generated correctly", {
         if_else(!A, x, y)
     }"),
     list(
-      "In a simple if/else statement",
+      rex::rex("Prefer `if (A) x else y`"),
       "Prefer `ifelse",
       "Prefer `fifelse",
       "Prefer `if_else"
@@ -77,7 +77,7 @@ test_that("multiple lints are generated correctly", {
 test_that("exceptions= argument works", {
   expect_lint(
     "if (!is.null(x)) x else y",
-    "In a simple if/else statement",
+    rex::rex("Prefer `if (A) x else y`"),
     if_not_else_linter(exceptions = character())
   )
 

tests/testthat/test-implicit_integer_linter.R

@@ -48,15 +48,15 @@ local({
   linter <- implicit_integer_linter()
   patrick::with_parameters_test_that(
     "single numerical constants are properly identified ",
-    expect_lint(num_value_str, if (should_lint) "Integers should not be implicit", linter),
+    expect_lint(num_value_str, if (should_lint) "Avoid implicit integers", linter),
     .cases = cases
   )
 })
 # styler: on
 
 test_that("linter returns the correct linting", {
-  lint_msg <- "Integers should not be implicit. Use the form 1L for integers or 1.0 for doubles."
   linter <- implicit_integer_linter()
+  lint_msg <- rex::rex("Avoid implicit integers. Make the type explicit by using the form 1L for integers or 1.0 for doubles.")
 
   expect_lint("x <<- 1L", NULL, linter)
   expect_lint("1.0/-Inf -> y", NULL, linter)
@@ -90,7 +90,7 @@ patrick::with_parameters_test_that(
   "numbers in a:b input are optionally not linted",
   expect_lint(
     paste0(left, ":", right),
-    if (n_lints > 0L) rep(list("Integers should not be implicit"), n_lints),
+    if (n_lints > 0L) rep(list("Avoid implicit integers"), n_lints),
     implicit_integer_linter(allow_colon = allow_colon)
   ),
   .cases = tibble::tribble(

tests/testthat/test-is_numeric_linter.R

@@ -1,22 +1,26 @@
 test_that("is_numeric_linter skips allowed usages involving ||", {
-  expect_lint("is.numeric(x) || is.integer(y)", NULL, is_numeric_linter())
+  linter <- is_numeric_linter()
+
+  expect_lint("is.numeric(x) || is.integer(y)", NULL, linter)
   # x is used, but not identically
-  expect_lint("is.numeric(x) || is.integer(foo(x))", NULL, is_numeric_linter())
+  expect_lint("is.numeric(x) || is.integer(foo(x))", NULL, linter)
   # not totally crazy, e.g. if input accepts a vector or a list
-  expect_lint("is.numeric(x) || is.integer(x[[1]])", NULL, is_numeric_linter())
+  expect_lint("is.numeric(x) || is.integer(x[[1]])", NULL, linter)
 })
 
 test_that("is_numeric_linter skips allowed usages involving %in%", {
+  linter <- is_numeric_linter()
+
   # false positives for class(x) %in% c('integer', 'numeric') style
-  expect_lint("class(x) %in% 1:10", NULL, is_numeric_linter())
-  expect_lint("class(x) %in% 'numeric'", NULL, is_numeric_linter())
-  expect_lint("class(x) %in% c('numeric', 'integer', 'factor')", NULL, is_numeric_linter())
-  expect_lint("class(x) %in% c('numeric', 'integer', y)", NULL, is_numeric_linter())
+  expect_lint("class(x) %in% 1:10", NULL, linter)
+  expect_lint("class(x) %in% 'numeric'", NULL, linter)
+  expect_lint("class(x) %in% c('numeric', 'integer', 'factor')", NULL, linter)
+  expect_lint("class(x) %in% c('numeric', 'integer', y)", NULL, linter)
 })
 
 test_that("is_numeric_linter blocks disallowed usages involving ||", {
   linter <- is_numeric_linter()
-  lint_msg <- rex::rex("same as is.numeric(x) || is.integer(x)")
+  lint_msg <- rex::rex("Use `is.numeric(x)` instead of equivalent `is.numeric(x) || is.integer(x)`.")
 
   expect_lint("is.numeric(x) || is.integer(x)", lint_msg, linter)
 
@@ -44,7 +48,7 @@ test_that("is_numeric_linter blocks disallowed usages involving ||", {
 
 test_that("is_numeric_linter blocks disallowed usages involving %in%", {
   linter <- is_numeric_linter()
-  lint_msg <- rex::rex('same as class(x) %in% c("integer", "numeric")')
+  lint_msg <- rex::rex('Use is.numeric(x) instead of equivalent class(x) %in% c("integer", "numeric")')
 
   expect_lint("class(x) %in% c('integer', 'numeric')", lint_msg, linter)
   expect_lint('class(x) %in% c("numeric", "integer")', lint_msg, linter)
@@ -54,7 +58,7 @@ test_that("raw strings are handled properly when testing in class", {
   skip_if_not_r_version("4.0.0")
 
   linter <- is_numeric_linter()
-  lint_msg <- rex::rex('same as class(x) %in% c("integer", "numeric")')
+  lint_msg <- rex::rex('Use is.numeric(x) instead of equivalent class(x) %in% c("integer", "numeric")')
 
   expect_lint("class(x) %in% c(R'(numeric)', 'integer', 'factor')", NULL, linter)
   expect_lint("class(x) %in% c('numeric', R'--(integer)--', y)", NULL, linter)

tests/testthat/test-knitr_formats.R

@@ -2,8 +2,8 @@ regexes <- list(
   assign = rex::rex("Use <-, not =, for assignment."),
   local_var = rex::rex("local variable"),
   quotes = rex::rex("Only use double-quotes."),
-  trailing = rex::rex("Trailing blank lines are superfluous."),
-  trailws = rex::rex("Trailing whitespace is superfluous."),
+  trailing = rex::rex("Remove trailing blank lines."),
+  trailws = rex::rex("Remove trailing whitespace."),
   indent = rex::rex("Indentation should be")
 )
 

tests/testthat/test-nested_ifelse_linter.R

@@ -14,48 +14,48 @@ test_that("nested_ifelse_linter skips allowed usages", {
 test_that("nested_ifelse_linter blocks simple disallowed usages", {
   expect_lint(
     "ifelse(l1, v1, ifelse(l2, v2, v3))",
-    rex::rex("Don't use nested ifelse() calls"),
+    rex::rex("Avoid nested ifelse() calls"),
     nested_ifelse_linter()
   )
 
   expect_lint(
     "ifelse(l1, ifelse(l2, v1, v2), v3)",
-    rex::rex("Don't use nested ifelse() calls"),
+    rex::rex("Avoid nested ifelse() calls"),
     nested_ifelse_linter()
   )
 })
 
 test_that("nested_ifelse_linter also catches dplyr::if_else", {
   expect_lint(
     "if_else(l1, v1, if_else(l2, v2, v3))",
-    rex::rex("Don't use nested if_else() calls"),
+    rex::rex("Avoid nested if_else() calls"),
     nested_ifelse_linter()
   )
 
   expect_lint(
     "dplyr::if_else(l1, dplyr::if_else(l2, v1, v2), v3)",
-    rex::rex("Don't use nested if_else() calls"),
+    rex::rex("Avoid nested if_else() calls"),
     nested_ifelse_linter()
   )
 })
 
 test_that("nested_ifelse_linter also catches data.table::fifelse", {
   expect_lint(
     "fifelse(l1, v1, fifelse(l2, v2, v3))",
-    rex::rex("Don't use nested fifelse() calls"),
+    rex::rex("Avoid nested fifelse() calls"),
     nested_ifelse_linter()
   )
 
   expect_lint(
     "data.table::fifelse(l1, v1, data.table::fifelse(l2, v2, v3))",
-    rex::rex("Don't use nested fifelse() calls"),
+    rex::rex("Avoid nested fifelse() calls"),
     nested_ifelse_linter()
   )
 
   # not sure why anyone would do this, but the readability still argument holds
   expect_lint(
     "data.table::fifelse(l1, dplyr::if_else(l2, v1, v2), v3)",
-    rex::rex("Don't use nested if_else() calls"),
+    rex::rex("Avoid nested if_else() calls"),
     nested_ifelse_linter()
   )
 })