Batch of message consistency checks

R/assignment_linter.R

@@ -111,7 +111,7 @@ assignment_linter <- function(allow_cascading_assign = TRUE,
     operator <- xml_text(bad_expr)
     lint_message_fmt <- rep("Use <-, not %s, for assignment.", length(operator))
     lint_message_fmt[operator %in% c("<<-", "->>")] <-
-      "%s can have hard-to-predict behavior; prefer assigning to a specific environment instead (with assign() or <-)."
+      "Avoid %s by assigning to a specific environment (with assign() or <-) to avoid hard-to-predict behavior."
     lint_message_fmt[operator == "%<>%"] <-
       "Avoid the assignment pipe %s; prefer using <- and %%>%% separately."
 

R/backport_linter.R

@@ -58,10 +58,7 @@ backport_linter <- function(r_version = getRversion(), except = character()) {
     needs_backport <- !is.na(bad_versions)
 
     lint_message <- sprintf(
-      paste(
-        "%s (R %s) is not available for dependency R >= %s.",
-        "Use the `except` argument of `backport_linter()` to configure available backports."
-      ),
+      "%s (R %s) is not available for dependency R >= %s.",
       all_names[needs_backport],
       bad_versions[needs_backport],
       r_version

R/brace_linter.R

@@ -194,7 +194,7 @@ brace_linter <- function(allow_single_line = FALSE) {
       xml_nodes_to_lints(
         xml_find_all(xml, xp_function_brace),
         source_expression = source_expression,
-        lint_message = "Any function spanning multiple lines should use curly braces."
+        lint_message = "Use curly braces for any function spanning multiple lines."
       )
     )
 

R/class_equals_linter.R

@@ -52,7 +52,7 @@ class_equals_linter <- function() {
 
     operator <- xml_find_chr(bad_expr, "string(*[2])")
     lint_message <- sprintf(
-      "Instead of comparing class(x) with %s, use inherits(x, 'class-name') or is.<class> or is(x, 'class')",
+      "use inherits(x, 'class-name') or is.<class> or is(x, 'class') instead of comparing class(x) with %s.",
       operator
     )
     xml_nodes_to_lints(

R/commas_linter.R

@@ -84,15 +84,15 @@ commas_linter <- function(allow_trailing = FALSE) {
     before_lints <- xml_nodes_to_lints(
       xml_find_all(xml, xpath_before),
       source_expression = source_expression,
-      lint_message = "Commas should never have a space before.",
+      lint_message = "Commas should never follow a space.",
       range_start_xpath = "number(./preceding-sibling::*[1]/@col2 + 1)", # start after preceding expression
       range_end_xpath = "number(./@col1 - 1)" # end before comma
     )
 
     after_lints <- xml_nodes_to_lints(
       xml_find_all(xml, xpath_after),
       source_expression = source_expression,
-      lint_message = "Commas should always have a space after.",
+      lint_message = "Commas should always precede a space.",
       range_start_xpath = "number(./@col2 + 1)", # start and end after comma
       range_end_xpath = "number(./@col2 + 1)"
     )

R/comment_linters.R

@@ -91,7 +91,7 @@ commented_code_linter <- function() {
     lint_list <- xml_nodes_to_lints(
       all_comment_nodes[is_parsable],
       source_expression = source_expression,
-      lint_message = "Commented code should be removed."
+      lint_message = "Remove commented code."
     )
 
     # Location info needs updating
@@ -175,7 +175,7 @@ todo_comment_linter <- function(todo = c("todo", "fixme")) {
           line_number = token[["line1"]],
           column_number = token[["col1"]],
           type = "style",
-          message = "TODO comments should be removed.",
+          message = "Remove TODO comments.",
           line = source_expression[["lines"]][[as.character(token[["line1"]])]],
           ranges = list(c(token[["col1"]], token[["col2"]]))
         )

R/condition_call_linter.R

@@ -58,58 +58,41 @@
 condition_call_linter <- function(display_call = FALSE) {
   call_xpath <- glue::glue("
     following-sibling::SYMBOL_SUB[text() = 'call.']
-    /following-sibling::expr[1]
-    /NUM_CONST[text() = '{!display_call}']
+      /following-sibling::expr[1]
+      /NUM_CONST[text() = '{!display_call}']
   ")
-  no_call_xpath <- "
-    parent::expr[
-      count(SYMBOL_SUB[text() = 'call.']) = 0
-    ]
-  "
+  no_call_xpath <- "parent::expr[not(SYMBOL_SUB[text() = 'call.'])]"
 
   if (is.na(display_call)) {
-    frag <- no_call_xpath
+    call_cond <- no_call_xpath
+    msg_fmt <- "Provide an explicit value for `call.` in %s()."
   } else if (display_call) {
-    frag <- call_xpath
+    call_cond <- call_xpath
+    msg_fmt <- "Use %s(.) to display the call in an error message."
   } else {
     # call. = TRUE can be expressed in two way:
     #  - either explicitly with call. = TRUE
     #  - or by implicitly relying on the default
-    frag <- xp_or(call_xpath, no_call_xpath)
+    call_cond <- xp_or(call_xpath, no_call_xpath)
+    msg_fmt <- "Use %s(., call. = FALSE) not to display the call in an error message."
   }
 
   xpath <- glue::glue("
     //SYMBOL_FUNCTION_CALL[text() = 'stop' or text() = 'warning']
-    /parent::expr[{frag}]
-    /parent::expr
+      /parent::expr[{call_cond}]
+      /parent::expr
   ")
 
   Linter(linter_level = "expression", function(source_expression) {
-
     xml <- source_expression$xml_parsed_content
     if (is.null(xml)) return(list())
 
     bad_expr <- xml_find_all(xml, xpath)
 
-    if (is.na(display_call)) {
-      msg <- glue::glue(
-        "Provide an explicit value for call. in {xp_call_name(bad_expr)}()."
-      )
-    } else if (display_call) {
-      msg <- glue::glue(
-        "Use {xp_call_name(bad_expr)}(.) to display call in error message."
-      )
-    } else {
-      msg <- glue::glue(
-        "Use {xp_call_name(bad_expr)}(., call. = FALSE)",
-        " to not display call in error message."
-      )
-    }
-
     xml_nodes_to_lints(
       bad_expr,
       source_expression = source_expression,
-      lint_message = msg,
+      lint_message = sprintf(msg_fmt, xp_call_name(bad_expr)),
       type = "warning"
     )
   })

R/conjunct_test_linter.R

@@ -127,13 +127,13 @@ conjunct_test_linter <- function(allow_named_stopifnot = TRUE,
     operator <- xml_find_chr(test_expr, "string(expr/*[self::AND2 or self::OR2])")
     replacement_fmt <- ifelse(
       matched_fun %in% c("expect_true", "expect_false"),
-      "write multiple expectations like %1$s(A) and %1$s(B)",
-      "write multiple conditions like %s(A, B)."
+      "Write multiple expectations like %1$s(A) and %1$s(B)",
+      "Write multiple conditions like %s(A, B)"
     )
     lint_message <- paste(
-      sprintf("Instead of %s(A %s B),", matched_fun, operator),
       # as.character() needed for 0-lint case where ifelse(logical(0)) returns logical(0)
       sprintf(as.character(replacement_fmt), matched_fun),
+      sprintf("instead of %s(A %s B).", matched_fun, operator),
       "The latter will produce better error messages in the case of failure."
     )
     lints <- xml_nodes_to_lints(

R/cyclocomp_linter.R

@@ -36,8 +36,8 @@ cyclocomp_linter <- function(complexity_limit = 15L) {
       column_number = source_expression[["column"]][1L],
       type = "style",
       message = sprintf(
-        "Functions should have cyclomatic complexity of less than %d, this has %d.",
-        complexity_limit, complexity
+        "Reduce the cyclomatic complexity of this function from %d to at most %d.",
+        complexity, complexity_limit
       ),
       ranges = list(rep(col1, 2L)),
       line = source_expression$lines[1L]

R/duplicate_argument_linter.R

@@ -58,7 +58,7 @@ duplicate_argument_linter <- function(except = c("mutate", "transmute")) {
     xml_nodes_to_lints(
       unlist(all_arg_nodes, recursive = FALSE)[unlist(is_duplicated)],
       source_expression = source_expression,
-      lint_message = "Duplicate arguments in function call.",
+      lint_message = "Remove duplicate arguments in function call.",
       type = "warning"
     )
   })

R/fixed_regex_linter.R

@@ -145,10 +145,13 @@ fixed_regex_linter <- function(allow_unescaped = FALSE) {
 
     patterns <- xml_find_all(xml, xpath)
     pattern_strings <- get_r_string(patterns)
+
     is_static <- is_not_regex(pattern_strings, allow_unescaped)
+    patterns <- patterns[is_static]
+    pattern_strings <- pattern_strings[is_static]
 
-    fixed_equivalent <- encodeString(get_fixed_string(pattern_strings[is_static]), quote = '"', justify = "none")
-    call_name <- xml_find_chr(patterns[is_static], "string(preceding-sibling::expr[last()]/SYMBOL_FUNCTION_CALL)")
+    fixed_equivalent <- encodeString(get_fixed_string(pattern_strings), quote = '"', justify = "none")
+    call_name <- xml_find_chr(patterns, "string(preceding-sibling::expr[last()]/SYMBOL_FUNCTION_CALL)")
 
     is_stringr <- startsWith(call_name, "str_")
     replacement <- ifelse(
@@ -157,13 +160,13 @@ fixed_regex_linter <- function(allow_unescaped = FALSE) {
       fixed_equivalent
     )
     msg <- paste(
-      "This regular expression is static, i.e., its matches can be expressed as a fixed substring expression, which",
-      "is faster to compute. Here, you can use",
-      replacement, ifelse(is_stringr, "as the pattern.", "with fixed = TRUE.")
+      "Use", replacement, ifelse(is_stringr, "as the pattern", "with fixed = TRUE"), "here.",
+      "This regular expression is static, i.e., its matches can be expressed as a fixed substring expression, ",
+      "which is faster to compute."
     )
 
     xml_nodes_to_lints(
-      patterns[is_static],
+      patterns,
       source_expression = source_expression,
       lint_message = msg,
       type = "warning"

R/if_not_else_linter.R

@@ -91,10 +91,7 @@ if_not_else_linter <- function(exceptions = c("is.null", "is.na", "missing")) {
     if_lints <- xml_nodes_to_lints(
       if_expr,
       source_expression = source_expression,
-      lint_message = paste(
-        "In a simple if/else statement,",
-        "prefer `if (A) x else y` to the less-readable `if (!A) y else x`."
-      ),
+      lint_message = "Prefer `if (A) x else y` to the less-readable `if (!A) y else x` in a simple if/else statement.",
       type = "warning"
     )
 

R/implicit_integer_linter.R

@@ -60,7 +60,8 @@ implicit_integer_linter <- function(allow_colon = FALSE) {
     xml_nodes_to_lints(
       numbers[is_implicit_integer(xml_text(numbers))],
       source_expression = source_expression,
-      lint_message = "Integers should not be implicit. Use the form 1L for integers or 1.0 for doubles.",
+      lint_message =
+        "Avoid implicit integers. Make the type explicit by using the form 1L for integers or 1.0 for doubles.",
       type = "style",
       column_number_xpath = "number(./@col2 + 1)", # mark at end
       range_end_xpath = "number(./@col2 + 1)" # end after number for easy fixing (enter "L" or ".0")

R/is_numeric_linter.R

@@ -78,7 +78,7 @@ is_numeric_linter <- function() {
       or_expr,
       source_expression = source_expression,
       lint_message = paste(
-        "is.numeric(x) is the same as is.numeric(x) || is.integer(x).",
+        "Use is.numeric(x) instead of equivalent is.numeric(x) || is.integer(x).",
         "Use is.double(x) to test for objects stored as 64-bit floating point."
       ),
       type = "warning"
@@ -97,7 +97,7 @@ is_numeric_linter <- function() {
       class_expr,
       source_expression = source_expression,
       lint_message = paste(
-        'is.numeric(x) is the same as class(x) %in% c("integer", "numeric").',
+        'Use is.numeric(x) instead of equivalent class(x) %in% c("integer", "numeric").',
         "Use is.double(x) to test for objects stored as 64-bit floating point."
       ),
       type = "warning"