Skip to content

Commit

Permalink
Allow relative box module paths when box.path = NULL (#120)
Browse files Browse the repository at this point in the history
  • Loading branch information
radbasa authored Jul 15, 2024
1 parent 3a4972a commit b5f6704
Show file tree
Hide file tree
Showing 13 changed files with 142 additions and 5 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package: box.linters
Title: Linters for 'box' Modules
Version: 0.9.1.9003
Version: 0.9.1.9004
Authors@R:
c(
person("Ricardo Rodrigo", "Basa", role = c("aut", "cre"), email = "[email protected]"),
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# box.linters (development version)

* [Bug fix] Allow relative box module paths (#110)
* Less verbose `box_alphabetical_calls_linter()`. Reports only the first out-of-place function.
* Added styling functions for `box::use()` calls.
* [Bug fix] Allow multiple `box::use(pkg)` calls (#111)
Expand Down
6 changes: 5 additions & 1 deletion R/box_mod_fun_exists_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ box_mod_fun_exists_linter <- function() {

xml <- source_expression$full_xml_parsed_content

mod_fun_not_exists <- check_attached_mod_funs(xml, xpath_module_functions)
working_dir <- get_module_working_dir(source_expression)

withr::with_dir(working_dir, {
mod_fun_not_exists <- check_attached_mod_funs(xml, xpath_module_functions)
})

lapply(mod_fun_not_exists$xml, function(xml_node) {
lintr::xml_nodes_to_lints(
Expand Down
11 changes: 11 additions & 0 deletions R/box_module_usage_helper_functions.R
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,14 @@ get_attached_mod_functions <- function(xml) {
text = attached_functions$text
)
}

#' @keywords internal
get_module_working_dir <- function(source_expression) {
box_path <- getOption("box.path")
if (is.null(box_path)) {
working_dir <- fs::path_dir(source_expression$filename)
} else {
working_dir <- box_path
}
working_dir
}
8 changes: 6 additions & 2 deletions R/box_unused_attached_mod_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,12 @@ box_unused_attached_mod_linter <- function() {

xml <- source_expression$full_xml_parsed_content

attached_modules <- get_attached_modules(xml)
attached_three_dots <- get_attached_mod_three_dots(xml)
working_dir <- get_module_working_dir(source_expression)

withr::with_dir(working_dir, {
attached_modules <- get_attached_modules(xml)
attached_three_dots <- get_attached_mod_three_dots(xml)
})
function_calls <- get_function_calls(xml)
glue_object_calls <- get_objects_in_strings(xml)
possible_module_calls <- get_object_calls(xml)
Expand Down
6 changes: 5 additions & 1 deletion R/box_unused_attached_mod_obj_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ box_unused_att_mod_obj_linter <- function() {

xml <- source_expression$full_xml_parsed_content

attached_functions_objects <- get_attached_mod_functions(xml)
working_dir <- get_module_working_dir(source_expression)

withr::with_dir(working_dir, {
attached_functions_objects <- get_attached_mod_functions(xml)
})
function_calls <- get_function_calls(xml)
object_calls <- get_object_calls(xml)
glue_object_calls <- get_objects_in_strings(xml)
Expand Down
5 changes: 5 additions & 0 deletions tests/testthat/mod/path/relative/module_d.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
box::use(
../to/module_a
)

module_a$a_fun_a()
5 changes: 5 additions & 0 deletions tests/testthat/mod/path/relative/module_e.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
box::use(
../to/module_a[a_fun_a]
)

a_fun_a()
5 changes: 5 additions & 0 deletions tests/testthat/mod/path/relative/module_f.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
box::use(
../to/module_a[not_exist]
)

a_fun_a()
3 changes: 3 additions & 0 deletions tests/testthat/mod/path/relative/module_g.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
box::use(
../to/module_a
)
31 changes: 31 additions & 0 deletions tests/testthat/test-box_mod_fun_exists_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,34 @@ test_that("box_mod_fun_exists_linter blocks aliased functions that do not exist

lintr::expect_lint(bad_box_usage, list(message = lint_message), linter)
})

test_that("box_mod_fun_exists_linter allows relative module paths", {
linter <- box_mod_fun_exists_linter()

withr::with_options(
list(
"box.path" = NULL
), {
withr::with_dir(file.path(getwd(), "mod", "path", "relative"), {
expect_no_message(lintr::lint("module_d.R", linters = linter))
})
}
)
})

test_that("box_mod_fun_exists_linter blocks non-existing functions in relative module paths", {
linter <- box_mod_fun_exists_linter()
lint_message <- "Function not exported by module."
withr::with_options(
list(
"box.path" = NULL
), {
withr::with_dir(file.path(getwd(), "mod", "path", "relative"), {
result <- lintr::lint("module_f.R", linters = linter)
})
}
)

expect_s3_class(result, "lints")
expect_equal(result[[1]]$message, lint_message)
})
29 changes: 29 additions & 0 deletions tests/testthat/test-box_unused_attached_mod_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,35 @@ test_that("box_unused_attached_mod_linter blocks unused objects in glue string t
lintr::expect_lint(bad_box_usage, list(message = lint_message), linters = linter)
})

test_that("box_unused_attached_mod_linter works with relative paths", {
linter <- box_unused_attached_mod_linter()

withr::with_options(
list(
"box.path" = NULL
),
withr::with_dir(file.path(getwd(), "mod", "path", "relative"), {
expect_no_message(lintr::lint("module_d.R", linters = linter))
})
)
})

test_that("box_unused_attached_mod_linter detects unused modules with relative paths", {
linter <- box_unused_attached_mod_linter()
lint_message <- "Attached module unused."

withr::with_options(
list(
"box.path" = NULL
),
withr::with_dir(file.path(getwd(), "mod", "path", "relative"), {
result <- lintr::lint("module_g.R", linters = linter)
})
)

expect_s3_class(result, "lints")
expect_equal(result[[1]]$message, lint_message)
})

# Box test interfaces, not implementations

Expand Down
35 changes: 35 additions & 0 deletions tests/testthat/test-box_unused_attached_mod_obj_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,38 @@ test_that("box_unused_att_mod_obj_linter blocks unused objects in glue string te

lintr::expect_lint(bad_box_usage, list(message = lint_message), linters = linter)
})

test_that("box_unused_att_mod_obj_linter allows relative module paths", {
linter <- box_unused_att_mod_obj_linter()

withr::with_options(
list(
"box.path" = NULL
), {
withr::with_dir(file.path(getwd(), "mod", "path", "relative"), {
code <- "box::use(../to/module_a[a_fun_a])
a_fun_a()
"
lintr::expect_lint(code, NULL, linters = linter)

})
}
)
})

test_that("box_unused_att_mod_obj_linter blocks unused functions from relative module paths", {
linter <- box_unused_att_mod_obj_linter()
lint_message <- rex::rex("Imported function/object unused.")

withr::with_options(
list(
"box.path" = NULL
), {
withr::with_dir(file.path(getwd(), "mod", "path", "relative"), {
code <- "box::use(../to/module_a[a_fun_a])
"
lintr::expect_lint(code, list(message = lint_message), linters = linter)
})
}
)
})

0 comments on commit b5f6704

Please sign in to comment.