From d4d1bc782ebc9386be19ba908da2ca3921da937e Mon Sep 17 00:00:00 2001 From: Ricardo Rodrigo Basa Date: Wed, 5 Jun 2024 14:09:43 +0800 Subject: [PATCH] Use box linters (#583) * placeholder for box.linters * update rhino for use with box.linters * delint for box.linters * delint R source file * delint e2e test file * news update. delint rhino test files. * force box.linters >= 0.9.1 * info about #' @import * remove Linters section in pkgdown references --- DESCRIPTION | 4 +- NAMESPACE | 6 +- NEWS.md | 5 +- R/box_linters.R | 432 ------------------ R/linters.R | 7 + inst/templates/app_structure/dot.lintr | 9 +- .../unit_tests/tests/testthat/test-main.R | 2 +- man/box_alphabetical_calls_linter.Rd | 71 --- man/box_func_import_count_linter.Rd | 43 -- man/box_separate_calls_linter.Rd | 36 -- man/box_trailing_commas_linter.Rd | 50 -- man/box_universal_import_linter.Rd | 40 -- pkgdown/_pkgdown.yml | 8 - tests/e2e/app-files/hello.R | 38 +- tests/e2e/app-files/say_hello.R | 1 + tests/testthat/test-linter_box_alphabetical.R | 136 ------ ...test-linter_box_func_import_count_linter.R | 87 ---- .../test-linter_box_separate_calls_linter.R | 87 ---- .../test-linter_box_trailing_commas_linter.R | 105 ----- .../test-linter_box_universal_import_linter.R | 73 --- 20 files changed, 32 insertions(+), 1208 deletions(-) delete mode 100644 R/box_linters.R create mode 100644 R/linters.R delete mode 100644 man/box_alphabetical_calls_linter.Rd delete mode 100644 man/box_func_import_count_linter.Rd delete mode 100644 man/box_separate_calls_linter.Rd delete mode 100644 man/box_trailing_commas_linter.Rd delete mode 100644 man/box_universal_import_linter.Rd delete mode 100644 tests/testthat/test-linter_box_alphabetical.R delete mode 100644 tests/testthat/test-linter_box_func_import_count_linter.R delete mode 100644 tests/testthat/test-linter_box_separate_calls_linter.R delete mode 100644 tests/testthat/test-linter_box_trailing_commas_linter.R delete mode 100644 tests/testthat/test-linter_box_universal_import_linter.R diff --git a/DESCRIPTION b/DESCRIPTION index c7e0496b..a6e30cf2 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: rhino Title: A Framework for Enterprise Shiny Applications -Version: 1.7.0.9001 +Version: 1.7.0.9002 Authors@R: c( person("Kamil", "Żyła", role = c("aut", "cre"), email = "opensource+kamil@appsilon.com"), @@ -24,6 +24,7 @@ Depends: R (>= 2.10) Imports: box (>= 1.1.3), + box.linters (>= 0.9.1), cli, config, fs, @@ -39,7 +40,6 @@ Imports: testthat (>= 3.0.0), utils, withr, - xml2, yaml Suggests: covr, diff --git a/NAMESPACE b/NAMESPACE index 392b78ac..9a787719 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,11 +1,6 @@ # Generated by roxygen2: do not edit by hand export(app) -export(box_alphabetical_calls_linter) -export(box_func_import_count_linter) -export(box_separate_calls_linter) -export(box_trailing_commas_linter) -export(box_universal_import_linter) export(build_js) export(build_sass) export(diagnostics) @@ -20,3 +15,4 @@ export(pkg_remove) export(react_component) export(test_e2e) export(test_r) +import(box.linters) diff --git a/NEWS.md b/NEWS.md index 1615853b..4f747fa9 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,9 @@ # rhino (development version) -1. New `box_alphabetical_imports_linter` checks if all imports are alphabetically sorted. +1. All linter functions migrated to `box.linters`. New rhino projects will be configured to use linters from `box.linters`. To migrate an existing rhino project, run: + ```r + box.linters::use_box_lintr(type = "rhino") + ``` 2. Update GitHub Workflow template triggers. To update your workflow run: ```r file.copy( diff --git a/R/box_linters.R b/R/box_linters.R deleted file mode 100644 index 987884ff..00000000 --- a/R/box_linters.R +++ /dev/null @@ -1,432 +0,0 @@ -# nolint start: line_length_linter -#' `box` library alphabetical module and function imports linter -#' -#' Checks that module and function imports are sorted alphabetically. Aliases are -#' ignored. The sort check is on package/module names and attached function names. -#' See the [Explanation: Rhino style guide](https://appsilon.github.io/rhino/articles/explanation/rhino-style-guide.html) -#' to learn about the details. -#' -#' @return A custom linter function for use with `r-lib/lintr`. -#' -#' @examples -#' # will produce lints -#' lintr::lint( -#' text = "box::use(packageB, packageA)", -#' linters = box_alphabetical_calls_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(package[functionB, functionA])", -#' linters = box_alphabetical_calls_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(path/to/B, path/to/A)", -#' linters = box_alphabetical_calls_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(path/to/A[functionB, functionA])", -#' linters = box_alphabetical_calls_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(path/to/A[alias = functionB, functionA])", -#' linters = box_alphabetical_calls_linter() -#' ) -#' -#' # okay -#' lintr::lint( -#' text = "box::use(packageA, packageB)", -#' linters = box_alphabetical_calls_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(package[functionA, functionB])", -#' linters = box_alphabetical_calls_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(path/to/A, path/to/B)", -#' linters = box_alphabetical_calls_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(path/to/A[functionA, functionB])", -#' linters = box_alphabetical_calls_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(path/to/A[functionA, alias = functionB])", -#' linters = box_alphabetical_calls_linter() -#' ) -#' -#' @export -# nolint end -box_alphabetical_calls_linter <- function() { - xpath_base <- "//SYMBOL_PACKAGE[(text() = 'box' and - following-sibling::SYMBOL_FUNCTION_CALL[text() = 'use'])] - /parent::expr - /parent::expr" - - xpath <- paste(xpath_base, " - /child::expr[ - descendant::SYMBOL - ]") - - xpath_modules_with_functions <- paste(xpath_base, " - /child::expr[ - descendant::SYMBOL and - descendant::OP-LEFT-BRACKET - ]") - - xpath_functions <- "./descendant::expr/SYMBOL[ - ../preceding-sibling::OP-LEFT-BRACKET and - ../following-sibling::OP-RIGHT-BRACKET - ]" - - lint_message <- "Module and function imports must be sorted alphabetically." - - lintr::Linter(function(source_expression) { - if (!lintr::is_lint_level(source_expression, "expression")) { - return(list()) - } - - xml <- source_expression$xml_parsed_content - xml_nodes <- xml2::xml_find_all(xml, xpath) - modules_called <- xml2::xml_text(xml_nodes) - modules_check <- modules_called == sort(modules_called) - - unsorted_modules <- which(modules_check == FALSE) - module_lint <- lintr::xml_nodes_to_lints( - xml_nodes[unsorted_modules], - source_expression = source_expression, - lint_message = lint_message, - type = "style" - ) - - xml_nodes_with_functions <- xml2::xml_find_all(xml_nodes, xpath_modules_with_functions) - - function_lint <- lapply(xml_nodes_with_functions, function(xml_node) { - imported_functions <- xml2::xml_find_all(xml_node, xpath_functions) - functions_called <- xml2::xml_text(imported_functions) - functions_check <- functions_called == sort(functions_called) - unsorted_functions <- which(functions_check == FALSE) - - lintr::xml_nodes_to_lints( - imported_functions[unsorted_functions], - source_expression = source_expression, - lint_message = lint_message, - type = "style" - ) - }) - - c(module_lint, function_lint) - }) -} - -# nolint start: line_length_linter -#' `box` library function import count linter -#' -#' Checks that function imports do not exceed the defined `max`. -#' See the [Explanation: Rhino style guide](https://appsilon.github.io/rhino/articles/explanation/rhino-style-guide.html) -#' to learn about the details. -#' -#' @param max Maximum function imports allowed between `[` and `]`. Defaults to 8. -#' -#' @return A custom linter function for use with `r-lib/lintr`. -#' -#' @examples -#' # will produce lints -#' lintr::lint( -#' text = "box::use(package[one, two, three, four, five, six, seven, eight, nine])", -#' linters = box_func_import_count_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(package[one, two, three, four])", -#' linters = box_func_import_count_linter(3) -#' ) -#' -#' # okay -#' lintr::lint( -#' text = "box::use(package[one, two, three, four, five])", -#' linters = box_func_import_count_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(package[one, two, three])", -#' linters = box_func_import_count_linter(3) -#' ) -#' -#' @export -# nolint end -box_func_import_count_linter <- function(max = 8L) { - xpath <- glue::glue("//SYMBOL_PACKAGE[ - (text() = 'box' and - following-sibling::SYMBOL_FUNCTION_CALL[text() = 'use']) - ] - /parent::expr - /parent::expr - /descendant::OP-LEFT-BRACKET[ - count( - following-sibling::expr[ - count(. | ..//OP-RIGHT-BRACKET/preceding-sibling::expr) = - count(../OP-RIGHT-BRACKET/preceding-sibling::expr) - ] - ) > {max} - ] - /parent::expr") - - lint_message <- glue::glue("Limit the function imports to a max of {max}.") - - lintr::Linter(function(source_expression) { - if (!lintr::is_lint_level(source_expression, "file")) { - return(list()) - } - - xml <- source_expression$full_xml_parsed_content - - bad_expr <- xml2::xml_find_all(xml, xpath) - - lintr::xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = lint_message, - type = "style" - ) - }) -} - -# nolint start: line_length_linter -#' `box` library separate packages and module imports linter -#' -#' Checks that packages and modules are imported in separate `box::use()` statements. -#' See the [Explanation: Rhino style guide](https://appsilon.github.io/rhino/articles/explanation/rhino-style-guide.html) -#' to learn about the details. -#' -#' @return A custom linter function for use with `r-lib/lintr` -#' -#' @examples -#' # will produce lints -#' lintr::lint( -#' text = "box::use(package, path/to/file)", -#' linters = box_separate_calls_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(path/to/file, package)", -#' linters = box_separate_calls_linter() -#' ) -#' -#' # okay -#' lintr::lint( -#' text = "box::use(package1, package2) -#' box::use(path/to/file1, path/to/file2)", -#' linters = box_separate_calls_linter() -#' ) -#' -#' @export -# nolint end -box_separate_calls_linter <- function() { - xpath <- " - //SYMBOL_PACKAGE[(text() = 'box' and following-sibling::SYMBOL_FUNCTION_CALL[text() = 'use'])] - /parent::expr - /parent::expr[ - ( - ./child::expr[child::SYMBOL] or - ./child::expr[ - child::expr[child::SYMBOL] and child::OP-LEFT-BRACKET - ] - ) and - ./child::expr[child::expr[child::OP-SLASH]] - ] - " - lint_message <- "Separate packages and modules in their respective box::use() calls." - - lintr::Linter(function(source_expression) { - if (!lintr::is_lint_level(source_expression, "file")) { - return(list()) - } - - xml <- source_expression$full_xml_parsed_content - - bad_expr <- xml2::xml_find_all(xml, xpath) - - lintr::xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = lint_message, - type = "style" - ) - }) -} - -# nolint start: line_length_linter -#' `box` library trailing commas linter -#' -#' Checks that all `box:use` imports have a trailing comma. This applies to -#' package or module imports between `(` and `)`, and, optionally, function imports between -#' `[` and `]`. Take note that `lintr::commas_linter()` may come into play. -#' See the [Explanation: Rhino style guide](https://appsilon.github.io/rhino/articles/explanation/rhino-style-guide.html) -#' to learn about the details. -#' -#' @param check_functions Boolean flag to include function imports between `[` and `]`. -#' Defaults to FALSE. -#' -#' @return A custom linter function for use with `r-lib/lintr` -#' -#' @examples -#' # will produce lints -#' lintr::lint( -#' text = "box::use(base, rlang)", -#' linters = box_trailing_commas_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use( -#' dplyr[select, mutate] -#' )", -#' linters = box_trailing_commas_linter() -#' ) -#' -#' # okay -#' lintr::lint( -#' text = "box::use(base, rlang, )", -#' linters = box_trailing_commas_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use( -#' dplyr[select, mutate], -#' )", -#' linters = box_trailing_commas_linter() -#' ) -#' -#' @export -# nolint end -box_trailing_commas_linter <- function(check_functions = FALSE) { - base_xpath <- "//SYMBOL_PACKAGE[ - ( - text() = 'box' and - following-sibling::SYMBOL_FUNCTION_CALL[text() = 'use'] - ) - ] - /parent::expr - " - - right_paren_xpath <- paste( - base_xpath, - "/following-sibling::OP-RIGHT-PAREN[ - preceding-sibling::*[1][not(self::OP-COMMA)] - ] - " - ) - - right_bracket_xpath <- paste( - base_xpath, - "/parent::expr - /descendant::OP-RIGHT-BRACKET[ - preceding-sibling::*[1][ - not(self::OP-COMMA) and - not(self::expr[ - SYMBOL[text() = '...'] - ]) - ] - ] - " - ) - - lintr::Linter(function(source_expression) { - if (!lintr::is_lint_level(source_expression, "file")) { - return(list()) - } - - xml <- source_expression$full_xml_parsed_content - - bad_right_paren_expr <- xml2::xml_find_all(xml, right_paren_xpath) - - paren_lints <- lintr::xml_nodes_to_lints( - bad_right_paren_expr, - source_expression = source_expression, - lint_message = "Always have a trailing comma at the end of imports, before a `)`.", - type = "style" - ) - - bracket_lints <- NULL - if (check_functions) { - bad_right_bracket_expr <- xml2::xml_find_all(xml, right_bracket_xpath) - - bracket_lints <- lintr::xml_nodes_to_lints( - bad_right_bracket_expr, - source_expression = source_expression, - lint_message = "Always have a trailing comma at the end of imports, before a `]`.", - type = "style" - ) - } - - c(paren_lints, bracket_lints) - }) -} - -# nolint start: line_length_linter -#' `box` library universal import linter -#' -#' Checks that all function imports are explicit. `package[...]` is not used. -#' See the [Explanation: Rhino style guide](https://appsilon.github.io/rhino/articles/explanation/rhino-style-guide.html) -#' to learn about the details. -#' -#' @return A custom linter function for use with `r-lib/lintr` -#' -#' @examples -#' # will produce lints -#' lintr::lint( -#' text = "box::use(base[...])", -#' linters = box_universal_import_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(path/to/file[...])", -#' linters = box_universal_import_linter() -#' ) -#' -#' # okay -#' lintr::lint( -#' text = "box::use(base[print])", -#' linters = box_universal_import_linter() -#' ) -#' -#' lintr::lint( -#' text = "box::use(path/to/file[do_something])", -#' linters = box_universal_import_linter() -#' ) -#' -#' @export -# nolint end -box_universal_import_linter <- function() { - lint_message <- "Explicitly declare imports rather than universally import with `...`." - - xpath <- " - //SYMBOL_PACKAGE[(text() = 'box' and following-sibling::SYMBOL_FUNCTION_CALL[text() = 'use'])] - /parent::expr - /parent::expr - //SYMBOL[text() = '...'] - " - - lintr::Linter(function(source_expression) { - if (!lintr::is_lint_level(source_expression, "file")) { - return(list()) - } - - xml <- source_expression$full_xml_parsed_content - - bad_expr <- xml2::xml_find_all(xml, xpath) - - lintr::xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = lint_message, - type = "style" - ) - }) -} diff --git a/R/linters.R b/R/linters.R new file mode 100644 index 00000000..ad8e83e4 --- /dev/null +++ b/R/linters.R @@ -0,0 +1,7 @@ +# R CMD Check "notes" that all declared Imports in DESCRIPTION should be used. box.linters is not +# used by the rhino package itself, but by the initialized rhino app. There are no calls to +# box.linters in the R/ or tests/ folders. box.linters is called in the rhino app dot.lintr +# template in inst/templates/app_structure. The following lines manually imports box.linters making +# R CMD Check happy. +#' @import box.linters +NULL diff --git a/inst/templates/app_structure/dot.lintr b/inst/templates/app_structure/dot.lintr index c285e634..da7cb04e 100644 --- a/inst/templates/app_structure/dot.lintr +++ b/inst/templates/app_structure/dot.lintr @@ -1,10 +1,5 @@ linters: linters_with_defaults( - line_length_linter = line_length_linter(100), - box_alphabetical_calls_linter = rhino::box_alphabetical_calls_linter(), - box_func_import_count_linter = rhino::box_func_import_count_linter(), - box_separate_calls_linter = rhino::box_separate_calls_linter(), - box_trailing_commas_linter = rhino::box_trailing_commas_linter(), - box_universal_import_linter = rhino::box_universal_import_linter(), - object_usage_linter = NULL # Does not work with `box::use()`. + defaults = box.linters::rhino_default_linters, + line_length_linter = line_length_linter(100) ) diff --git a/inst/templates/unit_tests/tests/testthat/test-main.R b/inst/templates/unit_tests/tests/testthat/test-main.R index 003ae7e8..b8ddd76a 100644 --- a/inst/templates/unit_tests/tests/testthat/test-main.R +++ b/inst/templates/unit_tests/tests/testthat/test-main.R @@ -3,7 +3,7 @@ box::use( testthat[expect_true, test_that], ) box::use( - app/main[server, ui], + app/main[server], ) test_that("main server works", { diff --git a/man/box_alphabetical_calls_linter.Rd b/man/box_alphabetical_calls_linter.Rd deleted file mode 100644 index 6010a604..00000000 --- a/man/box_alphabetical_calls_linter.Rd +++ /dev/null @@ -1,71 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/box_linters.R -\name{box_alphabetical_calls_linter} -\alias{box_alphabetical_calls_linter} -\title{\code{box} library alphabetical module and function imports linter} -\usage{ -box_alphabetical_calls_linter() -} -\value{ -A custom linter function for use with \code{r-lib/lintr}. -} -\description{ -Checks that module and function imports are sorted alphabetically. Aliases are -ignored. The sort check is on package/module names and attached function names. -See the \href{https://appsilon.github.io/rhino/articles/explanation/rhino-style-guide.html}{Explanation: Rhino style guide} -to learn about the details. -} -\examples{ -# will produce lints -lintr::lint( - text = "box::use(packageB, packageA)", - linters = box_alphabetical_calls_linter() -) - -lintr::lint( - text = "box::use(package[functionB, functionA])", - linters = box_alphabetical_calls_linter() -) - -lintr::lint( - text = "box::use(path/to/B, path/to/A)", - linters = box_alphabetical_calls_linter() -) - -lintr::lint( - text = "box::use(path/to/A[functionB, functionA])", - linters = box_alphabetical_calls_linter() -) - -lintr::lint( - text = "box::use(path/to/A[alias = functionB, functionA])", - linters = box_alphabetical_calls_linter() -) - -# okay -lintr::lint( - text = "box::use(packageA, packageB)", - linters = box_alphabetical_calls_linter() -) - -lintr::lint( - text = "box::use(package[functionA, functionB])", - linters = box_alphabetical_calls_linter() -) - -lintr::lint( - text = "box::use(path/to/A, path/to/B)", - linters = box_alphabetical_calls_linter() -) - -lintr::lint( - text = "box::use(path/to/A[functionA, functionB])", - linters = box_alphabetical_calls_linter() -) - -lintr::lint( - text = "box::use(path/to/A[functionA, alias = functionB])", - linters = box_alphabetical_calls_linter() -) - -} diff --git a/man/box_func_import_count_linter.Rd b/man/box_func_import_count_linter.Rd deleted file mode 100644 index 50d04d1e..00000000 --- a/man/box_func_import_count_linter.Rd +++ /dev/null @@ -1,43 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/box_linters.R -\name{box_func_import_count_linter} -\alias{box_func_import_count_linter} -\title{\code{box} library function import count linter} -\usage{ -box_func_import_count_linter(max = 8L) -} -\arguments{ -\item{max}{Maximum function imports allowed between \code{[} and \verb{]}. Defaults to 8.} -} -\value{ -A custom linter function for use with \code{r-lib/lintr}. -} -\description{ -Checks that function imports do not exceed the defined \code{max}. -See the \href{https://appsilon.github.io/rhino/articles/explanation/rhino-style-guide.html}{Explanation: Rhino style guide} -to learn about the details. -} -\examples{ -# will produce lints -lintr::lint( - text = "box::use(package[one, two, three, four, five, six, seven, eight, nine])", - linters = box_func_import_count_linter() -) - -lintr::lint( - text = "box::use(package[one, two, three, four])", - linters = box_func_import_count_linter(3) -) - -# okay -lintr::lint( - text = "box::use(package[one, two, three, four, five])", - linters = box_func_import_count_linter() -) - -lintr::lint( - text = "box::use(package[one, two, three])", - linters = box_func_import_count_linter(3) -) - -} diff --git a/man/box_separate_calls_linter.Rd b/man/box_separate_calls_linter.Rd deleted file mode 100644 index 725ec8f5..00000000 --- a/man/box_separate_calls_linter.Rd +++ /dev/null @@ -1,36 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/box_linters.R -\name{box_separate_calls_linter} -\alias{box_separate_calls_linter} -\title{\code{box} library separate packages and module imports linter} -\usage{ -box_separate_calls_linter() -} -\value{ -A custom linter function for use with \code{r-lib/lintr} -} -\description{ -Checks that packages and modules are imported in separate \code{box::use()} statements. -See the \href{https://appsilon.github.io/rhino/articles/explanation/rhino-style-guide.html}{Explanation: Rhino style guide} -to learn about the details. -} -\examples{ -# will produce lints -lintr::lint( - text = "box::use(package, path/to/file)", - linters = box_separate_calls_linter() -) - -lintr::lint( - text = "box::use(path/to/file, package)", - linters = box_separate_calls_linter() -) - -# okay -lintr::lint( - text = "box::use(package1, package2) - box::use(path/to/file1, path/to/file2)", - linters = box_separate_calls_linter() -) - -} diff --git a/man/box_trailing_commas_linter.Rd b/man/box_trailing_commas_linter.Rd deleted file mode 100644 index a804bff8..00000000 --- a/man/box_trailing_commas_linter.Rd +++ /dev/null @@ -1,50 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/box_linters.R -\name{box_trailing_commas_linter} -\alias{box_trailing_commas_linter} -\title{\code{box} library trailing commas linter} -\usage{ -box_trailing_commas_linter(check_functions = FALSE) -} -\arguments{ -\item{check_functions}{Boolean flag to include function imports between \code{[} and \verb{]}. -Defaults to FALSE.} -} -\value{ -A custom linter function for use with \code{r-lib/lintr} -} -\description{ -Checks that all \code{box:use} imports have a trailing comma. This applies to -package or module imports between \code{(} and \verb{)}, and, optionally, function imports between -\code{[} and \verb{]}. Take note that \code{lintr::commas_linter()} may come into play. -See the \href{https://appsilon.github.io/rhino/articles/explanation/rhino-style-guide.html}{Explanation: Rhino style guide} -to learn about the details. -} -\examples{ -# will produce lints -lintr::lint( - text = "box::use(base, rlang)", - linters = box_trailing_commas_linter() -) - -lintr::lint( - text = "box::use( - dplyr[select, mutate] - )", - linters = box_trailing_commas_linter() -) - -# okay -lintr::lint( - text = "box::use(base, rlang, )", - linters = box_trailing_commas_linter() -) - -lintr::lint( - text = "box::use( - dplyr[select, mutate], - )", - linters = box_trailing_commas_linter() -) - -} diff --git a/man/box_universal_import_linter.Rd b/man/box_universal_import_linter.Rd deleted file mode 100644 index f57d0aed..00000000 --- a/man/box_universal_import_linter.Rd +++ /dev/null @@ -1,40 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/box_linters.R -\name{box_universal_import_linter} -\alias{box_universal_import_linter} -\title{\code{box} library universal import linter} -\usage{ -box_universal_import_linter() -} -\value{ -A custom linter function for use with \code{r-lib/lintr} -} -\description{ -Checks that all function imports are explicit. \code{package[...]} is not used. -See the \href{https://appsilon.github.io/rhino/articles/explanation/rhino-style-guide.html}{Explanation: Rhino style guide} -to learn about the details. -} -\examples{ -# will produce lints -lintr::lint( - text = "box::use(base[...])", - linters = box_universal_import_linter() -) - -lintr::lint( - text = "box::use(path/to/file[...])", - linters = box_universal_import_linter() -) - -# okay -lintr::lint( - text = "box::use(base[print])", - linters = box_universal_import_linter() -) - -lintr::lint( - text = "box::use(path/to/file[do_something])", - linters = box_universal_import_linter() -) - -} diff --git a/pkgdown/_pkgdown.yml b/pkgdown/_pkgdown.yml index 6cd9b0d2..17ab1894 100644 --- a/pkgdown/_pkgdown.yml +++ b/pkgdown/_pkgdown.yml @@ -148,14 +148,6 @@ reference: - diagnostics - test_e2e -- title: Linters - contents: - - box_alphabetical_calls_linter - - box_func_import_count_linter - - box_separate_calls_linter - - box_trailing_commas_linter - - box_universal_import_linter - - title: Data contents: - rhinos diff --git a/tests/e2e/app-files/hello.R b/tests/e2e/app-files/hello.R index 44f36947..42b28ac4 100644 --- a/tests/e2e/app-files/hello.R +++ b/tests/e2e/app-files/hello.R @@ -1,40 +1,30 @@ box::use( - shiny[ # nolint - actionButton, - bootstrapPage, - isolate, - moduleServer, - NS, - observe, - observeEvent, - renderText, - tags, - textInput, - textOutput - ], + shiny, ) -box::use(app/logic/say_hello[say_hello], ) +box::use( + app/logic/say_hello[say_hello], +) #' @export ui <- function(id) { - ns <- NS(id) - bootstrapPage( - tags$div( + ns <- shiny$NS(id) + shiny$bootstrapPage( + shiny$tags$div( class = "input-and-click", - textInput(ns("name"), label = NULL, value = NULL), - actionButton(ns("say_hello"), label = "Say Hello") + shiny$textInput(ns("name"), label = NULL, value = NULL), + shiny$actionButton(ns("say_hello"), label = "Say Hello") ), - textOutput(ns("message")) + shiny$textOutput(ns("message")) ) } #' @export server <- function(id) { - moduleServer(id, function(input, output, session) { + shiny$moduleServer(id, function(input, output, session) { ns <- session$ns - observe({ + shiny$observe({ is_name_empty <- is.null(input$name) || input$name == "" session$sendCustomMessage( @@ -43,9 +33,9 @@ server <- function(id) { ) }) - observeEvent( + shiny$observeEvent( input$say_hello, { - output$message <- renderText(say_hello(isolate(input$name))) + output$message <- shiny$renderText(say_hello(shiny$isolate(input$name))) } ) }) diff --git a/tests/e2e/app-files/say_hello.R b/tests/e2e/app-files/say_hello.R index c69dbd17..7e221c9c 100644 --- a/tests/e2e/app-files/say_hello.R +++ b/tests/e2e/app-files/say_hello.R @@ -1,3 +1,4 @@ +#' @export say_hello <- function(name) { paste0("Hello, ", name, "!") } diff --git a/tests/testthat/test-linter_box_alphabetical.R b/tests/testthat/test-linter_box_alphabetical.R deleted file mode 100644 index e6ae9924..00000000 --- a/tests/testthat/test-linter_box_alphabetical.R +++ /dev/null @@ -1,136 +0,0 @@ -test_that("box_alphabetical_calls_linter() skips allowed box::use() calls", { - linter <- box_alphabetical_calls_linter() - - good_box_calls_1 <- "box::use( - dplyr, - shiny, - tidyr, - )" - - good_box_calls_2 <- "box::use( - path/to/fileA, - path/to/fileB, - path/to/fileC, - )" - - good_box_calls_3 <- "box::use( - dplyr[filter, mutate, select], - shiny, - tidyr[long, pivot, wide], - )" - - good_box_calls_4 <- "box::use( - path/to/fileA[functionA, functionB, functionC], - path/to/fileB, - path/to/fileC[functionD, functionE, functionF], - )" - - lintr::expect_lint(good_box_calls_1, NULL, linter) - lintr::expect_lint(good_box_calls_2, NULL, linter) - lintr::expect_lint(good_box_calls_3, NULL, linter) - lintr::expect_lint(good_box_calls_4, NULL, linter) -}) - -test_that("box_alphabetical_calls_linter() ignores aliases in box::use() calls", { - linter <- box_alphabetical_calls_linter() - - good_box_calls_alias_1 <- "box::use( - dplyr, - alias = shiny, - tidyr, - )" - - good_box_calls_alias_2 <- "box::use( - path/to/fileA, - a = path/to/fileB, - path/to/fileC, - )" - - good_box_calls_alias_3 <- "box::use( - dplyr[filter, alias = mutate, select], - shiny, - tidyr[long, pivot, wide], - )" - - good_box_calls_alias_4 <- "box::use( - path/to/fileA[functionA, alias = functionB, functionC], - path/to/fileB, - path/to/fileC[functionD, functionE, functionF], - )" - - lintr::expect_lint(good_box_calls_alias_1, NULL, linter) - lintr::expect_lint(good_box_calls_alias_2, NULL, linter) - lintr::expect_lint(good_box_calls_alias_3, NULL, linter) - lintr::expect_lint(good_box_calls_alias_4, NULL, linter) -}) - -test_that("box_alphabetical_calls_linter() blocks unsorted imports in box::use() call", { - linter <- box_alphabetical_calls_linter() - - bad_box_calls_1 <- "box::use( - dplyr, - tidyr, - shiny, - )" - - bad_box_calls_2 <- "box::use( - path/to/fileC, - path/to/fileB, - path/to/fileA, - )" - - bad_box_calls_3 <- "box::use( - dplyr[filter, mutate, select], - shiny, - tidyr[wide, pivot, long], - )" - - bad_box_calls_4 <- "box::use( - dplyr[select, mutate, filter], - shiny, - tidyr[wide, pivot, long], - )" - - bad_box_calls_5 <- "box::use( - path/to/fileA[functionC, functionB, functionA], - path/to/fileB, - path/to/fileC[functionD, functionE, functionF], - )" - - bad_box_calls_6 <- "box::use( - path/to/fileA[functionC, functionB, functionA], - path/to/fileB, - path/to/fileC[functionF, functionE, functionD], - )" - - lint_message <- rex::rex("Module and function imports must be sorted alphabetically.") - - lintr::expect_lint(bad_box_calls_1, list( - list(message = lint_message, line_number = 3), - list(message = lint_message, line_number = 4) - ), linter) - lintr::expect_lint(bad_box_calls_2, list( - list(message = lint_message, line_number = 2), - list(message = lint_message, line_number = 4) - ), linter) - lintr::expect_lint(bad_box_calls_3, list( - list(message = lint_message, line_number = 4, column_number = 11), - list(message = lint_message, line_number = 4, column_number = 24) - ), linter) - lintr::expect_lint(bad_box_calls_4, list( - list(message = lint_message, line_number = 2, column_number = 11), - list(message = lint_message, line_number = 2, column_number = 27), - list(message = lint_message, line_number = 4, column_number = 11), - list(message = lint_message, line_number = 4, column_number = 24) - ), linter) - lintr::expect_lint(bad_box_calls_5, list( - list(message = lint_message, line_number = 2, column_number = 19), - list(message = lint_message, line_number = 2, column_number = 41) - ), linter) - lintr::expect_lint(bad_box_calls_6, list( - list(message = lint_message, line_number = 2, column_number = 19), - list(message = lint_message, line_number = 2, column_number = 41), - list(message = lint_message, line_number = 4, column_number = 19), - list(message = lint_message, line_number = 4, column_number = 41) - ), linter) -}) diff --git a/tests/testthat/test-linter_box_func_import_count_linter.R b/tests/testthat/test-linter_box_func_import_count_linter.R deleted file mode 100644 index 6d080c91..00000000 --- a/tests/testthat/test-linter_box_func_import_count_linter.R +++ /dev/null @@ -1,87 +0,0 @@ -lint_message <- function(max = 5L) { - rex::rex(glue::glue("Limit the function imports to a max of {max}.")) -} - -test_that("box_func_import_count_linter skips allowed function count.", { - linter <- box_func_import_count_linter(max = 5) - - five_function_imports <- "box::use( - dplyr[ - arrange, - select, - mutate, - distinct, - filter, - ], - )" - - five_function_imports_inline <- "box::use( - dplyr[arrange, select, mutate, distinct, filter, ], - )" - - lintr::expect_lint(five_function_imports, NULL, linter) - lintr::expect_lint(five_function_imports_inline, NULL, linter) -}) - -test_that("box_func_import_count_linter skips allowed function count supplied max", { - linter <- box_func_import_count_linter(max = 3) - - three_function_imports <- "box::use( - dplyr[ - arrange, - select, - mutate, - ], - )" - - three_function_imports_inline <- "box::use( - dplyr[arrange, select, mutate, ], - )" - - lintr::expect_lint(three_function_imports, NULL, linter) - lintr::expect_lint(three_function_imports_inline, NULL, linter) -}) - -test_that("box_func_import_count_linter blocks function imports more than max", { - max <- 5 - linter <- box_func_import_count_linter(max = max) - lint_msg <- lint_message(max) - - six_function_imports <- "box::use( - dplyr[ - arrange, - select, - mutate, - distinct, - filter, - slice, - ], - )" - - six_function_imports_inline <- "box::use( - dplyr[arrange, select, mutate, distinct, filter, slice, ], - )" - - lintr::expect_lint(six_function_imports, list(message = lint_msg), linter) - lintr::expect_lint(six_function_imports_inline, list(message = lint_msg), linter) -}) - -test_that("box_func_import_count_linter resepect # nolint", { - linter <- box_func_import_count_linter() - - no_lint <- "box::use( - dplyr[ # nolint - arrange, - select, - mutate, - distinct - ], - )" - - no_lint_inline <- "box::use( - dplyr[arrange, select, mutate, distinct], # nolint - )" - - lintr::expect_lint(no_lint, NULL, linter) - lintr::expect_lint(no_lint_inline, NULL, linter) -}) diff --git a/tests/testthat/test-linter_box_separate_calls_linter.R b/tests/testthat/test-linter_box_separate_calls_linter.R deleted file mode 100644 index a8219062..00000000 --- a/tests/testthat/test-linter_box_separate_calls_linter.R +++ /dev/null @@ -1,87 +0,0 @@ -test_that("box_separate_calls_linter skips allowed box::use() calls", { - linter <- box_separate_calls_linter() - - good_box_calls_1 <- "box::use( - dplyr, - shiny, - ) - - box::use( - path/to/file1, - path/to/file2, - )" - - good_box_calls_2 <- "box::use( - dplyr[filter, select], - shiny, - ) - - box::use( - path/to/file1[function1, function2], - path/to/file2, - )" - - lintr::expect_lint(good_box_calls_1, NULL, linter) - lintr::expect_lint(good_box_calls_2, NULL, linter) -}) - -test_that("box_separate_calls_linter blocks packages and modules in same box::use() call", { - linter <- box_separate_calls_linter() - - bad_box_call_1 <- "box::use( - dplyr, - path/to/file, - )" - - bad_box_call_2 <- "box::use( - path/to/file, - dplyr, - )" - - bad_box_call_3 <- "box::use( - path/to/file, - dplyr[filter, select], - )" - - bad_box_call_4 <- "box::use( - path/to/file[function1, function2], - dplyr, - )" - - bad_box_call_5 <- "box::use( - path/to/file[function1, function2], - dplyr[filter, select], - )" - - bad_box_call_6 <- "box::use( - a = path/to/file, - dplyr, - )" - - bad_box_call_7 <- "box::use( - path/to/file, - a = dplyr, - )" - - bad_box_call_8 <- "box::use( - path/to/file, - dplyr[a = filter, select], - )" - - bad_box_call_9 <- "box::use( - path/to/file[a = function1, function2], - dplyr, - )" - - lint_message <- rex::rex("Separate packages and modules in their respective box::use() calls.") - - lintr::expect_lint(bad_box_call_1, list(message = lint_message), linter) - lintr::expect_lint(bad_box_call_2, list(message = lint_message), linter) - lintr::expect_lint(bad_box_call_3, list(message = lint_message), linter) - lintr::expect_lint(bad_box_call_4, list(message = lint_message), linter) - lintr::expect_lint(bad_box_call_5, list(message = lint_message), linter) - lintr::expect_lint(bad_box_call_6, list(message = lint_message), linter) - lintr::expect_lint(bad_box_call_7, list(message = lint_message), linter) - lintr::expect_lint(bad_box_call_8, list(message = lint_message), linter) - lintr::expect_lint(bad_box_call_9, list(message = lint_message), linter) -}) diff --git a/tests/testthat/test-linter_box_trailing_commas_linter.R b/tests/testthat/test-linter_box_trailing_commas_linter.R deleted file mode 100644 index 8aadc0ab..00000000 --- a/tests/testthat/test-linter_box_trailing_commas_linter.R +++ /dev/null @@ -1,105 +0,0 @@ -test_that("box_trailing_commas_linter skips allowed package import usage", { - linter <- box_trailing_commas_linter() - - good_package_commas <- "box::use( - dplyr, - stringr[ - select, - mutate - ], - )" - - good_package_commas_inline <- "box::use(dplyr, stringr[select, mutate], )" - - good_module_commas <- "box::use( - path/to/file1, - path/to/file2[ - first_function, - second_function - ], - )" - - good_module_commas_inline <- "box::use(path/to/file1, path/to/file2, )" - - lintr::expect_lint(good_package_commas, NULL, linter) - lintr::expect_lint(good_package_commas_inline, NULL, linter) - lintr::expect_lint(good_module_commas, NULL, linter) - lintr::expect_lint(good_module_commas_inline, NULL, linter) -}) - -test_that("box_trailing_commas_linter blocks no trailing commas in package imports", { - linter <- box_trailing_commas_linter() - - bad_package_commas <- "box::use( - dplyr, - stringr - )" - - bad_package_commas_inline <- "box::use(dplyr, stringr)" - - paren_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `)`.") - - lintr::expect_lint(bad_package_commas, list(message = paren_lint_msg), linter) - lintr::expect_lint(bad_package_commas_inline, list(message = paren_lint_msg), linter) -}) - -test_that("box_trailing_commas_linter check_functions = TRUE blocks no trailing commas", { - linter <- box_trailing_commas_linter(check_functions = TRUE) - - bracket_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `]`.") - - bad_package_function_commas <- "box::use( - dplyr, - stringr[ - select, - mutate - ], - )" - - bad_pkg_function_commas_inline <- "box::use(stringr[select, mutate],)" - - lintr::expect_lint(bad_package_function_commas, list(message = bracket_lint_msg), linter) - lintr::expect_lint(bad_pkg_function_commas_inline, list(message = bracket_lint_msg), linter) -}) - -test_that("box_trailing_comma_linter blocks no trailing commas in module imports", { - linter <- box_trailing_commas_linter() - - bad_module_commas <- "box::use( - path/to/file1, - path/to/file2 - )" - - bad_module_commas_inline <- "box::use(path/to/file1, path/to/file2)" - - paren_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `)`.") - - lintr::expect_lint(bad_module_commas, list(message = paren_lint_msg), linter) - lintr::expect_lint(bad_module_commas_inline, list(message = paren_lint_msg), linter) -}) - -test_that("box_trailing_commas_linter check_functions = TRUE blocks no trailing commas", { - linter <- box_trailing_commas_linter(check_functions = TRUE) - - bad_module_function_commas <- "box::use( - path/to/file2[ - first_function, - second_function - ], - )" - - bad_mod_function_commas_inline <- "box::use(path/to/file2[first_function, second_function], )" - - bracket_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `]`.") - - lintr::expect_lint(bad_module_function_commas, list(message = bracket_lint_msg), linter) - lintr::expect_lint(bad_mod_function_commas_inline, list(message = bracket_lint_msg), linter) -}) - -test_that("box_trailing_commas_linter should not lint outside of `box::use()`", { - linter <- box_trailing_commas_linter() - - should_not_lint <- "x <- c(1, 2, 3)" - - lintr::expect_lint(should_not_lint, NULL, linter) -}) diff --git a/tests/testthat/test-linter_box_universal_import_linter.R b/tests/testthat/test-linter_box_universal_import_linter.R deleted file mode 100644 index 1f4e39bb..00000000 --- a/tests/testthat/test-linter_box_universal_import_linter.R +++ /dev/null @@ -1,73 +0,0 @@ -test_that("box_universal_count_linter skips allowed package import usage", { - linter <- box_universal_import_linter() - - good_package_imports <- "box::use( - dplyr[select, mutate, ], - stringr[str_sub, str_match, ], - ) - " - - lintr::expect_lint(good_package_imports, NULL, linter) -}) - -test_that("box_universal_count_linter skips allowed module import usage", { - linter <- box_universal_import_linter() - - good_module_imports <- "box::use( - path/to/file1[do_something, do_another, ], - path/to/file2[find_x, find_y, ], - ) - " - - lintr::expect_lint(good_module_imports, NULL, linter) -}) - -test_that("box_universal_count_linter blocks disallowed package import usage", { - linter <- box_universal_import_linter() - - bad_package_imports <- "box::use( - dplyr[...], - stringr[str_sub, str_match, ], - ) - " - - lint_msg <- rex::rex("Explicitly declare imports rather than universally import with `...`.") - - lintr::expect_lint(bad_package_imports, list(message = lint_msg), linter) -}) - -test_that("box_universal_count_linter blocks disallowed module import usage", { - linter <- box_universal_import_linter() - - bad_module_imports <- "box::use( - path/to/file1[...], - path/to/file2[find_x, find_y, ], - ) - " - - lint_msg <- rex::rex("Explicitly declare imports rather than universally import with `...`.") - - lintr::expect_lint(bad_module_imports, list(message = lint_msg), linter) -}) - -test_that("box_universal_count_linter skips three dots in function declarations and calls", { - linter <- box_universal_import_linter() - - function_with_three_dots <- "some_function <- function(...) { - sum(...) - } - " - - lintr::expect_lint(function_with_three_dots, NULL, linter) -}) - -test_that("box_universal_count_linter respects #nolint", { - linter <- box_universal_import_linter() - - no_lint <- "box::use( - shiny[...], # nolint - ) - " - - lintr::expect_lint(no_lint, NULL, linter) -})