From 987c96700deb13c49bb8dc2f5439c54d1b7f46c0 Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Thu, 11 Jan 2024 11:10:17 +0800 Subject: [PATCH 01/20] future dev branch --- DESCRIPTION | 2 +- NEWS.md | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 6708609a..f399da4f 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: rhino Title: A Framework for Enterprise Shiny Applications -Version: 1.5.0.9003 +Version: 1.7.0.9000 Authors@R: c( person("Kamil", "Żyła", role = c("aut", "cre"), email = "opensource+kamil@appsilon.com"), diff --git a/NEWS.md b/NEWS.md index 4d26de0d..33cf765c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,6 @@ +# rhino (development-next) + + # rhino (development) 1. `pkg_install` supports installation from local sources, GitHub, and Bioconductor. From 18ec4190147c3234e1a3b56a8135067d251d42f3 Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Thu, 11 Jan 2024 14:40:53 +0800 Subject: [PATCH 02/20] box trailing commas linter code and tests --- R/linter_box_trailing_commas_linter.R | 93 +++++++++++++++++++ .../test-linter_box_trailing_commas_linter.R | 92 ++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 R/linter_box_trailing_commas_linter.R create mode 100644 tests/testthat/test-linter_box_trailing_commas_linter.R diff --git a/R/linter_box_trailing_commas_linter.R b/R/linter_box_trailing_commas_linter.R new file mode 100644 index 00000000..3c129187 --- /dev/null +++ b/R/linter_box_trailing_commas_linter.R @@ -0,0 +1,93 @@ +#' 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 function imports between +#' `[` and `]`. Take note that `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 +#' linter::lint( +#' text = "box::use(base, rlang,), +#' linters = box_trailing_commas_linter() +#' ) +#' +#' linter::lint( +#' text = "box::use( +#' dplyr[select, mutate,], +#' ), +#' linters = box_trailing_commas_linter() +#' ) +#' +#' @export +box_trailing_commas_linter <- function() { + 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) + bad_right_bracket_expr <- xml2::xml_find_all(xml, right_bracket_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 <- 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) + }) +} \ No newline at end of file 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..80ce0a33 --- /dev/null +++ b/tests/testthat/test-linter_box_trailing_commas_linter.R @@ -0,0 +1,92 @@ +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, )" + +bad_package_commas <- "box::use( + dplyr, + stringr +)" + +bad_package_commas_inline <- "box::use(dplyr, stringr)" + +bad_package_function_commas <- "box::use( + dplyr, + stringr[ + select, + mutate + ], +)" + +bad_package_function_commas_inline <- "box::use(stringr[select, mutate],)" + +bad_module_commas <- "box::use( + path/to/file1, + path/to/file2 +)" + +bad_module_commas_inline <- "box::use(path/to/file1, path/to/file2)" + +bad_module_function_commas <- "box::use( + path/to/file2[ + first_function, + second_function + ], +)" + +bad_module_function_commas_inline <- "box::use(path/to/file2[first_function, second_function], )" + +paren_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `)`.") +bracket_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `]`.") + +test_that("box_trailing_commas_linter skips allowed package import usage", { + linter <- box_trailing_commas_linter() + + 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() + + 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 blocks no trailing commas in package function imports", { + linter <- box_trailing_commas_linter() + + lintr::expect_lint(bad_package_function_commas, list(message = bracket_lint_msg), linter) + lintr::expect_lint(bad_package_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() + + 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 blocks no trailing commas in module function imports", { + linter <- box_trailing_commas_linter() + + lintr::expect_lint(bad_module_function_commas, list(message = bracket_lint_msg), linter) + lintr::expect_lint(bad_module_function_commas_inline, list(message = bracket_lint_msg), linter) +}) \ No newline at end of file From 496892484d17fd6709e7d74acadaebfab9f8051c Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Thu, 11 Jan 2024 14:42:03 +0800 Subject: [PATCH 03/20] box trailing commas linter documentation --- man/box_trailing_commas_linter.Rd | 13 +++++++++++++ pkgdown/_pkgdown.yml | 4 ++++ 2 files changed, 17 insertions(+) create mode 100644 man/box_trailing_commas_linter.Rd diff --git a/man/box_trailing_commas_linter.Rd b/man/box_trailing_commas_linter.Rd new file mode 100644 index 00000000..d028fe29 --- /dev/null +++ b/man/box_trailing_commas_linter.Rd @@ -0,0 +1,13 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/linter_box_trailing_commas_linter.R +\name{box_trailing_commas_linter} +\alias{box_trailing_commas_linter} +\title{Box library trailing commas linter} +\usage{ +box_trailing_commas_linter() +} +\description{ +Checks that all \code{box:use} imports have a trailing comma. This applies to +package or module imports between \code{(} and \verb{)}, and function imports between +\code{[} and \verb{]}. Take note that \code{lintr::commas_linter()} may come into play. +} diff --git a/pkgdown/_pkgdown.yml b/pkgdown/_pkgdown.yml index 759c93d4..ebee8ccb 100644 --- a/pkgdown/_pkgdown.yml +++ b/pkgdown/_pkgdown.yml @@ -143,6 +143,10 @@ reference: - diagnostics - test_e2e +- title: Linters + contents: + - box_trailing_commas_linter + - title: Data contents: - rhinos From 6b74cbf4d536bf303df058754ccb27920878c522 Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Thu, 11 Jan 2024 14:42:29 +0800 Subject: [PATCH 04/20] update rhino app template --- inst/templates/app_structure/dot.lintr | 1 + 1 file changed, 1 insertion(+) diff --git a/inst/templates/app_structure/dot.lintr b/inst/templates/app_structure/dot.lintr index 63fade9a..33b0a872 100644 --- a/inst/templates/app_structure/dot.lintr +++ b/inst/templates/app_structure/dot.lintr @@ -1,5 +1,6 @@ linters: linters_with_defaults( line_length_linter = line_length_linter(100), + box_trailing_commas_linter = rhino::box_trailing_commas_linter(), object_usage_linter = NULL # Does not work with `box::use()`. ) From 4968b3329a4c19c2addb5e549a851a80d9b6ac8b Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Thu, 11 Jan 2024 14:43:02 +0800 Subject: [PATCH 05/20] package maintenance --- DESCRIPTION | 6 ++++-- NAMESPACE | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index f399da4f..abd0f475 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: rhino Title: A Framework for Enterprise Shiny Applications -Version: 1.7.0.9000 +Version: 1.7.0.9002 Authors@R: c( person("Kamil", "Żyła", role = c("aut", "cre"), email = "opensource+kamil@appsilon.com"), @@ -46,7 +46,9 @@ Suggests: rcmdcheck, rmarkdown, shiny.react, - spelling + spelling, + xml2, + rex LazyData: true Config/testthat/edition: 3 Config/testthat/parallel: true diff --git a/NAMESPACE b/NAMESPACE index 2b25b2d5..66ec40b5 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(build_js) export(build_sass) export(diagnostics) From daf8a8a3100bde2e76ab1ff710be8890c13bc378 Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Thu, 11 Jan 2024 14:45:14 +0800 Subject: [PATCH 06/20] add an exception test --- .../testthat/test-linter_box_trailing_commas_linter.R | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-linter_box_trailing_commas_linter.R b/tests/testthat/test-linter_box_trailing_commas_linter.R index 80ce0a33..33ee9870 100644 --- a/tests/testthat/test-linter_box_trailing_commas_linter.R +++ b/tests/testthat/test-linter_box_trailing_commas_linter.R @@ -49,6 +49,8 @@ bad_module_function_commas <- "box::use( ], )" +should_not_lint <- "x <- c(1, 2, 3)" + bad_module_function_commas_inline <- "box::use(path/to/file2[first_function, second_function], )" paren_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `)`.") @@ -89,4 +91,10 @@ test_that("box_trailing_commas_linter blocks no trailing commas in module functi lintr::expect_lint(bad_module_function_commas, list(message = bracket_lint_msg), linter) lintr::expect_lint(bad_module_function_commas_inline, list(message = bracket_lint_msg), linter) -}) \ No newline at end of file +}) + +test_that("box_trailing_commas_linter should not lint outside of `box::use()`", { + linter <- box_trailing_commas_linter() + + lintr::expect_lint(should_not_lint, NULL, linter) +}) From 976d3ac80e3ba3b9d3657747df8583cb92b7a93c Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Thu, 11 Jan 2024 14:56:56 +0800 Subject: [PATCH 07/20] remove lint --- R/linter_box_trailing_commas_linter.R | 30 +++++++++---------- .../test-linter_box_trailing_commas_linter.R | 20 ++++++------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/R/linter_box_trailing_commas_linter.R b/R/linter_box_trailing_commas_linter.R index 3c129187..793aba7e 100644 --- a/R/linter_box_trailing_commas_linter.R +++ b/R/linter_box_trailing_commas_linter.R @@ -3,34 +3,34 @@ #' Checks that all `box:use` imports have a trailing comma. This applies to #' package or module imports between `(` and `)`, and function imports between #' `[` and `]`. Take note that `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 #' linter::lint( #' text = "box::use(base, rlang,), #' linters = box_trailing_commas_linter() #' ) -#' +#' #' linter::lint( #' text = "box::use( #' dplyr[select, mutate,], #' ), #' linters = box_trailing_commas_linter() #' ) -#' +#' #' @export box_trailing_commas_linter <- function() { base_xpath <- "//SYMBOL_PACKAGE[ @@ -41,7 +41,7 @@ box_trailing_commas_linter <- function() { ] /parent::expr " - + right_paren_xpath <- paste( base_xpath, "/following-sibling::OP-RIGHT-PAREN[ @@ -49,8 +49,8 @@ box_trailing_commas_linter <- function() { ] " ) - - right_bracket_xpath <-paste( + + right_bracket_xpath <- paste( base_xpath, "/parent::expr /descendant::OP-RIGHT-BRACKET[ @@ -63,31 +63,31 @@ box_trailing_commas_linter <- function() { ] " ) - + 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) bad_right_bracket_expr <- xml2::xml_find_all(xml, right_bracket_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 <- 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) }) -} \ No newline at end of file +} diff --git a/tests/testthat/test-linter_box_trailing_commas_linter.R b/tests/testthat/test-linter_box_trailing_commas_linter.R index 33ee9870..e8499523 100644 --- a/tests/testthat/test-linter_box_trailing_commas_linter.R +++ b/tests/testthat/test-linter_box_trailing_commas_linter.R @@ -33,7 +33,7 @@ bad_package_function_commas <- "box::use( ], )" -bad_package_function_commas_inline <- "box::use(stringr[select, mutate],)" +bad_pkg_function_commas_inline <- "box::use(stringr[select, mutate],)" bad_module_commas <- "box::use( path/to/file1, @@ -51,14 +51,14 @@ bad_module_function_commas <- "box::use( should_not_lint <- "x <- c(1, 2, 3)" -bad_module_function_commas_inline <- "box::use(path/to/file2[first_function, second_function], )" +bad_mod_function_commas_inline <- "box::use(path/to/file2[first_function, second_function], )" paren_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `)`.") bracket_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `]`.") test_that("box_trailing_commas_linter skips allowed package import usage", { linter <- box_trailing_commas_linter() - + 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) @@ -67,34 +67,34 @@ test_that("box_trailing_commas_linter skips allowed package import usage", { test_that("box_trailing_commas_linter blocks no trailing commas in package imports", { linter <- box_trailing_commas_linter() - + 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 blocks no trailing commas in package function imports", { linter <- box_trailing_commas_linter() - + lintr::expect_lint(bad_package_function_commas, list(message = bracket_lint_msg), linter) - lintr::expect_lint(bad_package_function_commas_inline, 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() - + 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 blocks no trailing commas in module function imports", { linter <- box_trailing_commas_linter() - + lintr::expect_lint(bad_module_function_commas, list(message = bracket_lint_msg), linter) - lintr::expect_lint(bad_module_function_commas_inline, 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() - + lintr::expect_lint(should_not_lint, NULL, linter) }) From 8eafc92baae64b2ec77678f26f4c9f4edfacbbcf Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Thu, 11 Jan 2024 17:13:23 +0800 Subject: [PATCH 08/20] reordered Suggests --- DESCRIPTION | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index abd0f475..103733eb 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -44,11 +44,11 @@ Suggests: knitr, mockery, rcmdcheck, + rex, rmarkdown, shiny.react, spelling, - xml2, - rex + xml2 LazyData: true Config/testthat/edition: 3 Config/testthat/parallel: true From 321e6eebbfa6baa1302db30a62c94d5f3fca3061 Mon Sep 17 00:00:00 2001 From: Ricardo Rodrigo Basa Date: Fri, 12 Jan 2024 12:35:04 +0800 Subject: [PATCH 09/20] update rhino app template unit test (#539) * update rhino app template unit test * update e2etest app files * remove what looks like stylr styling --- .../unit_tests/tests/testthat/test-main.R | 4 ++-- tests/e2e/app-files/hello.R | 18 +++++++++--------- tests/e2e/app-files/main.R | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/inst/templates/unit_tests/tests/testthat/test-main.R b/inst/templates/unit_tests/tests/testthat/test-main.R index f9d795b7..003ae7e8 100644 --- a/inst/templates/unit_tests/tests/testthat/test-main.R +++ b/inst/templates/unit_tests/tests/testthat/test-main.R @@ -1,9 +1,9 @@ box::use( shiny[testServer], - testthat[...], + testthat[expect_true, test_that], ) box::use( - app/main[...], + app/main[server, ui], ) test_that("main server works", { diff --git a/tests/e2e/app-files/hello.R b/tests/e2e/app-files/hello.R index 58bfbef1..19023488 100644 --- a/tests/e2e/app-files/hello.R +++ b/tests/e2e/app-files/hello.R @@ -1,20 +1,20 @@ box::use( shiny[ - bootstrapPage, - NS, - tags, - textInput, actionButton, - observeEvent, - textOutput, + bootstrapPage, + isolate, moduleServer, + NS, observe, + observeEvent, renderText, - isolate - ] + tags, + textInput, + textOutput + ], ) -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 c789ebc0..f18ba6ab 100644 --- a/tests/e2e/app-files/main.R +++ b/tests/e2e/app-files/main.R @@ -1,9 +1,9 @@ box::use( + rhino[log, react_component], shiny, - rhino[log, react_component] ) -box::use(app / view / hello) +box::use(app/view/hello) Box <- react_component("Box") # nolint object_name_linter From 06be5a3f45c5ded48e50f814d4d9f50e7c6de90c Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Fri, 12 Jan 2024 15:29:28 +0800 Subject: [PATCH 10/20] add check_functions argument --- R/linter_box_trailing_commas_linter.R | 31 ++++++++++++------- man/box_trailing_commas_linter.Rd | 8 +++-- tests/e2e/app-files/test-say_hello.R | 4 +-- .../test-linter_box_trailing_commas_linter.R | 20 ++++++------ 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/R/linter_box_trailing_commas_linter.R b/R/linter_box_trailing_commas_linter.R index 793aba7e..590f2823 100644 --- a/R/linter_box_trailing_commas_linter.R +++ b/R/linter_box_trailing_commas_linter.R @@ -1,9 +1,12 @@ #' 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 function imports between +#' 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. +#' #' @examples #' # will produce lints #' lintr::lint( @@ -13,26 +16,26 @@ #' #' lintr::lint( #' text = "box::use( -#' dplyr[select, mutate], +#' dplyr[select, mutate] #' ), #' linters = box_trailing_commas_linter() #' ) #' #' # okay #' linter::lint( -#' text = "box::use(base, rlang,), +#' text = "box::use(base, rlang, ), #' linters = box_trailing_commas_linter() #' ) #' #' linter::lint( #' text = "box::use( -#' dplyr[select, mutate,], +#' dplyr[select, mutate], #' ), #' linters = box_trailing_commas_linter() #' ) #' #' @export -box_trailing_commas_linter <- function() { +box_trailing_commas_linter <- function(check_functions = FALSE) { base_xpath <- "//SYMBOL_PACKAGE[ ( text() = 'box' and @@ -72,7 +75,6 @@ box_trailing_commas_linter <- function() { xml <- source_expression$full_xml_parsed_content bad_right_paren_expr <- xml2::xml_find_all(xml, right_paren_xpath) - bad_right_bracket_expr <- xml2::xml_find_all(xml, right_bracket_xpath) paren_lints <- lintr::xml_nodes_to_lints( bad_right_paren_expr, @@ -81,12 +83,17 @@ box_trailing_commas_linter <- function() { type = "style" ) - 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" - ) + 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) }) diff --git a/man/box_trailing_commas_linter.Rd b/man/box_trailing_commas_linter.Rd index d028fe29..979986d4 100644 --- a/man/box_trailing_commas_linter.Rd +++ b/man/box_trailing_commas_linter.Rd @@ -4,10 +4,14 @@ \alias{box_trailing_commas_linter} \title{Box library trailing commas linter} \usage{ -box_trailing_commas_linter() +box_trailing_commas_linter(check_functions = FALSE) +} +\arguments{ +\item{check_functions}{Boolean flag to include function imports between \code{[} and \verb{]}. +Defaults to FALSE.} } \description{ Checks that all \code{box:use} imports have a trailing comma. This applies to -package or module imports between \code{(} and \verb{)}, and function imports between +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. } diff --git a/tests/e2e/app-files/test-say_hello.R b/tests/e2e/app-files/test-say_hello.R index 84b393c6..52080749 100644 --- a/tests/e2e/app-files/test-say_hello.R +++ b/tests/e2e/app-files/test-say_hello.R @@ -1,6 +1,6 @@ -box::use(testthat[...]) +box::use(testthat[...], ) -box::use(app/logic/say_hello[say_hello]) +box::use(app/logic/say_hello[say_hello], ) describe("say_hello()", { it("should say hello with the correct name", { diff --git a/tests/testthat/test-linter_box_trailing_commas_linter.R b/tests/testthat/test-linter_box_trailing_commas_linter.R index e8499523..e1585a96 100644 --- a/tests/testthat/test-linter_box_trailing_commas_linter.R +++ b/tests/testthat/test-linter_box_trailing_commas_linter.R @@ -2,17 +2,17 @@ good_package_commas <- "box::use( dplyr, stringr[ select, - mutate, + mutate ], )" -good_package_commas_inline <- "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, + second_function ], )" @@ -49,10 +49,10 @@ bad_module_function_commas <- "box::use( ], )" -should_not_lint <- "x <- c(1, 2, 3)" - bad_mod_function_commas_inline <- "box::use(path/to/file2[first_function, second_function], )" +should_not_lint <- "x <- c(1, 2, 3)" + paren_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `)`.") bracket_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `]`.") @@ -72,8 +72,9 @@ test_that("box_trailing_commas_linter blocks no trailing commas in package impor lintr::expect_lint(bad_package_commas_inline, list(message = paren_lint_msg), linter) }) -test_that("box_trailing_commas_linter blocks no trailing commas in package function imports", { - linter <- box_trailing_commas_linter() +test_that("box_trailing_commas_linter check_functions = TRUE blocks no trailing commas + in package function imports", { + linter <- box_trailing_commas_linter(check_functions = TRUE) 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) @@ -86,8 +87,9 @@ test_that("box_trailing_comma_linter blocks no trailing commas in module imports lintr::expect_lint(bad_module_commas_inline, list(message = paren_lint_msg), linter) }) -test_that("box_trailing_commas_linter blocks no trailing commas in module function imports", { - linter <- box_trailing_commas_linter() +test_that("box_trailing_commas_linter check_functions = TRUE blocks no trailing commas + in module function imports", { + linter <- box_trailing_commas_linter(check_functions = TRUE) 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) From 370b84f5d5a8653f3e06161b6729219338cbaf20 Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Fri, 12 Jan 2024 15:36:10 +0800 Subject: [PATCH 11/20] remove lint --- tests/testthat/test-linter_box_trailing_commas_linter.R | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test-linter_box_trailing_commas_linter.R b/tests/testthat/test-linter_box_trailing_commas_linter.R index e1585a96..17b6b880 100644 --- a/tests/testthat/test-linter_box_trailing_commas_linter.R +++ b/tests/testthat/test-linter_box_trailing_commas_linter.R @@ -72,8 +72,7 @@ test_that("box_trailing_commas_linter blocks no trailing commas in package impor 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 - in package function imports", { +test_that("box_trailing_commas_linter check_functions = TRUE blocks no trailing commas", { linter <- box_trailing_commas_linter(check_functions = TRUE) lintr::expect_lint(bad_package_function_commas, list(message = bracket_lint_msg), linter) @@ -87,8 +86,7 @@ test_that("box_trailing_comma_linter blocks no trailing commas in module imports 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 - in module function imports", { +test_that("box_trailing_commas_linter check_functions = TRUE blocks no trailing commas", { linter <- box_trailing_commas_linter(check_functions = TRUE) lintr::expect_lint(bad_module_function_commas, list(message = bracket_lint_msg), linter) From 516a98e9d43fac84c5b965e0c78fa57208a21790 Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Fri, 12 Jan 2024 15:41:31 +0800 Subject: [PATCH 12/20] trailing commas lint in e2etest app files --- tests/e2e/app-files/hello.R | 2 +- tests/e2e/app-files/main.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 From 5db286cbb5dd35fae36a73a9c6e00d1d621d82c9 Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Fri, 26 Jan 2024 12:04:33 +0800 Subject: [PATCH 13/20] one version too far --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index f399da4f..202cdcf7 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: rhino Title: A Framework for Enterprise Shiny Applications -Version: 1.7.0.9000 +Version: 1.6.0.9000 Authors@R: c( person("Kamil", "Żyła", role = c("aut", "cre"), email = "opensource+kamil@appsilon.com"), From 2cd7761335deeeefd7a9ada7ba30e86884f857ec Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Fri, 26 Jan 2024 12:21:36 +0800 Subject: [PATCH 14/20] address comments/suggestions in PR --- ...trailing_commas_linter.R => box_linters.R} | 2 + man/box_trailing_commas_linter.Rd | 5 +- .../test-linter_box_trailing_commas_linter.R | 117 +++++++++--------- 3 files changed, 67 insertions(+), 57 deletions(-) rename R/{linter_box_trailing_commas_linter.R => box_linters.R} (97%) diff --git a/R/linter_box_trailing_commas_linter.R b/R/box_linters.R similarity index 97% rename from R/linter_box_trailing_commas_linter.R rename to R/box_linters.R index 590f2823..de7c842f 100644 --- a/R/linter_box_trailing_commas_linter.R +++ b/R/box_linters.R @@ -7,6 +7,8 @@ #' @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( diff --git a/man/box_trailing_commas_linter.Rd b/man/box_trailing_commas_linter.Rd index 979986d4..554c5fdd 100644 --- a/man/box_trailing_commas_linter.Rd +++ b/man/box_trailing_commas_linter.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/linter_box_trailing_commas_linter.R +% Please edit documentation in R/box_linters.R \name{box_trailing_commas_linter} \alias{box_trailing_commas_linter} \title{Box library trailing commas linter} @@ -10,6 +10,9 @@ box_trailing_commas_linter(check_functions = FALSE) \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 diff --git a/tests/testthat/test-linter_box_trailing_commas_linter.R b/tests/testthat/test-linter_box_trailing_commas_linter.R index 17b6b880..8aadc0ab 100644 --- a/tests/testthat/test-linter_box_trailing_commas_linter.R +++ b/tests/testthat/test-linter_box_trailing_commas_linter.R @@ -1,63 +1,25 @@ -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, )" - -bad_package_commas <- "box::use( - dplyr, - stringr -)" - -bad_package_commas_inline <- "box::use(dplyr, stringr)" - -bad_package_function_commas <- "box::use( - dplyr, - stringr[ - select, - mutate - ], -)" - -bad_pkg_function_commas_inline <- "box::use(stringr[select, mutate],)" - -bad_module_commas <- "box::use( - path/to/file1, - path/to/file2 -)" - -bad_module_commas_inline <- "box::use(path/to/file1, path/to/file2)" - -bad_module_function_commas <- "box::use( - path/to/file2[ - first_function, - second_function - ], -)" +test_that("box_trailing_commas_linter skips allowed package import usage", { + linter <- box_trailing_commas_linter() -bad_mod_function_commas_inline <- "box::use(path/to/file2[first_function, second_function], )" + good_package_commas <- "box::use( + dplyr, + stringr[ + select, + mutate + ], + )" -should_not_lint <- "x <- c(1, 2, 3)" + good_package_commas_inline <- "box::use(dplyr, stringr[select, mutate], )" -paren_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `)`.") -bracket_lint_msg <- rex::rex("Always have a trailing comma at the end of imports, before a `]`.") + good_module_commas <- "box::use( + path/to/file1, + path/to/file2[ + first_function, + second_function + ], + )" -test_that("box_trailing_commas_linter skips allowed package import usage", { - linter <- box_trailing_commas_linter() + 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) @@ -68,6 +30,15 @@ test_that("box_trailing_commas_linter skips allowed package import usage", { 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) }) @@ -75,6 +46,18 @@ test_that("box_trailing_commas_linter blocks no trailing commas in package impor 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) }) @@ -82,6 +65,15 @@ test_that("box_trailing_commas_linter check_functions = TRUE blocks no trailing 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) }) @@ -89,6 +81,17 @@ test_that("box_trailing_comma_linter blocks no trailing commas in module imports 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) }) @@ -96,5 +99,7 @@ test_that("box_trailing_commas_linter check_functions = TRUE blocks no trailing 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) }) From fea7b836cad52f30756cc35d33b82782fbe35a2f Mon Sep 17 00:00:00 2001 From: Ricardo Rodrigo Basa Date: Fri, 26 Jan 2024 12:26:00 +0800 Subject: [PATCH 15/20] Box universal import linter (#533) * box universal import linter * add box_universal_import_linter to .lintr template * documentation * package housekeeping * removed some lint * namespace box_universal_import_linter * reordered Suggests * update e2etest app files to rhino-box styleguide * address comments/suggestions in PR * fixed version number issue. moved xml2 to imports --- DESCRIPTION | 4 +- NAMESPACE | 1 + R/box_linters.R | 57 +++++++++++++++ inst/templates/app_structure/dot.lintr | 1 + man/box_universal_import_linter.Rd | 38 ++++++++++ pkgdown/_pkgdown.yml | 4 + tests/e2e/app-files/test-hello.R | 4 +- tests/e2e/app-files/test-say_hello.R | 4 +- .../test-linter_box_universal_import_linter.R | 73 +++++++++++++++++++ 9 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 R/box_linters.R create mode 100644 man/box_universal_import_linter.Rd create mode 100644 tests/testthat/test-linter_box_universal_import_linter.R diff --git a/DESCRIPTION b/DESCRIPTION index 202cdcf7..94e59bb0 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: rhino Title: A Framework for Enterprise Shiny Applications -Version: 1.6.0.9000 +Version: 1.6.0.9001 Authors@R: c( person("Kamil", "Żyła", role = c("aut", "cre"), email = "opensource+kamil@appsilon.com"), @@ -38,12 +38,14 @@ Imports: testthat (>= 3.0.0), utils, withr, + xml2, yaml Suggests: covr, knitr, mockery, rcmdcheck, + rex, rmarkdown, shiny.react, spelling diff --git a/NAMESPACE b/NAMESPACE index 2b25b2d5..521311fc 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,6 +1,7 @@ # Generated by roxygen2: do not edit by hand export(app) +export(box_universal_import_linter) export(build_js) export(build_sass) export(diagnostics) diff --git a/R/box_linters.R b/R/box_linters.R new file mode 100644 index 00000000..f0ba769c --- /dev/null +++ b/R/box_linters.R @@ -0,0 +1,57 @@ +#' Box library universal import linter +#' +#' Checks that all function imports are explicit. `package[...]` is not used. +#' +#' @return A custom linter function for use with `r-lib/lintr` +#' +#' @examples +#' # will produce lints +#' lintr::lint( +#' text = "box::use(base[...])", +#' linters = box_universal_import_linter() +#' ) +#' +#' lintr::lint( +#' text = "box::use(path/to/file[...])", +#' linters = box_universal_import_linter() +#' ) +#' +#' # okay +#' lintr::lint( +#' text = "box::use(base[print])", +#' linters = box_universal_import_linter() +#' ) +#' +#' lintr::lint( +#' text = "box::use(path/to/file[do_something])", +#' linters = box_universal_import_linter() +#' ) +#' +#' @export +box_universal_import_linter <- function() { + lint_message <- "Explicitly declare imports rather than universally import with `...`." + + xpath <- " + //SYMBOL_PACKAGE[(text() = 'box' and following-sibling::SYMBOL_FUNCTION_CALL[text() = 'use'])] + /parent::expr + /parent::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_expr <- xml2::xml_find_all(xml, xpath) + + lintr::xml_nodes_to_lints( + bad_expr, + source_expression = source_expression, + lint_message = lint_message, + type = "style" + ) + }) +} diff --git a/inst/templates/app_structure/dot.lintr b/inst/templates/app_structure/dot.lintr index 63fade9a..7dff4f80 100644 --- a/inst/templates/app_structure/dot.lintr +++ b/inst/templates/app_structure/dot.lintr @@ -1,5 +1,6 @@ linters: linters_with_defaults( line_length_linter = line_length_linter(100), + box_universal_import_linter = rhino::box_universal_import_linter(), object_usage_linter = NULL # Does not work with `box::use()`. ) diff --git a/man/box_universal_import_linter.Rd b/man/box_universal_import_linter.Rd new file mode 100644 index 00000000..f4a8057a --- /dev/null +++ b/man/box_universal_import_linter.Rd @@ -0,0 +1,38 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/box_linters.R +\name{box_universal_import_linter} +\alias{box_universal_import_linter} +\title{Box library universal import linter} +\usage{ +box_universal_import_linter() +} +\value{ +A custom linter function for use with \code{r-lib/lintr} +} +\description{ +Checks that all function imports are explicit. \code{package[...]} is not used. +} +\examples{ +# will produce lints +lintr::lint( + text = "box::use(base[...])", + linters = box_universal_import_linter() +) + +lintr::lint( + text = "box::use(path/to/file[...])", + linters = box_universal_import_linter() +) + +# okay +lintr::lint( + text = "box::use(base[print])", + linters = box_universal_import_linter() +) + +lintr::lint( + text = "box::use(path/to/file[do_something])", + linters = box_universal_import_linter() +) + +} diff --git a/pkgdown/_pkgdown.yml b/pkgdown/_pkgdown.yml index 759c93d4..6b5dc810 100644 --- a/pkgdown/_pkgdown.yml +++ b/pkgdown/_pkgdown.yml @@ -143,6 +143,10 @@ reference: - diagnostics - test_e2e +- title: Linters + contents: + - box_universal_import_linter + - title: Data contents: - rhinos diff --git a/tests/e2e/app-files/test-hello.R b/tests/e2e/app-files/test-hello.R index 54301473..a8d0a96a 100644 --- a/tests/e2e/app-files/test-hello.R +++ b/tests/e2e/app-files/test-hello.R @@ -1,9 +1,9 @@ box::use( shiny[testServer], - testthat[...], + testthat[describe, expect_identical, it], ) box::use( - app/view/hello[...], + app/view/hello[server], ) describe("hello$server()", { diff --git a/tests/e2e/app-files/test-say_hello.R b/tests/e2e/app-files/test-say_hello.R index 84b393c6..9ad2d999 100644 --- a/tests/e2e/app-files/test-say_hello.R +++ b/tests/e2e/app-files/test-say_hello.R @@ -1,6 +1,6 @@ -box::use(testthat[...]) +box::use(testthat[describe, expect_identical, it], ) -box::use(app/logic/say_hello[say_hello]) +box::use(app/logic/say_hello[say_hello], ) describe("say_hello()", { it("should say hello with the correct name", { diff --git a/tests/testthat/test-linter_box_universal_import_linter.R b/tests/testthat/test-linter_box_universal_import_linter.R new file mode 100644 index 00000000..1f4e39bb --- /dev/null +++ b/tests/testthat/test-linter_box_universal_import_linter.R @@ -0,0 +1,73 @@ +test_that("box_universal_count_linter skips allowed package import usage", { + linter <- box_universal_import_linter() + + good_package_imports <- "box::use( + dplyr[select, mutate, ], + stringr[str_sub, str_match, ], + ) + " + + lintr::expect_lint(good_package_imports, NULL, linter) +}) + +test_that("box_universal_count_linter skips allowed module import usage", { + linter <- box_universal_import_linter() + + good_module_imports <- "box::use( + path/to/file1[do_something, do_another, ], + path/to/file2[find_x, find_y, ], + ) + " + + lintr::expect_lint(good_module_imports, NULL, linter) +}) + +test_that("box_universal_count_linter blocks disallowed package import usage", { + linter <- box_universal_import_linter() + + bad_package_imports <- "box::use( + dplyr[...], + stringr[str_sub, str_match, ], + ) + " + + lint_msg <- rex::rex("Explicitly declare imports rather than universally import with `...`.") + + lintr::expect_lint(bad_package_imports, list(message = lint_msg), linter) +}) + +test_that("box_universal_count_linter blocks disallowed module import usage", { + linter <- box_universal_import_linter() + + bad_module_imports <- "box::use( + path/to/file1[...], + path/to/file2[find_x, find_y, ], + ) + " + + lint_msg <- rex::rex("Explicitly declare imports rather than universally import with `...`.") + + lintr::expect_lint(bad_module_imports, list(message = lint_msg), linter) +}) + +test_that("box_universal_count_linter skips three dots in function declarations and calls", { + linter <- box_universal_import_linter() + + function_with_three_dots <- "some_function <- function(...) { + sum(...) + } + " + + lintr::expect_lint(function_with_three_dots, NULL, linter) +}) + +test_that("box_universal_count_linter respects #nolint", { + linter <- box_universal_import_linter() + + no_lint <- "box::use( + shiny[...], # nolint + ) + " + + lintr::expect_lint(no_lint, NULL, linter) +}) From 13bb0b6105221726748fce9a2189ed560685353d Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Fri, 26 Jan 2024 12:32:01 +0800 Subject: [PATCH 16/20] avoid merge conflicts by adding white space? --- R/box_linters.R | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/R/box_linters.R b/R/box_linters.R index de7c842f..cc414f4a 100644 --- a/R/box_linters.R +++ b/R/box_linters.R @@ -1,3 +1,8 @@ + + + + + #' Box library trailing commas linter #' #' Checks that all `box:use` imports have a trailing comma. This applies to From ed002da11b5f3df206966cce8c3f2be60fbaaf96 Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Fri, 26 Jan 2024 12:33:21 +0800 Subject: [PATCH 17/20] revert rename to avoid merge conflict --- R/{box_linters.R => linter_box_trailing_commas.R} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename R/{box_linters.R => linter_box_trailing_commas.R} (100%) diff --git a/R/box_linters.R b/R/linter_box_trailing_commas.R similarity index 100% rename from R/box_linters.R rename to R/linter_box_trailing_commas.R From 748781a5ddbda5bc9ac6258ff33d56e5a339809c Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Fri, 26 Jan 2024 12:42:23 +0800 Subject: [PATCH 18/20] integrate trailing commas linter into box linters file --- R/box_linters.R | 103 ++++++++++++++++++++++++++++++ R/linter_box_trailing_commas.R | 102 ----------------------------- man/box_trailing_commas_linter.Rd | 2 +- 3 files changed, 104 insertions(+), 103 deletions(-) diff --git a/R/box_linters.R b/R/box_linters.R index f0ba769c..9b090ea6 100644 --- a/R/box_linters.R +++ b/R/box_linters.R @@ -1,3 +1,106 @@ +#' 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 +#' linter::lint( +#' text = "box::use(base, rlang, ), +#' linters = box_trailing_commas_linter() +#' ) +#' +#' linter::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/R/linter_box_trailing_commas.R b/R/linter_box_trailing_commas.R index de7c842f..e69de29b 100644 --- a/R/linter_box_trailing_commas.R +++ b/R/linter_box_trailing_commas.R @@ -1,102 +0,0 @@ -#' 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 -#' linter::lint( -#' text = "box::use(base, rlang, ), -#' linters = box_trailing_commas_linter() -#' ) -#' -#' linter::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) - }) -} diff --git a/man/box_trailing_commas_linter.Rd b/man/box_trailing_commas_linter.Rd index 540427a5..554c5fdd 100644 --- a/man/box_trailing_commas_linter.Rd +++ b/man/box_trailing_commas_linter.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/linter_box_trailing_commas.R +% Please edit documentation in R/box_linters.R \name{box_trailing_commas_linter} \alias{box_trailing_commas_linter} \title{Box library trailing commas linter} From ba2a90d02200488fe27271debea9245072a6c8c6 Mon Sep 17 00:00:00 2001 From: Rodrigo Basa Date: Tue, 6 Feb 2024 17:11:02 +0800 Subject: [PATCH 19/20] removed extra comma --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index a6108329..b10b66c0 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -49,7 +49,7 @@ Suggests: rex, rmarkdown, shiny.react, - spelling, + spelling LazyData: true Config/testthat/edition: 3 Config/testthat/parallel: true From 7df7de9ac041d3a07650824a3d2e384e9db136db Mon Sep 17 00:00:00 2001 From: Jakub Nowicki Date: Wed, 7 Feb 2024 16:13:08 +0100 Subject: [PATCH 20/20] chore: Minor documentation fixes. --- NEWS.md | 1 + R/box_linters.R | 14 +++++++------- R/linter_box_trailing_commas.R | 0 man/box_trailing_commas_linter.Rd | 30 +++++++++++++++++++++++++++++- man/box_universal_import_linter.Rd | 2 +- 5 files changed, 38 insertions(+), 9 deletions(-) delete mode 100644 R/linter_box_trailing_commas.R 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 9b090ea6..bc753b83 100644 --- a/R/box_linters.R +++ b/R/box_linters.R @@ -1,4 +1,4 @@ -#' Box library trailing commas 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 @@ -19,20 +19,20 @@ #' lintr::lint( #' text = "box::use( #' dplyr[select, mutate] -#' ), +#' )", #' linters = box_trailing_commas_linter() #' ) #' #' # okay -#' linter::lint( -#' text = "box::use(base, rlang, ), +#' lintr::lint( +#' text = "box::use(base, rlang, )", #' linters = box_trailing_commas_linter() #' ) #' -#' linter::lint( +#' lintr::lint( #' text = "box::use( #' dplyr[select, mutate], -#' ), +#' )", #' linters = box_trailing_commas_linter() #' ) #' @@ -101,7 +101,7 @@ box_trailing_commas_linter <- function(check_functions = FALSE) { }) } -#' Box library universal import linter +#' `box` library universal import linter #' #' Checks that all function imports are explicit. `package[...]` is not used. #' diff --git a/R/linter_box_trailing_commas.R b/R/linter_box_trailing_commas.R deleted file mode 100644 index e69de29b..00000000 diff --git a/man/box_trailing_commas_linter.Rd b/man/box_trailing_commas_linter.Rd index 554c5fdd..b2d75892 100644 --- a/man/box_trailing_commas_linter.Rd +++ b/man/box_trailing_commas_linter.Rd @@ -2,7 +2,7 @@ % Please edit documentation in R/box_linters.R \name{box_trailing_commas_linter} \alias{box_trailing_commas_linter} -\title{Box library trailing commas linter} +\title{\code{box} library trailing commas linter} \usage{ box_trailing_commas_linter(check_functions = FALSE) } @@ -18,3 +18,31 @@ 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() }