Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Box import separate packages modules #542

Merged
merged 16 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -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"),
Expand Down
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)).
Expand Down
76 changes: 76 additions & 0 deletions R/box_linters.R
Original file line number Diff line number Diff line change
@@ -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.
#'
Expand Down Expand Up @@ -30,6 +33,7 @@
#' )
#'
#' @export
# nolint end
box_func_import_count_linter <- function(max = 8L) {
xpath <- glue::glue("//SYMBOL_PACKAGE[
(text() = 'box' and
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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[
(
Expand Down Expand Up @@ -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`
#'
Expand Down Expand Up @@ -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 `...`."

Expand Down
1 change: 1 addition & 0 deletions inst/templates/app_structure/dot.lintr
Original file line number Diff line number Diff line change
Expand Up @@ -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()`.
Expand Down
2 changes: 2 additions & 0 deletions man/box_func_import_count_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 36 additions & 0 deletions man/box_separate_calls_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions man/box_trailing_commas_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions man/box_universal_import_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkgdown/_pkgdown.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
87 changes: 87 additions & 0 deletions tests/testthat/test-linter_box_separate_calls_linter.R
Original file line number Diff line number Diff line change
@@ -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)
})
Loading