From 358f2bffc97e1f72e62fcef71ca36cdf71d5f12f Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 14 Jan 2024 15:13:36 +0000 Subject: [PATCH] Friendlier error in assignment with trailing comma (#5467) * 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 --- NEWS.md | 2 ++ R/data.table.R | 11 ++++++++++- inst/tests/tests.Rraw | 8 ++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 650422d02..5505c4cbc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -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 diff --git a/R/data.table.R b/R/data.table.R index 2216e4b3e..6368df4eb 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -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") } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index b0a6367d2..a5a7708bc 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -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) @@ -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]")