Merge pull request #535 from Appsilon/feature/box_trailing_commas_linter

Box trailing commas linter

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: rhino
 Title: A Framework for Enterprise Shiny Applications
-Version: 1.6.0.9001
+Version: 1.6.0.9002
 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_trailing_commas_linter)
 export(box_universal_import_linter)
 export(build_js)
 export(build_sass)

NEWS.md

@@ -2,6 +2,7 @@
 
 1. Introduce linters for `box::use` statements:
     * `box_universal_import_linter` checks if all imports are explicit.
+    * `box_trailing_commas_linter` checks if statements include trailing commas.
 
 # [rhino 1.6.0](https://github.com/Appsilon/rhino/releases/tag/v1.6.0)
 

R/box_linters.R

@@ -1,4 +1,107 @@
-#' Box library universal import 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.
+#'
+#' @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
+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)
+  })
+}
+
+#' `box` library universal import linter
 #'
 #' Checks that all function imports are explicit. `package[...]` is not used.
 #'

inst/templates/app_structure/dot.lintr

@@ -1,6 +1,7 @@
 linters:
   linters_with_defaults(
     line_length_linter = line_length_linter(100),
+    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

@@ -145,6 +145,7 @@ reference:
 
 - title: Linters
   contents:
+  - box_trailing_commas_linter
   - box_universal_import_linter
 
 - title: Data

tests/e2e/app-files/hello.R

@@ -14,7 +14,7 @@ box::use(
   ],
 )
 
-box::use(app/logic/say_hello[say_hello])
+box::use(app/logic/say_hello[say_hello], )
 
 #' @export
 ui <- function(id) {

tests/e2e/app-files/main.R

@@ -3,7 +3,7 @@ box::use(
   shiny,
 )
 
-box::use(app/view/hello)
+box::use(app/view/hello, )
 
 Box <- react_component("Box") # nolint object_name_linter
 

tests/testthat/test-linter_box_trailing_commas_linter.R

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