Merge pull request #544 from Appsilon/feature/box-alphabetical

Box alphabetical imports linter.

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: rhino
 Title: A Framework for Enterprise Shiny Applications
-Version: 1.7.0.9000
+Version: 1.7.0.9001
 Authors@R:
   c(
     person("Kamil", "Żyła", role = c("aut", "cre"), email = "[email protected]"),

NAMESPACE

@@ -1,6 +1,7 @@
 # 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)

NEWS.md

@@ -1,5 +1,7 @@
 # rhino (development version)
 
+1. New `box_alphabetical_imports_linter` checks if all imports are alphabetically sorted.
+
 # [rhino 1.7.0](https://github.com/Appsilon/rhino/releases/tag/v1.7.0)
 
 See _[How-to: Rhino 1.7 Migration Guide](https://appsilon.github.io/rhino/articles/how-to/migrate-1-7.html)_

R/box_linters.R

@@ -1,3 +1,130 @@
+# 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
 #'

inst/templates/app_structure/dot.lintr

@@ -1,6 +1,7 @@
 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(),

pkgdown/_pkgdown.yml

@@ -149,6 +149,7 @@ reference:
 
 - title: Linters
   contents:
+  - box_alphabetical_calls_linter
   - box_func_import_count_linter
   - box_separate_calls_linter
   - box_trailing_commas_linter

tests/testthat/test-linter_box_alphabetical.R

@@ -0,0 +1,136 @@
+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)
+})

vignettes/explanation/box-modules.Rmd

@@ -61,7 +61,7 @@ Both `say_hello()` and `say_bye()` can be exported from `app/logic/messages.R`.
 
 ```r
 box::use(
-  app/logic/messages[say_hello, say_bye],
+  app/logic/messages[say_bye, say_hello],
 )
 
 #' @export
@@ -79,7 +79,7 @@ Modules can also be imported across directories; use code from `app/logic` in `a
 ```r
 # app/view/greet_module.R
 box::use(
-  shiny[moduleServer, NS, renderText, div, textOutput, req],
+  shiny[div, moduleServer, NS, renderText, req, textOutput],
   shiny.semantic[textInput],
 )