Mention the specific operator in lint message for vector_logic_linter…

…() (#2398) * mention the specific operator in lint message * use strrep --------- Co-authored-by: AshesITR <[email protected]>
r-lib · Dec 6, 2023 · f262dd1 · f262dd1
1 parent 5072724
commit f262dd1
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 12 deletions.
diff --git a/R/vector_logic_linter.R b/R/vector_logic_linter.R
@@ -86,10 +86,11 @@ vector_logic_linter <- function() {
     if (is.null(xml)) return(list())
     bad_expr <- xml_find_all(xml, xpath)
 
+    op <- xml_text(bad_expr)
     xml_nodes_to_lints(
       bad_expr,
       source_expression = source_expression,
-      lint_message = "Conditional expressions require scalar logical operators (&& and ||)",
+      lint_message = sprintf("Use `%s` in conditional expressions.", strrep(op, 2L)),
       type = "warning"
     )
   })

diff --git a/tests/testthat/test-vector_logic_linter.R b/tests/testthat/test-vector_logic_linter.R
@@ -29,30 +29,37 @@ test_that("vector_logic_linter skips allowed usages", {
 
 test_that("vector_logic_linter blocks simple disallowed usages", {
   linter <- vector_logic_linter()
-  lint_msg <- rex::rex("Conditional expressions require scalar logical operators")
 
-  expect_lint("if (TRUE & FALSE) 1", lint_msg, linter)
-  expect_lint("while (TRUE | TRUE) 2", lint_msg, linter)
+  expect_lint("if (TRUE & FALSE) 1", rex::rex("Use `&&` in conditional expressions."), linter)
+  expect_lint("while (TRUE | TRUE) 2", rex::rex("Use `||` in conditional expressions."), linter)
 })
 
 test_that("vector_logic_linter detects nested conditions", {
   linter <- vector_logic_linter()
-  lint_msg <- rex::rex("Conditional expressions require scalar logical operators")
 
-  expect_lint("if (TRUE & TRUE || FALSE) 4", lint_msg, linter)
-  expect_lint("if (TRUE && (TRUE | FALSE)) 4", lint_msg, linter)
+  expect_lint(
+    "if (TRUE & TRUE || FALSE) 4",
+    list(rex::rex("Use `&&` in conditional expressions."), column_number = 10L),
+    linter
+  )
+  expect_lint(
+    "if (TRUE && (TRUE | FALSE)) 4",
+    list(rex::rex("Use `||` in conditional expressions."), column_number = 19L),
+    linter
+  )
 })
 
 test_that("vector_logic_linter catches usages in expect_true()/expect_false()", {
   linter <- vector_logic_linter()
-  lint_msg <- rex::rex("Conditional expressions require scalar logical operators")
+  and_msg <- rex::rex("Use `&&` in conditional expressions.")
+  or_msg <- rex::rex("Use `||` in conditional expressions.")
 
-  expect_lint("expect_true(TRUE & FALSE)", lint_msg, linter)
-  expect_lint("expect_false(TRUE | TRUE)", lint_msg, linter)
+  expect_lint("expect_true(TRUE & FALSE)", and_msg, linter)
+  expect_lint("expect_false(TRUE | TRUE)", or_msg, linter)
 
   # ditto with namespace qualification
-  expect_lint("testthat::expect_true(TRUE & FALSE)", lint_msg, linter)
-  expect_lint("testthat::expect_false(TRUE | TRUE)", lint_msg, linter)
+  expect_lint("testthat::expect_true(TRUE & FALSE)", and_msg, linter)
+  expect_lint("testthat::expect_false(TRUE | TRUE)", or_msg, linter)
 })
 
 test_that("vector_logic_linter doesn't get mixed up from complex usage", {