include line length in lint message (#2066)

* include line length in lint message

* slight refactor, test multiple lints

* rm vestigial

* adjust incidental test

* NEWS grammar

* trailing ws

* explicit integer

Author: MichaelChirico

NEWS.md

@@ -23,6 +23,7 @@
   + `unreachable_code_linter()`
   + `yoda_test_linter()`
 * `sprintf_linter()` is pipe-aware, so that `x %>% sprintf(fmt = "%s")` no longer lints (#1943, @MichaelChirico).
+* `line_length_linter()` helpfully includes the line length in the lint message (#2057, @MichaelChirico).
 
 ### New linters
 

R/line_length_linter.R

@@ -23,6 +23,8 @@
 #' - <https://style.tidyverse.org/syntax.html#long-lines>
 #' @export
 line_length_linter <- function(length = 80L) {
+  general_msg <- paste("Lines should not be more than", length, "characters.")
+
   Linter(function(source_expression) {
     # Only go over complete file
     if (!is_lint_level(source_expression, "file")) {
@@ -32,18 +34,20 @@ line_length_linter <- function(length = 80L) {
     line_lengths <- nchar(source_expression$file_lines)
     long_lines <- which(line_lengths > length)
 
-    lint_message <- sprintf("Lines should not be more than %d characters.", length)
-
-    lapply(long_lines, function(long_line) {
-      Lint(
-        filename = source_expression$filename,
-        line_number = long_line,
-        column_number = length + 1L,
-        type = "style",
-        message = lint_message,
-        line = source_expression$file_lines[long_line],
-        ranges = list(c(1L, line_lengths[long_line]))
-      )
-    })
+    mapply(
+      function(long_line, line_length) {
+        Lint(
+          filename = source_expression$filename,
+          line_number = long_line,
+          column_number = length + 1L,
+          type = "style",
+          message = paste(general_msg, "This line is", line_length, "characters."),
+          line = source_expression$file_lines[long_line],
+          ranges = list(c(1L, line_length))
+        )
+      },
+      long_lines, line_lengths[long_lines],
+      SIMPLIFY = FALSE
+    )
   })
 }

tests/testthat/test-line_length_linter.R

@@ -7,7 +7,7 @@ test_that("line_length_linter skips allowed usages", {
 
 test_that("line_length_linter blocks disallowed usages", {
   linter <- line_length_linter(80L)
-  lint_msg <- rex::rex("Lines should not be more than 80 characters")
+  lint_msg <- rex::rex("Lines should not be more than 80 characters. This line is 81 characters.")
 
   expect_lint(
     strrep("x", 81L),
@@ -34,7 +34,7 @@ test_that("line_length_linter blocks disallowed usages", {
   )
 
   linter <- line_length_linter(20L)
-  lint_msg <- rex::rex("Lines should not be more than 20 characters")
+  lint_msg <- rex::rex("Lines should not be more than 20 characters. This line is 22 characters.")
   expect_lint(strrep("a", 20L), NULL, linter)
   expect_lint(
     strrep("a", 22L),
@@ -55,3 +55,14 @@ test_that("line_length_linter blocks disallowed usages", {
     1L
   )
 })
+
+test_that("Multiple lints give custom messages", {
+  expect_lint(
+    trim_some("{
+      abcdefg
+      hijklmnop
+    }"),
+    list("9 characters", "11 characters"),
+    line_length_linter(5L)
+  )
+})

tests/testthat/test-with.R

@@ -50,7 +50,10 @@ test_that("all default linters are tagged default", {
   skip_if_not_r_version("4.1.0") # Desired all.equal behavior only available in >= 4.1
   expect_identical(
     all.equal(linters_with_tags("default"), linters_with_defaults(line_length_linter(120L))),
-    'Component "line_length_linter": Component "length": Mean relative difference: 0.5'
+    c(
+      'Component "line_length_linter": Component "general_msg": 1 string mismatch',
+      'Component "line_length_linter": Component "length": Mean relative difference: 0.5'
+    )
   )
 })