diff --git a/DESCRIPTION b/DESCRIPTION index 119a62de..b10b66c0 100644 --- a/DESCRIPTION +++ b/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 = "opensource+kamil@appsilon.com"), diff --git a/NAMESPACE b/NAMESPACE index 521311fc..2977622c 100644 --- a/NAMESPACE +++ b/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) diff --git a/NEWS.md b/NEWS.md index 80a526bb..ea08c98a 100644 --- a/NEWS.md +++ b/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) diff --git a/R/box_linters.R b/R/box_linters.R index f0ba769c..bc753b83 100644 --- a/R/box_linters.R +++ b/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. #' diff --git a/inst/templates/app_structure/dot.lintr b/inst/templates/app_structure/dot.lintr index 7dff4f80..e5c6ac24 100644 --- a/inst/templates/app_structure/dot.lintr +++ b/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()`. ) diff --git a/man/box_trailing_commas_linter.Rd b/man/box_trailing_commas_linter.Rd new file mode 100644 index 00000000..b2d75892 --- /dev/null +++ b/man/box_trailing_commas_linter.Rd @@ -0,0 +1,48 @@ +% 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. +} +\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 index f4a8057a..ce83b851 100644 --- a/man/box_universal_import_linter.Rd +++ b/man/box_universal_import_linter.Rd @@ -2,7 +2,7 @@ % Please edit documentation in R/box_linters.R \name{box_universal_import_linter} \alias{box_universal_import_linter} -\title{Box library universal import linter} +\title{\code{box} library universal import linter} \usage{ box_universal_import_linter() } diff --git a/pkgdown/_pkgdown.yml b/pkgdown/_pkgdown.yml index 6b5dc810..8b86c7e4 100644 --- a/pkgdown/_pkgdown.yml +++ b/pkgdown/_pkgdown.yml @@ -145,6 +145,7 @@ reference: - title: Linters contents: + - box_trailing_commas_linter - box_universal_import_linter - title: Data diff --git a/tests/e2e/app-files/hello.R b/tests/e2e/app-files/hello.R index 19023488..2e85bf30 100644 --- a/tests/e2e/app-files/hello.R +++ b/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) { diff --git a/tests/e2e/app-files/main.R b/tests/e2e/app-files/main.R index f18ba6ab..ce3bd35d 100644 --- a/tests/e2e/app-files/main.R +++ b/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 diff --git a/tests/testthat/test-linter_box_trailing_commas_linter.R b/tests/testthat/test-linter_box_trailing_commas_linter.R new file mode 100644 index 00000000..8aadc0ab --- /dev/null +++ b/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) +})