From 50163da0e24a6eb6cf9ab58249cb8f1c13cfcbd2 Mon Sep 17 00:00:00 2001 From: Nicholas Masel Date: Fri, 28 Jul 2023 11:45:12 +0000 Subject: [PATCH 01/11] add library call linter --- DESCRIPTION | 1 + NAMESPACE | 1 + R/library_call_linter.R | 55 +++++++++++++++++++++++ inst/lintr/linters.csv | 1 + man/best_practices_linters.Rd | 1 + man/library_call_linter.Rd | 36 +++++++++++++++ man/linters.Rd | 5 ++- man/style_linters.Rd | 1 + tests/testthat/test-library_call_linter.R | 39 ++++++++++++++++ 9 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 R/library_call_linter.R create mode 100644 man/library_call_linter.Rd create mode 100644 tests/testthat/test-library_call_linter.R diff --git a/DESCRIPTION b/DESCRIPTION index 0358b669f..c9feea16b 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -114,6 +114,7 @@ Collate: 'is_lint_level.R' 'is_numeric_linter.R' 'lengths_linter.R' + 'library_call_linter.R' 'line_length_linter.R' 'lint.R' 'linter_tag_docs.R' diff --git a/NAMESPACE b/NAMESPACE index 5caba4239..f55595c3b 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -78,6 +78,7 @@ export(inner_combine_linter) export(is_lint_level) export(is_numeric_linter) export(lengths_linter) +export(library_call_linter) export(line_length_linter) export(lint) export(lint_dir) diff --git a/R/library_call_linter.R b/R/library_call_linter.R new file mode 100644 index 000000000..b40518beb --- /dev/null +++ b/R/library_call_linter.R @@ -0,0 +1,55 @@ +#' Library call linter +#' +#' 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() +#' ) +#' +#' # okay +#' lint( +#' text = c("library(dplyr)", "print('test')"), +#' linters = library_call_linter() +#' ) +#' +#' lint( +#' text = c("# 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()] + " + Linter(function(source_expression) { + if (!is_lint_level(source_expression, "file")) { + return(list()) + } + + xml <- source_expression$full_xml_parsed_content + + bad_expr <- xml2::xml_find_all(xml, xpath) + + if (length(bad_expr) == 0L) { + return(list()) + } + + bad_expr_library<- xml2::xml_find_all(xml, "//SYMBOL_FUNCTION_CALL[text() = 'library'][last()]") + + xml_nodes_to_lints( + bad_expr_library, + source_expression = source_expression, + lint_message = "Move all library calls to the top of the script.", + type = "warning" + ) + }) +} diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv index e6bbebaed..1898a45dc 100644 --- a/inst/lintr/linters.csv +++ b/inst/lintr/linters.csv @@ -42,6 +42,7 @@ infix_spaces_linter,style readability default configurable inner_combine_linter,efficiency consistency readability is_numeric_linter,readability best_practices consistency lengths_linter,efficiency readability best_practices +library_call_linter,style best_practices line_length_linter,style readability default configurable literal_coercion_linter,best_practices consistency efficiency matrix_apply_linter,readability efficiency diff --git a/man/best_practices_linters.Rd b/man/best_practices_linters.Rd index 397b8e9c2..7223e44ed 100644 --- a/man/best_practices_linters.Rd +++ b/man/best_practices_linters.Rd @@ -41,6 +41,7 @@ The following linters are tagged with 'best_practices': \item{\code{\link{implicit_integer_linter}}} \item{\code{\link{is_numeric_linter}}} \item{\code{\link{lengths_linter}}} +\item{\code{\link{library_call_linter}}} \item{\code{\link{literal_coercion_linter}}} \item{\code{\link{nonportable_path_linter}}} \item{\code{\link{outer_negation_linter}}} diff --git a/man/library_call_linter.Rd b/man/library_call_linter.Rd new file mode 100644 index 000000000..dc1e68413 --- /dev/null +++ b/man/library_call_linter.Rd @@ -0,0 +1,36 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/library_call_linter.R +\name{library_call_linter} +\alias{library_call_linter} +\title{Library call linter} +\usage{ +library_call_linter() +} +\description{ +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() + ) + + # okay + lint( + text = c("library(dplyr)", "print('test')"), + linters = library_call_linter() + ) + + lint( + text = c("# comment", "library(dplyr)"), + linters = library_call_linter() + ) + +} +\seealso{ +\link{linters} for a complete list of linters available in lintr. +} +\section{Tags}{ +\link[=best_practices_linters]{best_practices}, \link[=style_linters]{style} +} diff --git a/man/linters.Rd b/man/linters.Rd index 8cce38cf6..bb2a2f784 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -17,7 +17,7 @@ see also \code{\link[=available_tags]{available_tags()}}. \section{Tags}{ The following tags exist: \itemize{ -\item{\link[=best_practices_linters]{best_practices} (50 linters)} +\item{\link[=best_practices_linters]{best_practices} (51 linters)} \item{\link[=common_mistakes_linters]{common_mistakes} (7 linters)} \item{\link[=configurable_linters]{configurable} (29 linters)} \item{\link[=consistency_linters]{consistency} (18 linters)} @@ -29,7 +29,7 @@ The following tags exist: \item{\link[=package_development_linters]{package_development} (14 linters)} \item{\link[=readability_linters]{readability} (47 linters)} \item{\link[=robustness_linters]{robustness} (14 linters)} -\item{\link[=style_linters]{style} (34 linters)} +\item{\link[=style_linters]{style} (35 linters)} } } \section{Linters}{ @@ -76,6 +76,7 @@ The following linters exist: \item{\code{\link{inner_combine_linter}} (tags: consistency, efficiency, readability)} \item{\code{\link{is_numeric_linter}} (tags: best_practices, consistency, readability)} \item{\code{\link{lengths_linter}} (tags: best_practices, efficiency, readability)} +\item{\code{\link{library_call_linter}} (tags: best_practices, style)} \item{\code{\link{line_length_linter}} (tags: configurable, default, readability, style)} \item{\code{\link{literal_coercion_linter}} (tags: best_practices, consistency, efficiency)} \item{\code{\link{matrix_apply_linter}} (tags: efficiency, readability)} diff --git a/man/style_linters.Rd b/man/style_linters.Rd index 1c4e93c17..a3f8a8c67 100644 --- a/man/style_linters.Rd +++ b/man/style_linters.Rd @@ -25,6 +25,7 @@ The following linters are tagged with 'style': \item{\code{\link{implicit_integer_linter}}} \item{\code{\link{indentation_linter}}} \item{\code{\link{infix_spaces_linter}}} +\item{\code{\link{library_call_linter}}} \item{\code{\link{line_length_linter}}} \item{\code{\link{numeric_leading_zero_linter}}} \item{\code{\link{object_length_linter}}} diff --git a/tests/testthat/test-library_call_linter.R b/tests/testthat/test-library_call_linter.R new file mode 100644 index 000000000..84a325b4f --- /dev/null +++ b/tests/testthat/test-library_call_linter.R @@ -0,0 +1,39 @@ +test_that("library_call_linter skips allowed usages", { + + expect_lint(c("library(dplyr)", "print('test')"), + NULL, + library_call_linter() + ) + + expect_lint(c("print('test')"), + NULL, + library_call_linter() + ) + + expect_lint(c("# comment", "library(dplyr)"), + NULL, + library_call_linter() + ) +}) + +test_that("library_call_linter warns on disallowed usages", { + expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)"), + rex("Move all library calls to the top of the script."), + 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."), + library_call_linter() + ) + + expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)", "print('test')"), + rex("Move all library calls to the top of the script."), + library_call_linter() + ) + + expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)", "print('test')", "library(tidyr)"), + rex("Move all library calls to the top of the script."), + library_call_linter() + ) +}) From 97955c2c8ce8fda3dbd27150204b709773c38ee3 Mon Sep 17 00:00:00 2001 From: Nicholas Masel Date: Fri, 28 Jul 2023 11:59:41 +0000 Subject: [PATCH 02/11] tests --- tests/testthat/test-library_call_linter.R | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/testthat/test-library_call_linter.R b/tests/testthat/test-library_call_linter.R index 84a325b4f..91f7b5748 100644 --- a/tests/testthat/test-library_call_linter.R +++ b/tests/testthat/test-library_call_linter.R @@ -14,6 +14,11 @@ test_that("library_call_linter skips allowed usages", { NULL, library_call_linter() ) + + expect_lint(c("print('test')", "# library(dplyr)"), + NULL, + library_call_linter() + ) }) test_that("library_call_linter warns on disallowed usages", { From 346d052cc54335f8412aec0c4ea88ddfb2c7619c Mon Sep 17 00:00:00 2001 From: Nicholas Masel Date: Mon, 31 Jul 2023 19:33:38 +0000 Subject: [PATCH 03/11] correct lint findings --- R/library_call_linter.R | 5 ++++- tests/testthat/test-library_call_linter.R | 15 +++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/R/library_call_linter.R b/R/library_call_linter.R index b40518beb..9355212f6 100644 --- a/R/library_call_linter.R +++ b/R/library_call_linter.R @@ -43,7 +43,10 @@ library_call_linter <- function() { return(list()) } - bad_expr_library<- xml2::xml_find_all(xml, "//SYMBOL_FUNCTION_CALL[text() = 'library'][last()]") + bad_expr_library <- xml2::xml_find_all( + xml, + "//SYMBOL_FUNCTION_CALL[text() = 'library'][last()]" + ) xml_nodes_to_lints( bad_expr_library, diff --git a/tests/testthat/test-library_call_linter.R b/tests/testthat/test-library_call_linter.R index 91f7b5748..63662cdda 100644 --- a/tests/testthat/test-library_call_linter.R +++ b/tests/testthat/test-library_call_linter.R @@ -5,7 +5,7 @@ test_that("library_call_linter skips allowed usages", { library_call_linter() ) - expect_lint(c("print('test')"), + expect_lint("print('test')", NULL, library_call_linter() ) @@ -22,8 +22,15 @@ 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)"), - rex("Move all library calls to the top of the script."), + lint_message, + library_call_linter() + ) + + expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)", "library(purrr)"), + lint_message, library_call_linter() ) @@ -33,12 +40,12 @@ test_that("library_call_linter warns on disallowed usages", { ) expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)", "print('test')"), - rex("Move all library calls to the top of the script."), + lint_message, library_call_linter() ) expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)", "print('test')", "library(tidyr)"), - rex("Move all library calls to the top of the script."), + lint_message, library_call_linter() ) }) From 25f76afceba24e834b360b795d0bb9a0606334b6 Mon Sep 17 00:00:00 2001 From: Nicholas Masel Date: Mon, 31 Jul 2023 20:18:22 +0000 Subject: [PATCH 04/11] fix lint results --- R/library_call_linter.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/library_call_linter.R b/R/library_call_linter.R index 9355212f6..738bd7473 100644 --- a/R/library_call_linter.R +++ b/R/library_call_linter.R @@ -46,7 +46,7 @@ library_call_linter <- function() { bad_expr_library <- xml2::xml_find_all( xml, "//SYMBOL_FUNCTION_CALL[text() = 'library'][last()]" - ) + ) xml_nodes_to_lints( bad_expr_library, From 6a566196d74356d668be9302a455174e9a044eb7 Mon Sep 17 00:00:00 2001 From: Nicholas Masel Date: Tue, 1 Aug 2023 15:10:45 +0000 Subject: [PATCH 05/11] return last node rather than last element --- R/library_call_linter.R | 20 ++++++++++++-------- man/library_call_linter.Rd | 5 +++++ tests/testthat/test-library_call_linter.R | 4 ++-- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/R/library_call_linter.R b/R/library_call_linter.R index 738bd7473..b4e06af48 100644 --- a/R/library_call_linter.R +++ b/R/library_call_linter.R @@ -9,6 +9,11 @@ #' linters = library_call_linter() #' ) #' +#' lint( +#' text = c("library(dplyr)", "print('test')", "library(tidyr)", "library(purrr)"), +#' linters = library_call_linter() +#' ) +#' #' # okay #' lint( #' text = c("library(dplyr)", "print('test')"), @@ -26,10 +31,12 @@ library_call_linter <- function() { xpath <- " - //SYMBOL_FUNCTION_CALL[text() = 'library'][last()] + (//SYMBOL_FUNCTION_CALL[text() = 'library'])[last()] //preceding::expr - /SYMBOL_FUNCTION_CALL[text() != 'library'][last()] + //SYMBOL_FUNCTION_CALL[text() != 'library'][last()] + //following::expr[SYMBOL_FUNCTION_CALL[text() = 'library']] " + Linter(function(source_expression) { if (!is_lint_level(source_expression, "file")) { return(list()) @@ -37,19 +44,16 @@ library_call_linter <- function() { xml <- source_expression$full_xml_parsed_content + writeLines(as.character(xml)) + bad_expr <- xml2::xml_find_all(xml, xpath) if (length(bad_expr) == 0L) { return(list()) } - bad_expr_library <- xml2::xml_find_all( - xml, - "//SYMBOL_FUNCTION_CALL[text() = 'library'][last()]" - ) - xml_nodes_to_lints( - bad_expr_library, + bad_expr, source_expression = source_expression, lint_message = "Move all library calls to the top of the script.", type = "warning" diff --git a/man/library_call_linter.Rd b/man/library_call_linter.Rd index dc1e68413..5fe048603 100644 --- a/man/library_call_linter.Rd +++ b/man/library_call_linter.Rd @@ -16,6 +16,11 @@ Force library calls to all be at the top of the script. linters = library_call_linter() ) + lint( + text = c("library(dplyr)", "print('test')", "library(tidyr)", "library(purrr)"), + linters = library_call_linter() + ) + # okay lint( text = c("library(dplyr)", "print('test')"), diff --git a/tests/testthat/test-library_call_linter.R b/tests/testthat/test-library_call_linter.R index 63662cdda..52a719a9a 100644 --- a/tests/testthat/test-library_call_linter.R +++ b/tests/testthat/test-library_call_linter.R @@ -30,7 +30,7 @@ test_that("library_call_linter warns on disallowed usages", { ) expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)", "library(purrr)"), - lint_message, + list(lint_message, lint_message), library_call_linter() ) @@ -45,7 +45,7 @@ test_that("library_call_linter warns on disallowed usages", { ) expect_lint(c("library(dplyr)", "print('test')", "library(tidyr)", "print('test')", "library(tidyr)"), - lint_message, + list(lint_message, lint_message), library_call_linter() ) }) From c857d5af5322e7e75e34cb06aedb42a048f8c337 Mon Sep 17 00:00:00 2001 From: Nicholas Masel Date: Tue, 1 Aug 2023 15:41:36 +0000 Subject: [PATCH 06/11] remove debug code --- R/library_call_linter.R | 2 -- 1 file changed, 2 deletions(-) diff --git a/R/library_call_linter.R b/R/library_call_linter.R index b4e06af48..0d965eb44 100644 --- a/R/library_call_linter.R +++ b/R/library_call_linter.R @@ -44,8 +44,6 @@ library_call_linter <- function() { xml <- source_expression$full_xml_parsed_content - writeLines(as.character(xml)) - bad_expr <- xml2::xml_find_all(xml, xpath) if (length(bad_expr) == 0L) { From 33abc2f5d5d8a91ef206abd914c3537a08b6245c Mon Sep 17 00:00:00 2001 From: Nicholas Masel Date: Wed, 2 Aug 2023 21:01:08 +0000 Subject: [PATCH 07/11] review comments --- R/library_call_linter.R | 59 ++++++++++++------- man/library_call_linter.Rd | 51 +++++++++++------ tests/testthat/test-library_call_linter.R | 70 +++++++++++++++++++---- 3 files changed, 129 insertions(+), 51 deletions(-) diff --git a/R/library_call_linter.R b/R/library_call_linter.R index 0d965eb44..49f0edb61 100644 --- a/R/library_call_linter.R +++ b/R/library_call_linter.R @@ -3,27 +3,42 @@ #' 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. @@ -31,10 +46,10 @@ 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 index 5fe048603..9f7d2f0b8 100644 --- a/man/library_call_linter.Rd +++ b/man/library_call_linter.Rd @@ -10,27 +10,42 @@ library_call_linter() 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() +) } \seealso{ diff --git a/tests/testthat/test-library_call_linter.R b/tests/testthat/test-library_call_linter.R index 52a719a9a..2c543d6ed 100644 --- 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() ) }) From a47fdb3f49ece9d4ef64044d349bb068ccbbbe83 Mon Sep 17 00:00:00 2001 From: Nicholas Masel Date: Wed, 2 Aug 2023 21:13:02 +0000 Subject: [PATCH 08/11] add explicit L for integers --- tests/testthat/test-library_call_linter.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test-library_call_linter.R b/tests/testthat/test-library_call_linter.R index 2c543d6ed..8e2184f1e 100644 --- a/tests/testthat/test-library_call_linter.R +++ b/tests/testthat/test-library_call_linter.R @@ -54,8 +54,8 @@ test_that("library_call_linter warns on disallowed usages", { library(purrr) "), list( - list(lint_message, line_number = 3, column_number = 1), - list(lint_message, line_number = 4, column_number = 1) + list(lint_message, line_number = 3L, column_number = 1L), + list(lint_message, line_number = 4L, column_number = 1L) ), library_call_linter() ) @@ -91,8 +91,8 @@ test_that("library_call_linter warns on disallowed usages", { library(purrr) "), list( - list(lint_message, line_number = 3, column_number = 1), - list(lint_message, line_number = 5, column_number = 1) + list(lint_message, line_number = 3L, column_number = 1L), + list(lint_message, line_number = 5L, column_number = 1L) ), library_call_linter() ) From 8d414fc93efa951ea98b3a3b85d9b2944099492d Mon Sep 17 00:00:00 2001 From: AshesITR Date: Thu, 3 Aug 2023 01:25:25 +0200 Subject: [PATCH 09/11] indent XPath one more level --- R/library_call_linter.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/library_call_linter.R b/R/library_call_linter.R index 49f0edb61..52411f8fe 100644 --- a/R/library_call_linter.R +++ b/R/library_call_linter.R @@ -47,9 +47,9 @@ 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']] + /preceding::expr + /SYMBOL_FUNCTION_CALL[text() != 'library'][last()] + /following::expr[SYMBOL_FUNCTION_CALL[text() = 'library']] " Linter(function(source_expression) { From cccdbc4bb8d0ad540b169d5855f1cd625a5ecaf7 Mon Sep 17 00:00:00 2001 From: Nicholas Masel Date: Thu, 3 Aug 2023 15:57:31 +0000 Subject: [PATCH 10/11] add readability --- inst/lintr/linters.csv | 2 +- man/library_call_linter.Rd | 2 +- man/linters.Rd | 4 ++-- man/readability_linters.Rd | 1 + 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv index 1898a45dc..ba873d4cb 100644 --- a/inst/lintr/linters.csv +++ b/inst/lintr/linters.csv @@ -42,7 +42,7 @@ infix_spaces_linter,style readability default configurable inner_combine_linter,efficiency consistency readability is_numeric_linter,readability best_practices consistency lengths_linter,efficiency readability best_practices -library_call_linter,style best_practices +library_call_linter,style best_practices readability line_length_linter,style readability default configurable literal_coercion_linter,best_practices consistency efficiency matrix_apply_linter,readability efficiency diff --git a/man/library_call_linter.Rd b/man/library_call_linter.Rd index 9f7d2f0b8..2ee4260e5 100644 --- a/man/library_call_linter.Rd +++ b/man/library_call_linter.Rd @@ -52,5 +52,5 @@ lint( \link{linters} for a complete list of linters available in lintr. } \section{Tags}{ -\link[=best_practices_linters]{best_practices}, \link[=style_linters]{style} +\link[=best_practices_linters]{best_practices}, \link[=readability_linters]{readability}, \link[=style_linters]{style} } diff --git a/man/linters.Rd b/man/linters.Rd index bb2a2f784..cf75146cc 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -27,7 +27,7 @@ The following tags exist: \item{\link[=efficiency_linters]{efficiency} (23 linters)} \item{\link[=executing_linters]{executing} (5 linters)} \item{\link[=package_development_linters]{package_development} (14 linters)} -\item{\link[=readability_linters]{readability} (47 linters)} +\item{\link[=readability_linters]{readability} (48 linters)} \item{\link[=robustness_linters]{robustness} (14 linters)} \item{\link[=style_linters]{style} (35 linters)} } @@ -76,7 +76,7 @@ The following linters exist: \item{\code{\link{inner_combine_linter}} (tags: consistency, efficiency, readability)} \item{\code{\link{is_numeric_linter}} (tags: best_practices, consistency, readability)} \item{\code{\link{lengths_linter}} (tags: best_practices, efficiency, readability)} -\item{\code{\link{library_call_linter}} (tags: best_practices, style)} +\item{\code{\link{library_call_linter}} (tags: best_practices, readability, style)} \item{\code{\link{line_length_linter}} (tags: configurable, default, readability, style)} \item{\code{\link{literal_coercion_linter}} (tags: best_practices, consistency, efficiency)} \item{\code{\link{matrix_apply_linter}} (tags: efficiency, readability)} diff --git a/man/readability_linters.Rd b/man/readability_linters.Rd index d1f32efec..9a3979583 100644 --- a/man/readability_linters.Rd +++ b/man/readability_linters.Rd @@ -34,6 +34,7 @@ The following linters are tagged with 'readability': \item{\code{\link{inner_combine_linter}}} \item{\code{\link{is_numeric_linter}}} \item{\code{\link{lengths_linter}}} +\item{\code{\link{library_call_linter}}} \item{\code{\link{line_length_linter}}} \item{\code{\link{matrix_apply_linter}}} \item{\code{\link{nested_ifelse_linter}}} From 2c20f5d56f951749b6bec6d5baba42aa3200767a Mon Sep 17 00:00:00 2001 From: Nicholas Masel Date: Thu, 3 Aug 2023 15:59:04 +0000 Subject: [PATCH 11/11] added news --- NEWS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.md b/NEWS.md index 63322d2f4..cbb36398e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,11 @@ * `inner_combine_linter()` no longer throws on length-1 calls to `c()` like `c(exp(2))` or `c(log(3))` (#2017, @MichaelChirico). Such usage is discouraged by `unnecessary_concatenation_linter()`, but `inner_combine_linter()` _per se_ does not apply. +## New and improved features + +* `library_call_linter()` can detect if all library calls are not at the top of your script (#2027, @nicholas-masel). + + ## Changes to defaults * `assignment_linter()` lints the {magrittr} assignment pipe `%<>%` (#2008, @MichaelChirico). This can be deactivated by setting the new argument `allow_pipe_assign` to `TRUE`.