DT[,foo:=bar]: earlier check for selfrefok

[aitap]

Kludge towards #7498; amends #6789 and an earlier half-fix, #7488. Maybe it gives someone a better idea, I don't know. It does fix bbotk.

The check needs to happen before eval(jsub) / .Call(dogroups) because the latter will try to assign the results into the table, potentially trying to resize the number of columns.

Use static analysis to detect common forms of column deletion: foo := NULL and c("bar", "baz") := .(NULL, NULL). Static analysis is doomed to miss things like frob := if (runif(1) < .5) 42 else NULL, but hopefully it covers the needs of our reverse dependencies.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.04%. Comparing base (64e9ec1) to head (5786d1f).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7502   +/-   ##
=======================================
  Coverage   99.04%   99.04%           
=======================================
  Files          87       87           
  Lines       16699    16704    +5     
=======================================
+ Hits        16539    16544    +5     
  Misses        160      160           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

• HEAD=earlier-resize-checks stopped early for isoweek improved in #7144
  

Generated via commit 5786d1f

Download link for the artifact containing the test results: ↓ atime-results.zip

Task	Duration
R setup and installing dependencies	3 minutes and 15 seconds
Installing different package versions	45 seconds
Running and plotting the test cases	5 minutes and 9 seconds

[aitap]

Fixes: bbotk, blocking, circhelp, hicp, metR, mlr3tuning, SeaVal, slim, tidyrules, tidytable, ume, webtrackR, DramaAnalysis.

Details

Before:

dir.create('extrastuff/Library', FALSE, TRUE)
p <- c("bbotk", "blocking", "circhelp", "hicp", "metR", "mlr3tuning",
"SeaVal", "slim", "tidyrules", "tidytable", "ume", "webtrackR",
"DramaAnalysis")
download.packages(p, 'extrastuff')
install.packages('https://cran.r-project.org/incoming/waiting/data.table_1.18.0.tar.gz', lib = 'extras
tuff/Library')
tools::check_packages_in_dir('extrastuff', xvfb=TRUE, Ncpus=13)
tools::check_packages_in_dir_details('extrastuff') -> de_1.18
de_1.18 |> within(Status <- ordered(Status, c('OK','NOTE','WARNING','ERROR'))) |> aggregate(Status ~ Package, max)

         Package Status
1          bbotk  ERROR
2       blocking  ERROR
3       circhelp  ERROR
4  DramaAnalysis  ERROR
5           hicp  ERROR
6           metR  ERROR
7     mlr3tuning  ERROR
8         SeaVal  ERROR
9           slim  ERROR
10     tidyrules  ERROR
11     tidytable  ERROR
12           ume  ERROR
13     webtrackR  ERROR

After:

install.packages('data.table_1.17.99.tar.gz', lib = 'extrastuff/Library')
tools::check_packages_in_dir('extrastuff', xvfb=TRUE, Ncpus=13)
tools::check_packages_in_dir_details('extrastuff') -> de_pr7502
de_pr7502 |> within(Status <- ordered(Status, c('OK','NOTE','WARNING','ERROR'))) |> aggregate(Status ~ Package, max)

         Package  Status
1          bbotk      OK
2       blocking      OK
3       circhelp    NOTE
4  DramaAnalysis      OK
5           hicp      OK
6           metR      OK
7     mlr3tuning      OK
8         SeaVal      OK
9           slim WARNING
10     tidyrules      OK
11     tidytable      OK
12           ume      OK
13     webtrackR      OK

subset(de_pr7502, Status != 'OK') # unrelated to data.table

Package: circhelp 1.1
Check: DESCRIPTION meta-information, Result: NOTE
    Missing dependency on R >= 4.1.0 because package code uses the pipe
    |> or function shorthand \(...) syntax added in R 4.1.0.
    File(s) using such syntax:
      ‘functions.R’

Package: slim 0.1.1
Check: Rd files, Result: NOTE
  checkRd: (-1) fit_slim.Rd:24: Lost braces; missing escapes or markup?
      24 | sum(x' V^{-1} (y - x beta)) = 0 as the covariance V tends from V[, , 1] +
         |          ^

Package: slim 0.1.1
Check: re-building of vignette outputs, Result: WARNING
  Error(s) in re-building vignettes:
    ...
  --- пересоздаю ‘slim.Rmd’ с использованием rmarkdown
  ! LaTeX Error: Unicode character З (U+0417)
                 not set up for use with LaTeX.

  Try other LaTeX engines instead (e.g., xelatex) if you are using pdflatex. See https://bookdown.org/yihui/rmarkdown-cookbook/latex-unicode.html
  Ошибка: выполнение очерка 'slim.Rmd' не удалось с диагностикой:
  LaTeX failed to compile /home/ivan/vcs/data.table/extrastuff/slim.Rcheck/vign_test/slim/vignettes/slim.tex. See https://yihui.org/tinytex/r/#debugging for debugging tips. See slim.log for more info.
  --- не удалось пересоздать ‘slim.Rmd’

  РЕЗУЛЬТАТ: обработка следующих файлов не удалась:
    ‘slim.Rmd’

  Ошибка: Vignette re-building failed.
  Выполнение остановлено

What if we checked for new columns before ordinary/by-group evaluation and additionally checked for column removal after ordinary evaluation? x[, foo := NULL, by = bar] is not something people normally run, right?

[HughParsonage]

If by is absent, then evaluate jsub to materialize jval then inspect jval for NULL then setalloc.

Just to avoid this triggering other regressions if people have been using symbols to refer to NULL programmatically rather than syntactically

[aitap]

Thank you! With this commit, the 13 packages flagged for this issue pass their R CMD checks with nothing worse than NOTE.

[ben-schwen]

Nice! I guess we need to battle test this throughly but as a bandaid it should hold