Skip to content

Commit 8208b4e

Browse files
Copilotm7pr
andcommitted
Fix improper code dependency issue - exclude function calls from left side of assignments
Co-authored-by: m7pr <[email protected]>
1 parent 9feda36 commit 8208b4e

File tree

2 files changed

+104
-1
lines changed

2 files changed

+104
-1
lines changed

R/utils-get_code_dependency.R

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,28 @@ extract_occurrence <- function(pd) {
302302
}
303303

304304
after <- match(min(x$id[assign_cond]), sort(x$id[c(min(assign_cond), sym_cond)])) - 1
305-
ans <- append(x[sym_cond, "text"], "<-", after = max(1, after))
305+
306+
# Separate symbols before and after assignment
307+
symbols_before_assign <- sym_cond[x$id[sym_cond] < min(x$id[assign_cond])]
308+
symbols_after_assign <- sym_cond[x$id[sym_cond] > min(x$id[assign_cond])]
309+
310+
# Filter out function calls from left side of assignment (before assignment operator)
311+
# Function calls should only appear as dependencies (right side), not as assignment targets
312+
if (length(symbols_before_assign) > 0) {
313+
is_function_call_before <- x[symbols_before_assign, "token"] == "SYMBOL_FUNCTION_CALL"
314+
symbols_before_assign <- symbols_before_assign[!is_function_call_before]
315+
}
316+
317+
# Combine all symbols (filtered left side + all right side)
318+
filtered_sym_cond <- c(symbols_before_assign, symbols_after_assign)
319+
320+
# Update the after position based on filtered symbols
321+
if (length(filtered_sym_cond) > 0) {
322+
after <- match(min(x$id[assign_cond]), sort(x$id[c(min(assign_cond), filtered_sym_cond)])) - 1
323+
ans <- append(x[filtered_sym_cond, "text"], "<-", after = max(1, after))
324+
} else {
325+
ans <- "<-"
326+
}
306327
roll <- in_parenthesis(pd)
307328
if (length(roll)) {
308329
c(setdiff(ans, roll), roll)
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
testthat::test_that("function calls should not appear on left side of assignment in dependencies", {
2+
# Test for the specific issue reported in GitHub issue #262
3+
4+
# Create a minimal example that reproduces the problem
5+
code1 <- 'ADMH[c("test")] <- c("value")'
6+
7+
# Parse and extract dependencies
8+
parsed1 <- parse(text = code1, keep.source = TRUE)
9+
10+
# Extract dependencies using the internal function
11+
deps1 <- teal.code:::extract_dependency(parsed1)
12+
13+
# The issue: function 'c' should not appear on left side of assignment
14+
# In deps1, 'c' appears before '<-', which is incorrect
15+
assign_pos1 <- which(deps1 == "<-")
16+
left_side1 <- if(length(assign_pos1) > 0) deps1[seq_len(assign_pos1[1] - 1)] else character(0)
17+
18+
# Function calls should NOT appear on the left side of assignment
19+
testthat::expect_false("c" %in% left_side1,
20+
info = "Function 'c' should not appear on left side of assignment in dependencies")
21+
22+
# Only actual variables/objects should appear on left side
23+
# In this case, only 'ADMH' should be on the left side
24+
testthat::expect_true("ADMH" %in% left_side1,
25+
info = "Variable 'ADMH' should appear on left side as it's being assigned to")
26+
})
27+
28+
testthat::test_that("complex assignment with function calls handles dependencies correctly", {
29+
# This test reproduces the exact scenario from the GitHub issue
30+
31+
code_admh <- 'teal.data::col_labels(ADMH[c("MHDISTAT")]) <- c("Status of Disease")'
32+
33+
parsed_admh <- parse(text = code_admh, keep.source = TRUE)
34+
35+
deps_admh <- teal.code:::extract_dependency(parsed_admh)
36+
37+
# Check ADMH dependencies
38+
assign_pos_admh <- which(deps_admh == "<-")
39+
left_side_admh <- if(length(assign_pos_admh) > 0) deps_admh[seq_len(assign_pos_admh[1] - 1)] else character(0)
40+
right_side_admh <- if(length(assign_pos_admh) > 0) deps_admh[seq(assign_pos_admh[1] + 1, length(deps_admh))] else character(0)
41+
42+
# Function calls should not be on left side of assignment
43+
testthat::expect_false("c" %in% left_side_admh,
44+
info = "Function 'c' should not be on left side in ADMH dependencies")
45+
testthat::expect_false("col_labels" %in% left_side_admh,
46+
info = "Function 'col_labels' should not be on left side in ADMH dependencies")
47+
48+
# Functions can be on right side as dependencies
49+
testthat::expect_true("c" %in% right_side_admh,
50+
info = "Function 'c' can appear on right side as a dependency")
51+
52+
# Variables being modified should be on left side
53+
testthat::expect_true("ADMH" %in% left_side_admh,
54+
info = "Variable 'ADMH' should be on left side as it's being modified")
55+
})
56+
57+
testthat::test_that("function calls in complex expressions are handled correctly", {
58+
# Test various complex expressions to ensure function calls don't appear on left side
59+
60+
test_cases <- list(
61+
'result <- fun(a, b)',
62+
'data[filter(x)] <- transform(y)',
63+
'obj$method(params) <- compute(values)'
64+
)
65+
66+
for (test_code in test_cases) {
67+
parsed_code <- parse(text = test_code, keep.source = TRUE)
68+
deps <- teal.code:::extract_dependency(parsed_code)
69+
70+
assign_pos <- which(deps == "<-")
71+
if (length(assign_pos) > 0) {
72+
left_side <- deps[seq_len(assign_pos[1] - 1)]
73+
74+
# No function calls should appear on left side
75+
function_names <- c("fun", "filter", "transform", "method", "compute")
76+
for (func_name in function_names) {
77+
testthat::expect_false(func_name %in% left_side,
78+
info = paste("Function", func_name, "should not appear on left side in:", test_code))
79+
}
80+
}
81+
}
82+
})

0 commit comments

Comments
 (0)