diff --git a/DESCRIPTION b/DESCRIPTION index 24537cdc..20bebc99 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: rhino Title: A Framework for Enterprise Shiny Applications -Version: 1.6.0.9006 +Version: 1.6.0.9007 Authors@R: c( person("Kamil", "Żyła", role = c("aut", "cre"), email = "opensource+kamil@appsilon.com"), diff --git a/NAMESPACE b/NAMESPACE index aeefc620..251911f8 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -2,6 +2,7 @@ export(app) export(box_func_import_count_linter) +export(box_separate_calls_linter) export(box_trailing_commas_linter) export(box_universal_import_linter) export(build_js) diff --git a/NEWS.md b/NEWS.md index 777196d9..37395212 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,7 @@ * `box_universal_import_linter` checks if all imports are explicit. * `box_trailing_commas_linter` checks if statements include trailing commas. * `box_func_import_count_linter` checks if the number of function imports does not exceed the limit. + * `box_separate_calls_linter` checks if packages and modules are imported in separate statements. 2. Major refactor of `rhino::app()`: * The `request` parameter is now correctly forwarded to the UI function when using a `legacy_entrypoint` ([#395](https://github.com/Appsilon/rhino/issues/395)). diff --git a/R/box_linters.R b/R/box_linters.R index 8338160d..f2e70134 100644 --- a/R/box_linters.R +++ b/R/box_linters.R @@ -1,6 +1,9 @@ +# 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. #' @@ -30,6 +33,7 @@ #' ) #' #' @export +# nolint end box_func_import_count_linter <- function(max = 8L) { xpath <- glue::glue("//SYMBOL_PACKAGE[ (text() = 'box' and @@ -67,11 +71,78 @@ box_func_import_count_linter <- function(max = 8L) { }) } +# 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. @@ -106,6 +177,7 @@ box_func_import_count_linter <- function(max = 8L) { #' ) #' #' @export +# nolint end box_trailing_commas_linter <- function(check_functions = FALSE) { base_xpath <- "//SYMBOL_PACKAGE[ ( @@ -170,9 +242,12 @@ box_trailing_commas_linter <- function(check_functions = FALSE) { }) } +# 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` #' @@ -200,6 +275,7 @@ box_trailing_commas_linter <- function(check_functions = FALSE) { #' ) #' #' @export +# nolint end box_universal_import_linter <- function() { lint_message <- "Explicitly declare imports rather than universally import with `...`." diff --git a/inst/templates/app_structure/dot.lintr b/inst/templates/app_structure/dot.lintr index 0681c6a7..fa3c8d97 100644 --- a/inst/templates/app_structure/dot.lintr +++ b/inst/templates/app_structure/dot.lintr @@ -2,6 +2,7 @@ linters: linters_with_defaults( line_length_linter = line_length_linter(100), 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()`. diff --git a/man/box_func_import_count_linter.Rd b/man/box_func_import_count_linter.Rd index 1960ce5a..50d04d1e 100644 --- a/man/box_func_import_count_linter.Rd +++ b/man/box_func_import_count_linter.Rd @@ -14,6 +14,8 @@ 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 diff --git a/man/box_separate_calls_linter.Rd b/man/box_separate_calls_linter.Rd new file mode 100644 index 00000000..725ec8f5 --- /dev/null +++ b/man/box_separate_calls_linter.Rd @@ -0,0 +1,36 @@ +% 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 index b2d75892..a804bff8 100644 --- a/man/box_trailing_commas_linter.Rd +++ b/man/box_trailing_commas_linter.Rd @@ -17,6 +17,8 @@ A custom linter function for use with \code{r-lib/lintr} 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 diff --git a/man/box_universal_import_linter.Rd b/man/box_universal_import_linter.Rd index ce83b851..f57d0aed 100644 --- a/man/box_universal_import_linter.Rd +++ b/man/box_universal_import_linter.Rd @@ -11,6 +11,8 @@ 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 diff --git a/pkgdown/_pkgdown.yml b/pkgdown/_pkgdown.yml index f3958c37..cf861aaf 100644 --- a/pkgdown/_pkgdown.yml +++ b/pkgdown/_pkgdown.yml @@ -148,6 +148,7 @@ reference: - title: Linters contents: - box_func_import_count_linter + - box_separate_calls_linter - box_trailing_commas_linter - box_universal_import_linter diff --git a/tests/testthat/test-linter_box_separate_calls_linter.R b/tests/testthat/test-linter_box_separate_calls_linter.R new file mode 100644 index 00000000..a8219062 --- /dev/null +++ b/tests/testthat/test-linter_box_separate_calls_linter.R @@ -0,0 +1,87 @@ +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) +})