improve error message for nested assignment (#6484)

* improve error message for nested assignment

* new test case (will fail)

* working now

* improve error message for nested assignment

* need jj-1 (call itself is 1->offset)

* fix test to be applicable here

* Catch partially-named nested usage too

* consistent local naming

* move NEWS

* Use a helper to reduce repetition

Author: MichaelChirico

NEWS.md

@@ -79,6 +79,8 @@ rowwiseDT(
 
 6. `set()` and `:=` now provide some extra guidance for common incorrect approaches to assigning `NULL` to some rows of a list column. The correct way is to put `list(list(NULL))` on the RHS of `:=` (or `.(.(NULL))` for short). Thanks to @MichaelChirico for the suggestion and @Nj221102 for the implementation.
 
+7. Improved the error message when trying to write code like `DT[, ":="(a := b, c := d)]` (which should be `DT[, ":="(a = b, c = d)]`), [#5296](https://github.com/Rdatatable/data.table/issues/5296). Thanks @MichaelChirico for the suggestion & fix.
+
 # data.table [v1.16.0](https://github.com/Rdatatable/data.table/milestone/30)  (25 August 2024)
 
 ## BREAKING CHANGES

R/data.table.R

@@ -1111,7 +1111,10 @@ replace_dot_alias = function(e) {
         if (is.null(names(jsub))) {
           # regular LHS:=RHS usage, or `:=`(...) with no named arguments (an error)
           # `:=`(LHS,RHS) is valid though, but more because can't see how to detect that, than desire
-          if (length(jsub)!=3L) stopf("In %s(col1=val1, col2=val2, ...) form, all arguments must be named.", if (root == "let") "let" else "`:=`")
+          this_call = if (root == "let") "let" else "`:=`"
+          .check_nested_walrus(jsub, 2:length(jsub), this_call)
+          if (length(jsub) != 3L)
+            stopf("In %s(col1=val1, col2=val2, ...) form, all arguments must be named.", this_call)
           lhs = jsub[[2L]]
           jsub = jsub[[3L]]
           if (is.name(lhs)) {
@@ -1131,11 +1134,12 @@ replace_dot_alias = function(e) {
           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 "`:=`"
+            this_call <- if (root == "let") "let" else "`:=`"
+            .check_nested_walrus(jsub, which(!named_idx)+1L, this_call)
             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)
+              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?", this_call)
             } 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)))
+              stopf("In %s(col1=val1, col2=val2, ...) form, all arguments must be named, but these arguments lack names: %s.", this_call, brackify(which(!named_idx)))
             }
           }
           names(jsub)=""
@@ -3144,6 +3148,11 @@ is_constantish = function(q, check_singleton=FALSE) {
   q
 }
 
+.check_nested_walrus = function(e, check_entries, call_name) {
+  for (jj in check_entries) if (e[[jj]] %iscall% ":=")
+    stopf("It looks like you re-used `:=` in argument %d a functional assignment call -- use `=` instead: %s(col1=val1, col2=val2, ...)", jj-1L, call_name)
+}
+
 .prepareFastSubset = function(isub, x, enclos, notjoin, verbose = FALSE){
   ## helper that decides, whether a fast binary search can be performed, if i is a call
   ## For details on the supported queries, see \code{\link{datatable-optimize}}

inst/tests/tests.Rraw

@@ -19227,3 +19227,13 @@ if (test_bit64) {
 dt = data.table(a=1:2, b=expression(1,2))
 test(2289.1, dt[1,], data.table(a=1L, b=expression(1)))
 test(2289.2, dt[,b,a], dt)
+
+# error message improvement, #5296
+DT = data.table(a = 1)
+test(2290.1, DT[, `:=`(a := 2)], error="It looks like you re-used `:=`")
+# special case where length(jsub) == 3 that would have failed later
+test(2290.2, DT[, `:=`(a := 2, c := 3)], error="It looks like you re-used `:=` in argument 1")
+# NB: _not_ a = 2, because that means partially-named --> not currently covered
+test(2290.3, DT[, `:=`(a, c := 3)], error="It looks like you re-used `:=` in argument 2")
+# partially-named `:=`(...) --> different branch, same error
+test(2290.4, DT[, `:=`(a = 2, c := 3)], error="It looks like you re-used `:=` in argument 2")