Skip to content

Commit

Permalink
Friendlier error in assignment with trailing comma (#5467)
Browse files Browse the repository at this point in the history
* friendlier error in assignment with trailing comma

e.g. `DT[, `:=`(a = 1, b = 2,)`.

WIP. Need to add tests and such, but editing from browser before I forget.

* Another pass

* include unnamed indices on RHS too

* tests

* NEWS

* test numbering

* explicit example in NEWS
  • Loading branch information
MichaelChirico committed Feb 17, 2024
1 parent 2dfa42b commit 358f2bf
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

3. data.table now depends on R 3.2.0 (2015) instead of 3.1.0 (2014). 1.17.0 will likely move to R 3.3.0 (2016). Recent versions of R have good features that we would gradually like to incorporate, and we see next to no usage of these very old versions of R.

4. Erroneous assignment calls in `[` with a trailing comma (e.g. ``DT[, `:=`(a = 1, b = 2,)]``) get a friendlier error since this situation is common during refactoring and easy to miss visually. Thanks @MichaelChirico for the fix.

# data.table [v1.15.0](https://github.com/Rdatatable/data.table/milestone/29) (30 Jan 2024)

## BREAKING CHANGE
Expand Down
11 changes: 10 additions & 1 deletion R/data.table.R
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,16 @@ replace_dot_alias = function(e) {
} else {
# `:=`(c2=1L,c3=2L,...)
lhs = names(jsub)[-1L]
if (any(lhs=="")) stopf("In %s(col1=val1, col2=val2, ...) form, all arguments must be named.", if (root == "let") "let" else "`:=`")
if (!all(named_idx <- nzchar(lhs))) {
# friendly error for common case: trailing terminal comma
n_lhs = length(lhs)
root_name <- if (root == "let") "let" else "`:=`"
if (!named_idx[n_lhs] && all(named_idx[-n_lhs])) {
stopf("In %s(col1=val1, col2=val2, ...) form, all arguments must be named, but the last argument has no name. Did you forget a trailing comma?", root_name)
} else {
stopf("In %s(col1=val1, col2=val2, ...) form, all arguments must be named, but these arguments lack names: %s.", root_name, brackify(which(!named_idx)))
}
}
names(jsub)=""
jsub[[1L]]=as.name("list")
}
Expand Down
8 changes: 8 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -18245,6 +18245,7 @@ test(2243.35, dt[, min(y, na.rm=as.logical(j)), g, verbose=TRUE], data.table(
test(2243.36, dt[, max(y, na.rm=as.logical(j)), g, verbose=TRUE], data.table(g=c(1L,2L), V1=c(3L,4L)), output="GForce FALSE")
test(2243.37, dt[, var(y, na.rm=as.logical(j)), g, verbose=TRUE], data.table(g=c(1L,2L), V1=c(2,2)), output="GForce FALSE")
test(2243.38, dt[, sd(y, na.rm=as.logical(j)), g, verbose=TRUE], data.table(g=c(1L,2L), V1=c(sqrt(c(2,2)))), output="GForce FALSE")

# revdeps
dt = data.table(x = c(2,2,1,1), y = 1:4, z=letters[1:4])
i=c(1,2)
Expand All @@ -18260,3 +18261,10 @@ test(2243.54, dt[, .I[j], x]$V1, c(1L, 3L), output="GForce TRUE")
test(2243.55, dt[, .I[i], x]$V1, 1:4, output="GForce FALSE")
test(2243.56, dt[, .I[1:2], x]$V1, 1:4, output="GForce FALSE")
options(old)

DT = data.table(1)
test(2244.1, DT[, `:=`(a=1, )], error="`:=`.*Did you forget a trailing comma\\?")
test(2244.2, DT[, let(a=1, )], error="let.*Did you forget a trailing comma\\?")
test(2244.3, DT[, `:=`(a=1, , b=2)], error="`:=`.*\\[2\\]")
test(2244.4, DT[, let(a=1, , b=2)], error="let.*\\[2\\]")
test(2244.5, DT[, `:=`(a=1, , b=2, )], error="[2, 4]")

0 comments on commit 358f2bf

Please sign in to comment.