review comments

r-lib · Aug 2, 2023 · 33abc2f · 33abc2f
1 parent fb19002
commit 33abc2f
Show file tree

Hide file tree

Showing 3 changed files with 129 additions and 51 deletions.
diff --git a/R/library_call_linter.R b/R/library_call_linter.R
@@ -3,38 +3,53 @@
 #' Force library calls to all be at the top of the script.
 #'
 #' @examples
-#'  # will produce lints
-#'  lint(
-#'    text = c("library(dplyr)", "print('test')", "library(tidyr)"),
-#'    linters = library_call_linter()
-#'  )
+#' # will produce lints
+#' lint(
+#'   text = "
+#'     library(dplyr)
+#'     print('test')
+#'     library(tidyr)
+#'   ",
+#'   linters = library_call_linter()
+#' )
 #'
-#'  lint(
-#'    text = c("library(dplyr)", "print('test')", "library(tidyr)", "library(purrr)"),
-#'    linters = library_call_linter()
-#'  )
+#' lint(
+#'   text = "
+#'     library(dplyr)
+#'     print('test')
+#'     library(tidyr)
+#'     library(purrr)
+#'   ",
+#'   linters = library_call_linter()
+#' )
 #'
-#'  # okay
-#'  lint(
-#'    text = c("library(dplyr)", "print('test')"),
-#'    linters = library_call_linter()
-#'  )
+#' # okay
+#' lint(
+#'   text = "
+#'     library(dplyr)
+#'     print('test')
+#'   ",
+#'   linters = library_call_linter()
+#' )
 #'
-#'  lint(
-#'    text = c("# comment", "library(dplyr)"),
-#'    linters = library_call_linter()
-#'  )
+#' lint(
+#'   text = "
+#'     # comment
+#'     library(dplyr)
+#'   ",
+#'   linters = library_call_linter()
+#' )
 #'
 #' @evalRd rd_tags("library_call_linter")
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
 library_call_linter <- function() {
 
   xpath <- "
-  (//SYMBOL_FUNCTION_CALL[text() = 'library'])[last()]
-  //preceding::expr
-  //SYMBOL_FUNCTION_CALL[text() != 'library'][last()]
-  //following::expr[SYMBOL_FUNCTION_CALL[text() = 'library']]
+    (//SYMBOL_FUNCTION_CALL[text() = 'library'])[last()]
+    /preceding::expr
+    /SYMBOL_FUNCTION_CALL[text() != 'library'][last()]
+    /following::expr[SYMBOL_FUNCTION_CALL[text() = 'library']]
   "
 
   Linter(function(source_expression) {

diff --git a/man/library_call_linter.Rd b/man/library_call_linter.Rd
diff --git a/tests/testthat/test-library_call_linter.R b/tests/testthat/test-library_call_linter.R
@@ -1,6 +1,10 @@
 test_that("library_call_linter skips allowed usages", {
 
-  expect_lint(c("library(dplyr)", "print('test')"),
+  expect_lint(
+    trim_some("
+      library(dplyr)
+      print('test')
+    "),
     NULL,
     library_call_linter()
   )
@@ -10,12 +14,20 @@ test_that("library_call_linter skips allowed usages", {
     library_call_linter()
   )
 
-  expect_lint(c("# comment", "library(dplyr)"),
+  expect_lint(
+    trim_some("
+      # comment
+      library(dplyr)
+    "),
     NULL,
     library_call_linter()
   )
 
-  expect_lint(c("print('test')", "# library(dplyr)"),
+  expect_lint(
+    trim_some("
+      print('test')
+      # library(dplyr)
+    "),
     NULL,
     library_call_linter()
   )
@@ -24,28 +36,64 @@ test_that("library_call_linter skips allowed usages", {
 test_that("library_call_linter warns on disallowed usages", {
   lint_message <- rex::rex("Move all library calls to the top of the script.")
 
-  expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)"),
+  expect_lint(
+    trim_some("
+      library(dplyr)
+      print('test')
+      library(tidyr)
+    "),
     lint_message,
     library_call_linter()
   )
 
-  expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)", "library(purrr)"),
-    list(lint_message, lint_message),
+  expect_lint(
+    trim_some("
+      library(dplyr)
+      print('test')
+      library(tidyr)
+      library(purrr)
+    "),
+    list(
+      list(lint_message, line_number = 3, column_number = 1),
+      list(lint_message, line_number = 4, column_number = 1)
+    ),
     library_call_linter()
   )
 
-  expect_lint(c("library(dplyr)", "print('test')", "print('test')", "library(tidyr)"),
-    rex("Move all library calls to the top of the script."),
+  expect_lint(
+    trim_some("
+      library(dplyr)
+      print('test')
+      print('test')
+      library(tidyr)
+    "),
+    lint_message,
     library_call_linter()
   )
 
-  expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)", "print('test')"),
+  expect_lint(
+    trim_some("
+      library(dplyr)
+      print('test')
+      library(tidyr)
+      print('test')
+    "),
     lint_message,
     library_call_linter()
   )
 
-  expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)", "print('test')", "library(tidyr)"),
-    list(lint_message, lint_message),
+  expect_lint(
+    trim_some("
+      library(dplyr)
+      print('test')
+      library(tidyr)
+      print('test')
+      library(purrr)
+    "),
+    list(
+      list(lint_message, line_number = 3, column_number = 1),
+      list(lint_message, line_number = 5, column_number = 1)
+    ),
     library_call_linter()
   )
 })