Merge pull request #542 from Appsilon/feature/box-import-separate-pac…

…kages-modules

Box import separate packages modules

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 = "[email protected]"),

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)

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)).

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 `...`."
 

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()`.

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
 

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)
+})