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 trailing commas linter #535

Merged
merged 25 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
987c967
future dev branch
radbasa Jan 11, 2024
18ec419
box trailing commas linter code and tests
radbasa Jan 11, 2024
4968924
box trailing commas linter documentation
radbasa Jan 11, 2024
6b74cbf
update rhino app template
radbasa Jan 11, 2024
4968b33
package maintenance
radbasa Jan 11, 2024
daf8a8a
add an exception test
radbasa Jan 11, 2024
976d3ac
remove lint
radbasa Jan 11, 2024
8eafc92
reordered Suggests
radbasa Jan 11, 2024
321e6ee
update rhino app template unit test (#539)
radbasa Jan 12, 2024
2faa95f
Merge branch 'dev_next' into feature/box_trailing_commas_linter
radbasa Jan 12, 2024
06be5a3
add check_functions argument
radbasa Jan 12, 2024
370b84f
remove lint
radbasa Jan 12, 2024
516a98e
trailing commas lint in e2etest app files
radbasa Jan 12, 2024
5db286c
one version too far
radbasa Jan 26, 2024
a1b184e
Merge branch 'main' into dev_next
radbasa Jan 26, 2024
3197a07
Merge branch 'dev_next' into feature/box_trailing_commas_linter
radbasa Jan 26, 2024
2cd7761
address comments/suggestions in PR
radbasa Jan 26, 2024
fea7b83
Box universal import linter (#533)
radbasa Jan 26, 2024
13bb0b6
avoid merge conflicts by adding white space?
radbasa Jan 26, 2024
ed002da
revert rename to avoid merge conflict
radbasa Jan 26, 2024
5cbeded
Merge branch 'dev_next' into feature/box_trailing_commas_linter
radbasa Jan 26, 2024
748781a
integrate trailing commas linter into box linters file
radbasa Jan 26, 2024
4dde4ea
Merge branch 'main' into feature/box_trailing_commas_linter
radbasa Feb 6, 2024
ba2a90d
removed extra comma
radbasa Feb 6, 2024
7df7de9
chore: Minor documentation fixes.
jakubnowicki Feb 7, 2024
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.9001
Version: 1.6.0.9002
Authors@R:
c(
person("Kamil", "Żyła", role = c("aut", "cre"), email = "[email protected]"),
Expand Down
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
105 changes: 104 additions & 1 deletion R/box_linters.R
Original file line number Diff line number Diff line change
@@ -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.
#'
Expand Down
1 change: 1 addition & 0 deletions inst/templates/app_structure/dot.lintr
Original file line number Diff line number Diff line change
@@ -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()`.
)
48 changes: 48 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: 1 addition & 1 deletion 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 @@ -145,6 +145,7 @@ reference:

- title: Linters
contents:
- box_trailing_commas_linter
- box_universal_import_linter

- title: Data
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/app-files/hello.R
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/app-files/main.R
Original file line number Diff line number Diff line change
Expand Up @@ -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

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