Skip to content

Commit f3371af

Browse files
committed
Refine iterable coverage
1 parent 8147c3d commit f3371af

4 files changed

Lines changed: 124 additions & 11 deletions

File tree

R/r2f.R

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -822,15 +822,6 @@ seq_like_parse <- function(name, args, scope) {
822822
from <- seq_call$from
823823
to <- seq_call$to
824824
by <- seq_call$by
825-
if (is_missing(from)) {
826-
from <- NULL
827-
}
828-
if (is_missing(to)) {
829-
to <- NULL
830-
}
831-
if (is_missing(by)) {
832-
by <- NULL
833-
}
834825

835826
if (is.null(from) || is.null(to)) {
836827
stop("seq() requires both `from` and `to`", call. = FALSE)
@@ -2030,8 +2021,7 @@ r2f_for_iterable <- function(iterable, scope, ...) {
20302021
...,
20312022
context = "for",
20322023
reversed = TRUE
2033-
),
2034-
stop_unsupported(original)
2024+
)
20352025
)
20362026
}
20372027

tests/testthat/test-for-iterables.R

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,14 @@ test_that("for() iterable errors are clear", {
115115
s
116116
}
117117

118+
literal_iterable <- function() {
119+
s <- 0L
120+
for (i in 1L) {
121+
s <- s + i
122+
}
123+
s
124+
}
125+
118126
expect_error(
119127
quick(unsupported_iterable),
120128
regexp = "unsupported iterable in for\\(\\): rev\\(list\\("
@@ -127,6 +135,10 @@ test_that("for() iterable errors are clear", {
127135
quick(non_integer_seq_len),
128136
regexp = "seq_len\\(\\) expects an integer scalar"
129137
)
138+
expect_error(
139+
quick(literal_iterable),
140+
regexp = "unsupported iterable in for\\(\\): 1"
141+
)
130142
})
131143

132144
test_that("for() supports rev() on index iterables", {
@@ -171,6 +183,35 @@ test_that("for() supports rev() on index iterables", {
171183
expect_quick_identical(rev_seq_along, numeric(), c(1, 2, 3))
172184
})
173185

186+
test_that("for() rev(seq()) validates scalar bounds and step", {
187+
non_scalar_bounds <- function(x) {
188+
declare(type(x = integer(NA)))
189+
s <- 0L
190+
for (i in rev(seq(1L, x))) {
191+
s <- s + i
192+
}
193+
s
194+
}
195+
196+
non_scalar_step <- function(x) {
197+
declare(type(x = integer(NA)))
198+
s <- 0L
199+
for (i in rev(seq(1L, 5L, by = x))) {
200+
s <- s + i
201+
}
202+
s
203+
}
204+
205+
expect_error(
206+
quick(non_scalar_bounds),
207+
regexp = "seq\\(\\) iterable bounds must be scalars"
208+
)
209+
expect_error(
210+
quick(non_scalar_step),
211+
regexp = "seq\\(\\) iterable step must be a scalar"
212+
)
213+
})
214+
174215
test_that("for() supports iterating over a symbol (value iteration)", {
175216
sum_values <- function(x) {
176217
declare(type(x = double(NA)))

tests/testthat/test-internal-utils.R

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,73 @@ test_that("defer schedules cleanup code on exit", {
117117
f()
118118
expect_true(isTRUE(e$done))
119119
})
120+
121+
test_that("iterable helpers cover edge cases", {
122+
expect_null(quickr:::r2f_iterable_context(NULL))
123+
expect_null(quickr:::r2f_iterable_context(character()))
124+
expect_null(quickr:::r2f_iterable_context("seq"))
125+
expect_null(quickr:::r2f_iterable_context(c("foo", "seq")))
126+
expect_identical(quickr:::r2f_iterable_context(c("[", "seq")), "[")
127+
expect_identical(quickr:::r2f_iterable_context(c("for", "seq")), "for")
128+
129+
var_na <- quickr:::Variable("double", list(NA_integer_))
130+
expect_identical(quickr:::value_length_expr(var_na), NA_integer_)
131+
132+
expect_identical(quickr:::seq_like_length_expr(NULL, 1L), NA_integer_)
133+
expect_identical(quickr:::seq_like_length_expr(1L, 1), 1L)
134+
expect_error(
135+
quickr:::seq_like_length_expr(1L, 2L, 0L),
136+
"invalid '\\(to - from\\)/by'"
137+
)
138+
139+
scope <- new.env(parent = emptyenv())
140+
expect_error(
141+
quickr:::seq_like_parse(
142+
"seq",
143+
list(from = quote(expr = ), to = 5L),
144+
scope
145+
),
146+
"seq\\(\\) requires both `from` and `to`"
147+
)
148+
expect_error(
149+
quickr:::seq_like_parse(
150+
"seq",
151+
list(from = 5L, to = quote(expr = )),
152+
scope
153+
),
154+
"seq\\(\\) requires both `from` and `to`"
155+
)
156+
out <- quickr:::seq_like_parse(
157+
"seq",
158+
list(from = 1L, to = 3L, by = quote(expr = )),
159+
scope
160+
)
161+
expect_null(out$by)
162+
expect_error(
163+
quickr:::seq_like_parse("banana", list(), scope),
164+
"unsupported iterable"
165+
)
166+
167+
expect_false(quickr:::iterable_is_singleton_one(1L, scope))
168+
expect_false(quickr:::iterable_is_singleton_one(quote(foo(1L)), scope))
169+
expect_false(quickr:::iterable_is_singleton_one(quote(seq(1L)), scope))
170+
expect_false(
171+
quickr:::iterable_is_singleton_one(quote(seq_along(x)), scope)
172+
)
173+
174+
scope2 <- quickr:::new_scope(function() NULL)
175+
expect_error(
176+
quickr:::r2f_for_iterable(quote(x), scope2),
177+
"unsupported iterable in for\\(\\)"
178+
)
179+
})
180+
181+
test_that("iterable_is_singleton_one handles unknown iterable kinds", {
182+
scope <- new.env(parent = emptyenv())
183+
local_mocked_bindings(
184+
seq_like_parse = function(...) list(kind = "bogus"),
185+
.package = "quickr"
186+
)
187+
188+
expect_false(quickr:::iterable_is_singleton_one(quote(seq_len(1L)), scope))
189+
})

tests/testthat/test-seq.R

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,18 @@ test_that("seq_along returns linear indices for arrays", {
4242
)
4343
})
4444

45+
test_that("seq_along requires a value", {
46+
bad_seq_along <- function() {
47+
out <- seq_along(NULL)
48+
out
49+
}
50+
51+
expect_error(
52+
quick(bad_seq_along),
53+
regexp = "seq_along\\(\\) argument must have a value"
54+
)
55+
})
56+
4557
test_that("seq rejects length.out and along.with", {
4658
bad_length_out <- function() {
4759
out <- seq(1L, 5L, length.out = 3L)

0 commit comments

Comments
 (0)