Skip to content

Commit 0461fbe

Browse files
Fixes bug in execution with multiple assignments inside expression (#250)
# Pull Request <!--- Replace `#nnn` with your issue link for reference. --> Fixes #249 # Changes description - Detects expression blocks and handles them one by one - Any co-occurence is tracked to the single line --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 3b0935c commit 0461fbe

File tree

4 files changed

+158
-8
lines changed

4 files changed

+158
-8
lines changed

NEWS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# teal.code 0.6.1.9001
22

3+
### Bug fixes
4+
5+
* Fix a problem detecting co-occurrences when expression has multiple lines.
6+
37
# teal.code 0.6.1
48

59
### Bug fixes

R/utils-get_code_dependency.R

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -340,15 +340,72 @@ extract_side_effects <- function(pd) {
340340
#' @keywords internal
341341
#' @noRd
342342
extract_dependency <- function(parsed_code) {
343-
pd <- normalize_pd(utils::getParseData(parsed_code))
344-
reordered_pd <- extract_calls(pd)
345-
if (length(reordered_pd) > 0) {
346-
# extract_calls is needed to reorder the pd so that assignment operator comes before symbol names
347-
# extract_calls is needed also to substitute assignment operators into specific format with fix_arrows
348-
# extract_calls is needed to omit empty calls that contain only one token `"';'"`
349-
# This cleaning is needed as extract_occurrence assumes arrows are fixed, and order is different than in original pd
350-
c(extract_side_effects(reordered_pd[[1]]), extract_occurrence(reordered_pd[[1]]))
343+
full_pd <- normalize_pd(utils::getParseData(parsed_code))
344+
reordered_full_pd <- extract_calls(full_pd)
345+
346+
# Early return on empty code
347+
if (length(reordered_full_pd) == 0L) {
348+
return(NULL)
349+
}
350+
351+
if (length(parsed_code) == 0L) {
352+
return(extract_side_effects(reordered_full_pd[[1]]))
353+
}
354+
expr_ix <- lapply(parsed_code[[1]], class) == "{"
355+
356+
# Build queue of expressions to parse individually
357+
queue <- list()
358+
parsed_code_list <- if (all(!expr_ix)) {
359+
list(parsed_code)
360+
} else {
361+
queue <- as.list(parsed_code[[1]][expr_ix])
362+
new_list <- parsed_code[[1]]
363+
new_list[expr_ix] <- NULL
364+
list(parse(text = as.expression(new_list), keep.source = TRUE))
365+
}
366+
367+
while (length(queue) > 0) {
368+
current <- queue[[1]]
369+
queue <- queue[-1]
370+
if (identical(current[[1L]], as.name("{"))) {
371+
queue <- append(queue, as.list(current)[-1L])
372+
} else {
373+
parsed_code_list[[length(parsed_code_list) + 1]] <- parse(text = as.expression(current), keep.source = TRUE)
374+
}
351375
}
376+
377+
parsed_occurences <- lapply(
378+
parsed_code_list,
379+
function(parsed_code) {
380+
pd <- normalize_pd(utils::getParseData(parsed_code))
381+
reordered_pd <- extract_calls(pd)
382+
if (length(reordered_pd) > 0) {
383+
# extract_calls is needed to reorder the pd so that assignment operator comes before symbol names
384+
# extract_calls is needed also to substitute assignment operators into specific format with fix_arrows
385+
# extract_calls is needed to omit empty calls that contain only one token `"';'"`
386+
# This cleaning is needed as extract_occurrence assumes arrows are fixed, and order is different
387+
# than in original pd
388+
extract_occurrence(reordered_pd[[1]])
389+
}
390+
}
391+
)
392+
393+
# Merge results together
394+
result <- Reduce(
395+
function(u, v) {
396+
ix <- if ("<-" %in% v) min(which(v == "<-")) else 0
397+
u$left_side <- c(u$left_side, v[seq_len(max(0, ix - 1))])
398+
u$right_side <- c(
399+
u$right_side,
400+
if (ix == length(v)) character(0L) else v[seq(ix + 1, max(ix + 1, length(v)))]
401+
)
402+
u
403+
},
404+
init = list(left_side = character(0L), right_side = character(0L)),
405+
x = parsed_occurences
406+
)
407+
408+
c(extract_side_effects(reordered_full_pd[[1]]), result$left_side, "<-", result$right_side)
352409
}
353410

354411
# graph_parser ----

teal.code.Rproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
Version: 1.0
2+
ProjectId: 97c2c74b-5d15-476b-a5df-d94d70687438
23

34
RestoreWorkspace: No
45
SaveWorkspace: No
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
testthat::describe("get_code with single assignments inside an expression", {
2+
testthat::it("detects assign() function", {
3+
td <- qenv() |>
4+
within({
5+
for (i in 1:10) {
6+
assign("var1", iris)
7+
}
8+
})
9+
10+
code_source <- "for (i in 1:10) {\n assign(\"var1\", iris)\n}"
11+
12+
testthat::expect_equal(get_code(td, names = "var1"), code_source)
13+
})
14+
15+
testthat::it("detects <-", {
16+
td <- qenv() |>
17+
within({
18+
for (i in 1:10) {
19+
var1 <- iris
20+
}
21+
})
22+
23+
code_source <- "for (i in 1:10) {\n var1 <- iris\n}"
24+
25+
testthat::expect_equal(get_code(td, names = "var1"), code_source)
26+
})
27+
28+
testthat::it("detects ->", {
29+
td <- qenv() |>
30+
within({
31+
for (i in 1:10) {
32+
iris -> var1 # nolint: assignment.
33+
}
34+
})
35+
36+
# Reversed order of operation
37+
code_source <- "for (i in 1:10) {\n var1 <- iris\n}"
38+
39+
testthat::expect_equal(get_code(td, names = "var1"), code_source)
40+
})
41+
})
42+
43+
testthat::describe("get_code with multiple assignments inside an expression", {
44+
testthat::it("detects assign() function", {
45+
td <- qenv() |>
46+
within({
47+
for (i in 1:10) {
48+
assign("var1", iris)
49+
assign("var2", mtcars)
50+
}
51+
})
52+
53+
code_source <- "for (i in 1:10) {\n assign(\"var1\", iris)\n assign(\"var2\", mtcars)\n}"
54+
55+
testthat::expect_equal(get_code(td, names = "var1"), code_source)
56+
testthat::expect_equal(get_code(td, names = "var2"), code_source)
57+
})
58+
59+
testthat::it("detects <- function", {
60+
td <- qenv() |>
61+
within({
62+
for (i in 1:10) {
63+
var1 <- iris
64+
var2 <- mtcars
65+
}
66+
})
67+
68+
code_source <- "for (i in 1:10) {\n var1 <- iris\n var2 <- mtcars\n}"
69+
70+
testthat::expect_equal(get_code(td, names = "var1"), code_source)
71+
testthat::expect_equal(get_code(td, names = "var2"), code_source)
72+
})
73+
74+
testthat::it("detects -> function", {
75+
td <- qenv() |>
76+
within({
77+
for (i in 1:10) {
78+
iris -> var1 # nolint: assignment.
79+
mtcars -> var2 # nolint: assignment.
80+
}
81+
})
82+
83+
code_source <- "for (i in 1:10) {\n var1 <- iris\n var2 <- mtcars\n}"
84+
85+
testthat::expect_equal(get_code(td, names = "var1"), code_source)
86+
testthat::expect_equal(get_code(td, names = "var2"), code_source)
87+
})
88+
})

0 commit comments

Comments
 (0)