From 8f745ca7b62819dfc32b6413e7d5442ffa646557 Mon Sep 17 00:00:00 2001
From: Rodrigo Basa <rodrigo@appsilon.com>
Date: Thu, 11 Jan 2024 12:03:46 +0800
Subject: [PATCH 01/10] box universal import linter

---
 R/linter_box_universal_import_linter.R        | 55 ++++++++++++++
 .../test-linter_box_universal_import_linter.R | 71 +++++++++++++++++++
 2 files changed, 126 insertions(+)
 create mode 100644 R/linter_box_universal_import_linter.R
 create mode 100644 tests/testthat/test-linter_box_universal_import_linter.R

diff --git a/R/linter_box_universal_import_linter.R b/R/linter_box_universal_import_linter.R
new file mode 100644
index 00000000..a6982836
--- /dev/null
+++ b/R/linter_box_universal_import_linter.R
@@ -0,0 +1,55 @@
+#' Box library universal import linter
+#'
+#' Checks that all function imports are explicit. `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()
+#' )
+#' 
+#' @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/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..aa1e08a3
--- /dev/null
+++ b/tests/testthat/test-linter_box_universal_import_linter.R
@@ -0,0 +1,71 @@
+good_package_imports <- "box::use(
+    dplyr[select, mutate, ],
+    stringr[str_sub, str_match, ],
+  )
+"
+
+good_module_imports <- "box::use(
+    path/to/file1[do_something, do_another, ],
+    path/to/file2[find_x, find_y, ],
+  )
+"
+
+bad_package_imports <- "box::use(
+    dplyr[...],
+    stringr[str_sub, str_match, ],
+  )
+"
+
+bad_module_imports <- "box::use(
+    path/to/file1[...],
+    path/to/file2[find_x, find_y, ],
+  )
+"
+
+function_with_three_dots <- "some_function <- function(...) {
+    sum(...)
+  }
+"
+
+no_lint <- "box::use(
+    shiny[...],         # nolint
+  )
+"
+
+lint_msg <- rex::rex("Explicitly declare imports rather than universally import with `...`.")
+
+test_that("box_universal_count_linter skips allowed package import usage", {
+  linter <- box_universal_import_linter()
+  
+  lintr::expect_lint(good_package_imports, NULL, linter)
+})
+
+test_that("box_universal_count_linter skips allowed module import usage", {
+  linter <- box_universal_import_linter()
+  
+  lintr::expect_lint(good_module_imports, NULL, linter)
+})
+
+test_that("box_universal_count_linter blocks disallowed package import usage", {
+  linter <- box_universal_import_linter()
+  
+  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()
+
+  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()
+  
+  lintr::expect_lint(function_with_three_dots, NULL, linter)
+})
+
+test_that("box_universal_count_linter respects #nolint", {
+  linter <- box_universal_import_linter()
+  
+  lintr::expect_lint(no_lint, NULL, linter)
+})

From 1030a7cd6174eedecd03e259e8f310b404dc917c Mon Sep 17 00:00:00 2001
From: Rodrigo Basa <rodrigo@appsilon.com>
Date: Thu, 11 Jan 2024 12:05:58 +0800
Subject: [PATCH 02/10] add box_universal_import_linter to .lintr 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..a89a1290 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 = box_universal_import_linter(),
     object_usage_linter = NULL  # Does not work with `box::use()`.
   )

From cde39490c7643017e092a5b15bef63a6f958edc0 Mon Sep 17 00:00:00 2001
From: Rodrigo Basa <rodrigo@appsilon.com>
Date: Thu, 11 Jan 2024 12:06:26 +0800
Subject: [PATCH 03/10] documentation

---
 man/box_universal_import_linter.Rd | 35 ++++++++++++++++++++++++++++++
 pkgdown/_pkgdown.yml               |  4 ++++
 2 files changed, 39 insertions(+)
 create mode 100644 man/box_universal_import_linter.Rd

diff --git a/man/box_universal_import_linter.Rd b/man/box_universal_import_linter.Rd
new file mode 100644
index 00000000..09291c5d
--- /dev/null
+++ b/man/box_universal_import_linter.Rd
@@ -0,0 +1,35 @@
+% Generated by roxygen2: do not edit by hand
+% Please edit documentation in R/linter_box_universal_import_linter.R
+\name{box_universal_import_linter}
+\alias{box_universal_import_linter}
+\title{Box library universal import linter}
+\usage{
+box_universal_import_linter()
+}
+\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

From 44c458bce1653db5c088a178896323a4f6b467ee Mon Sep 17 00:00:00 2001
From: Rodrigo Basa <rodrigo@appsilon.com>
Date: Thu, 11 Jan 2024 12:06:39 +0800
Subject: [PATCH 04/10] package housekeeping

---
 DESCRIPTION | 6 ++++--
 NAMESPACE   | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index f399da4f..ca9d1d10 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.9001
 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..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)

From 5b358868b5fea3d728d83b9f7e8633987f01d168 Mon Sep 17 00:00:00 2001
From: Rodrigo Basa <rodrigo@appsilon.com>
Date: Thu, 11 Jan 2024 12:19:55 +0800
Subject: [PATCH 05/10] removed some lint

---
 R/linter_box_universal_import_linter.R                 | 10 +++++-----
 .../testthat/test-linter_box_universal_import_linter.R | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/R/linter_box_universal_import_linter.R b/R/linter_box_universal_import_linter.R
index a6982836..e0c04555 100644
--- a/R/linter_box_universal_import_linter.R
+++ b/R/linter_box_universal_import_linter.R
@@ -8,23 +8,23 @@
 #'   text = "box::use(base[...])",
 #'   linters = box_universal_import_linter()
 #' )
-#' 
+#'
 #' lintr::lint(
 #'   text = "box::use(path/to/file[...])",
 #'   linters = box_universal_import_linter()
 #' )
-#' 
-#' # okay 
+#'
+#' # 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 `...`."
diff --git a/tests/testthat/test-linter_box_universal_import_linter.R b/tests/testthat/test-linter_box_universal_import_linter.R
index aa1e08a3..06b30f4d 100644
--- a/tests/testthat/test-linter_box_universal_import_linter.R
+++ b/tests/testthat/test-linter_box_universal_import_linter.R
@@ -36,19 +36,19 @@ lint_msg <- rex::rex("Explicitly declare imports rather than universally import
 
 test_that("box_universal_count_linter skips allowed package import usage", {
   linter <- box_universal_import_linter()
-  
+
   lintr::expect_lint(good_package_imports, NULL, linter)
 })
 
 test_that("box_universal_count_linter skips allowed module import usage", {
   linter <- box_universal_import_linter()
-  
+
   lintr::expect_lint(good_module_imports, NULL, linter)
 })
 
 test_that("box_universal_count_linter blocks disallowed package import usage", {
   linter <- box_universal_import_linter()
-  
+
   lintr::expect_lint(bad_package_imports, list(message = lint_msg), linter)
 })
 
@@ -60,12 +60,12 @@ test_that("box_universal_count_linter blocks disallowed module import usage", {
 
 test_that("box_universal_count_linter skips three dots in function declarations and calls", {
   linter <- box_universal_import_linter()
-  
+
   lintr::expect_lint(function_with_three_dots, NULL, linter)
 })
 
 test_that("box_universal_count_linter respects #nolint", {
   linter <- box_universal_import_linter()
-  
+
   lintr::expect_lint(no_lint, NULL, linter)
 })

From da72729c5c4855ab3ce688693e996f88e9e9114d Mon Sep 17 00:00:00 2001
From: Rodrigo Basa <rodrigo@appsilon.com>
Date: Thu, 11 Jan 2024 12:25:25 +0800
Subject: [PATCH 06/10] namespace box_universal_import_linter

---
 inst/templates/app_structure/dot.lintr | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/inst/templates/app_structure/dot.lintr b/inst/templates/app_structure/dot.lintr
index a89a1290..7dff4f80 100644
--- a/inst/templates/app_structure/dot.lintr
+++ b/inst/templates/app_structure/dot.lintr
@@ -1,6 +1,6 @@
 linters:
   linters_with_defaults(
     line_length_linter = line_length_linter(100),
-    box_universal_import_linter = box_universal_import_linter(),
+    box_universal_import_linter = rhino::box_universal_import_linter(),
     object_usage_linter = NULL  # Does not work with `box::use()`.
   )

From 628fae554834e609d5a158809224572c30445cb7 Mon Sep 17 00:00:00 2001
From: Rodrigo Basa <rodrigo@appsilon.com>
Date: Thu, 11 Jan 2024 17:14:06 +0800
Subject: [PATCH 07/10] reordered Suggests

---
 DESCRIPTION | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index ca9d1d10..9aa99a30 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 24e0fe8c558efeb07f61f001f5bb75fef559b94f Mon Sep 17 00:00:00 2001
From: Rodrigo Basa <rodrigo@appsilon.com>
Date: Fri, 12 Jan 2024 16:00:02 +0800
Subject: [PATCH 08/10] update e2etest app files to rhino-box styleguide

---
 tests/e2e/app-files/test-hello.R     | 4 ++--
 tests/e2e/app-files/test-say_hello.R | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

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", {

From 2900a8ea63fb6c2a84dcfdd767c3969c15ba514e Mon Sep 17 00:00:00 2001
From: Rodrigo Basa <rodrigo@appsilon.com>
Date: Fri, 26 Jan 2024 12:03:35 +0800
Subject: [PATCH 09/10] address comments/suggestions in PR

---
 ...niversal_import_linter.R => box_linters.R} |  2 +
 man/box_universal_import_linter.Rd            |  7 +-
 .../test-linter_box_universal_import_linter.R | 74 ++++++++++---------
 3 files changed, 45 insertions(+), 38 deletions(-)
 rename R/{linter_box_universal_import_linter.R => box_linters.R} (95%)

diff --git a/R/linter_box_universal_import_linter.R b/R/box_linters.R
similarity index 95%
rename from R/linter_box_universal_import_linter.R
rename to R/box_linters.R
index e0c04555..f0ba769c 100644
--- a/R/linter_box_universal_import_linter.R
+++ b/R/box_linters.R
@@ -2,6 +2,8 @@
 #'
 #' 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(
diff --git a/man/box_universal_import_linter.Rd b/man/box_universal_import_linter.Rd
index 09291c5d..f4a8057a 100644
--- a/man/box_universal_import_linter.Rd
+++ b/man/box_universal_import_linter.Rd
@@ -1,11 +1,14 @@
 % Generated by roxygen2: do not edit by hand
-% Please edit documentation in R/linter_box_universal_import_linter.R
+% 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.
 }
@@ -21,7 +24,7 @@ lintr::lint(
   linters = box_universal_import_linter()
 )
 
-# okay 
+# okay
 lintr::lint(
   text = "box::use(base[print])",
   linters = box_universal_import_linter()
diff --git a/tests/testthat/test-linter_box_universal_import_linter.R b/tests/testthat/test-linter_box_universal_import_linter.R
index 06b30f4d..1f4e39bb 100644
--- a/tests/testthat/test-linter_box_universal_import_linter.R
+++ b/tests/testthat/test-linter_box_universal_import_linter.R
@@ -1,71 +1,73 @@
-good_package_imports <- "box::use(
-    dplyr[select, mutate, ],
-    stringr[str_sub, str_match, ],
-  )
-"
-
-good_module_imports <- "box::use(
-    path/to/file1[do_something, do_another, ],
-    path/to/file2[find_x, find_y, ],
-  )
-"
-
-bad_package_imports <- "box::use(
-    dplyr[...],
-    stringr[str_sub, str_match, ],
-  )
-"
-
-bad_module_imports <- "box::use(
-    path/to/file1[...],
-    path/to/file2[find_x, find_y, ],
-  )
-"
-
-function_with_three_dots <- "some_function <- function(...) {
-    sum(...)
-  }
-"
-
-no_lint <- "box::use(
-    shiny[...],         # nolint
-  )
-"
-
-lint_msg <- rex::rex("Explicitly declare imports rather than universally import with `...`.")
-
 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 63a980b0d58bea685ff86403a84f86145f5b72a0 Mon Sep 17 00:00:00 2001
From: Rodrigo Basa <rodrigo@appsilon.com>
Date: Fri, 26 Jan 2024 12:05:19 +0800
Subject: [PATCH 10/10] fixed version number issue. moved xml2 to imports

---
 DESCRIPTION | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index 9aa99a30..94e59bb0 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -1,6 +1,6 @@
 Package: rhino
 Title: A Framework for Enterprise Shiny Applications
-Version: 1.7.0.9001
+Version: 1.6.0.9001
 Authors@R:
   c(
     person("Kamil", "Żyła", role = c("aut", "cre"), email = "opensource+kamil@appsilon.com"),
@@ -38,6 +38,7 @@ Imports:
   testthat (>= 3.0.0),
   utils,
   withr,
+  xml2,
   yaml
 Suggests:
   covr,
@@ -47,8 +48,7 @@ Suggests:
   rex,
   rmarkdown,
   shiny.react,
-  spelling,
-  xml2
+  spelling
 LazyData: true
 Config/testthat/edition: 3
 Config/testthat/parallel: true