Skip to content

Commit

Permalink
Ban <<- from code base
Browse files Browse the repository at this point in the history
  • Loading branch information
klmr committed Aug 28, 2022
1 parent 389b034 commit 0345109
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 22 deletions.
32 changes: 17 additions & 15 deletions R/export.r
Original file line number Diff line number Diff line change
Expand Up @@ -119,21 +119,21 @@ parse_export_specs = function (info, exprs, mod_ns) {
}
}

block_error = function (export) {
throw(
'the {"@export";"} tag may only be applied to assignments or ',
'{"use";"} declarations;\n',
'used incorrectly in line {location}:\n',
' {paste(code, collapse = "\n ")}',
code = deparse(attr(export, 'call'), backtick = TRUE),
location = attr(export, 'location')[1L]
)
}

exports = parse_export_tags(info, exprs, mod_ns)
unique(flatmap_chr(parse_export, exports))
}

block_error = function (export) {
throw(
'the {"@export";"} tag may only be applied to assignments or ',
'{"use";"} declarations;\n',
'used incorrectly in line {location}:\n',
' {paste(code, collapse = "\n ")}',
code = deparse(attr(export, 'call'), backtick = TRUE),
location = attr(export, 'location')[1L]
)
}

#' @keywords internal
#' @rdname parse_export_specs
use_call = quote(box::use)
Expand Down Expand Up @@ -311,14 +311,16 @@ add_comments = function (refs) {
#' tag, \code{FALSE} otherwise.
#' @keywords internal
has_export_tag = function (ref) {
self = environment()

next_char = function () {
pos <<- pos + 1L
self$pos = pos + 1L
substr(line, pos, pos)
}

consume_char = function (chars) {
matched = next_char() %in% chars
if (! matched) pos <<- pos - 1L
if (! matched) self$pos = pos - 1L
matched
}

Expand All @@ -336,7 +338,7 @@ has_export_tag = function (ref) {
if (! consume_char('#')) return(FALSE)
consume_chars('#')
if (! consume_char("'")) {
pos <<- prev
self$pos = prev
return(FALSE)
}
consume_whitespace()
Expand All @@ -349,7 +351,7 @@ has_export_tag = function (ref) {
}

is_export = function () {
pos <<- pos + 1L
self$pos = pos + 1L
len = nchar('@export')
substr(line, pos, pos + len) == '@export' &&
(pos + len > nchar(line) || substr(line, pos + len, pos + len) %in% c(' ', '\t'))
Expand Down
3 changes: 2 additions & 1 deletion tests/testthat/mod/a.r
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ private_modname = box::name()
# Variables at namespace scope are locked so mutable variables need to be
# wrapped inside a closure.
make_counter = function () {
self = environment()
counter = 1L

list(
get = function () counter,
inc = function () counter <<- counter + 1L
inc = function () self$counter = counter + 1L
)
}

Expand Down
3 changes: 2 additions & 1 deletion tests/testthat/mod/hooks/a.r
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ on_load_called = 0L

#' @export
register_unload_callback = local({
self = environment()
unloaded = NULL

function (callback) {
unloaded <<- callback
self$unloaded = callback
}
}, envir = (callback = new.env()))

Expand Down
4 changes: 3 additions & 1 deletion tests/testthat/mod/issue76.r
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ h = function() {
helper()
}

ns = environment()

#' @export
helper_var = 0L

helper = function() { helper_var <<- helper_var + 1L }
helper = function() { ns$helper_var = helper_var + 1L }

f()
g()
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-S3.r
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,6 @@ test_that('nested functions are parsed correctly', {

test_that('functions with missing arguments are parsed correctly', {
expect_error(is_S3(quote(tag$span('foo', ))), NA)
expect_error(is_S3(quote(base$quote(expr = ))), NA)
expect_error(is_S3(quote((quote)(expr = ))), NA)
expect_error(is_S3(quote(base$quote(expr =))), NA)
expect_error(is_S3(quote((quote)(expr =))), NA)
})
6 changes: 4 additions & 2 deletions tests/testthat/test-hooks.r
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,23 @@ test_that('on_load hook is invoked only once', {
})

test_that('on_unload hook is invoked during unloading', {
self = environment()
unload_called = 0L

box::use(mod/hooks/a)
a$register_unload_callback(function () unload_called <<- unload_called + 1L)
a$register_unload_callback(function () self$unload_called = unload_called + 1L)

expect_equal(unload_called, 0L)
box::unload(a)
expect_equal(unload_called, 1L)
})

test_that('hooks are invoked during reloading', {
self = environment()
unload_called = 0L

box::use(mod/hooks/a)
a$register_unload_callback(function () unload_called <<- unload_called + 1L)
a$register_unload_callback(function () self$unload_called = unload_called + 1L)

expect_equal(unload_called, 0L)
box::reload(a)
Expand Down

0 comments on commit 0345109

Please sign in to comment.