Surface diagnostics for ambiguous effects

[lionel-]

Progress towards #1338

Follow-up to #1379

• Surfaces the EffectAmbiguity diagnostic (previously only logged) as real LSP diagnostics, gated behind a new oak.diagnostics.experimental.enable setting (off by default).

• Adds a second, independent diagnostic: UninstalledPackage, flagging a library()/require() call whose target package doesn't resolve. Also gated by the experimental setting.

• Diagnostic tests are now insta snapshots rendered via rustc-style annotate-snippets (the same renderer ty and ruff use), showing the annotated source instead of asserted ranges.

Positron Release Notes

New Features

• N/A

Bug Fixes

• N/A

[lionel-]

@thomasp85 I've had Claude generate a QA review guide for this stack.

The main theme is that Oak now recognises these effects for a bunch of functions whose semantics are documented in an effects registry:

• NSE / local evaluation, e.g. local() or test_that().
• Attach, e.g. library()
• Source, e.g. source()
• Assignment, e.g. := or delayedAssign()

The effects are resolved, meaning that you can import effectful functions via attach or source (or a combination, e.g. sourcing a file that attaches packages). Or on the contrary, masking the symbol in some way with some other non-effectful function should remove the effect. The combination of control-flow, NSE, source, attach, and assignment effects may lead to very complex interactions, some of which are ambiguous. The following guide documents basic effects, interactions, and ambiguity lints.

---

Covers the whole oak-nse/1 through oak-nse/22 stack (progress towards
#1338). Organized by testable feature
area rather than by PR, since most of these PRs build toward one coherent
capability: Oak's semantic index correctly recognizing R's non-standard
evaluation (NSE) patterns — calls whose effect on scoping, symbol resolution,
or the file's import graph can't be read off the syntax alone.

For each area: what to try, and what goto-definition / find-references /
rename / diagnostics should do.

Setup

Section 2 covers experimental diagnostics, off by default. Turn them on up
front so they're live for the rest of this pass, in settings.json:

{
  "oak.diagnostics.experimental.enable": true
}

1. Effect kinds recognized

Each of these makes a call "effectful" from Oak's point of view: it changes
what's in scope, what's bound, or what file/package is involved, in a way
that isn't just "this is a function call."

1.1 Scope-creating NSE — local(), test_that(), with(), reactive() (#1339)

test_that("some test", {
  x <- 1
  x
})

x should resolve within the test_that() body as its own scope, same as a
function body would. Try goto-def/rename on x from inside and outside the
block — outside, x shouldn't be visible.

Distinguish eager (local(), test_that()) from lazy (reactive(),
rlang::on_load()) — a lazy body is scanned as if it runs after the rest of
the enclosing scope, so it can see names bound anywhere in that scope, not
just before it:

f <- function() {
  reactive({ y })  # sees `y` even though it's bound below
  y <- 1
}

1.2 Attach — library() / require() (#1340)

library(shiny)
reactive({ x <- 1 })  # `reactive` now resolves as shiny's NSE function

Try shadowing it:

local(library <- identity)
library(dplyr)   # still attaches — the shadow was local to `local()`

library <- identity
library(dplyr)   # does NOT attach — `library` is shadowed here

1.3 Source — source() (#1341)

source("helpers.R")
helper_fn()   # should resolve to a definition in helpers.R, with goto-def working

source() should be shadowable and importable like any other effect:

source <- identity
source("helpers.R")  # no longer sources anything

1.4 Assign — assign(), delayedAssign(), %<>%, %<~%, := (#1342, #1348)

assign("x", 1)
x   # should resolve, goto-def should land on the `assign()` call


# These should not define new bindings because the corresponding packages are
# not attached
x %<>% identity()
x %<~% 1
x := 1

# After attaching, references should resolve to the special assignments

library(magrittr)
x %<>% identity()
x

library(rlang)
x %<~% 1
x

library(S7)
x := 1

Magrittr's %<>% is compound — x %<>% f() means x <- x %>% f() — so its
left operand is read as well as bound. %<~% and := only bind.

library(magrittr)
y <- 1
y %<>% identity()
y

Goto-definition on the y in the %<>% line should land on y <- 1 rather
than on itself, since the read happens before the rebinding. Find-references
from y <- 1 should list that site too, and the trailing y should still
resolve to the %<>% binding.

Contrast with a write-only operator, whose left operand is a binding target and
nothing else:

library(S7)
z <- 1
z := 1   # goto-def on this `z` lands on itself, not on `z <- 1`

A %<>% read that resolves to nothing (no prior binding of that name) produces
no diagnostic. Nothing in section 2 covers unresolved names.

Rename: renaming x bound via assign("x", ...) should rename in place as
a string, keeping the quotes (assign("y", ...)), not unquote it.

Each site is spelled for where it sits, so a non-syntactic target lands bare
inside the string and backticked at the identifier:

assign("foo", 1)
foo

Renaming to foo bar should give assign("foo bar", 1) and `foo bar`.
Typing the backticks yourself (`foo bar`) should produce exactly the same
edits — the backticks are R syntax, not part of the name, so they must not end
up inside the string. Reserved words (if, TRUE) should be refused outright,
with no edits applied anywhere.

assign()/delayedAssign() with envir = or pos = should be treated as
not binding locally (can't state where the binding lands), so no bogus
local definition should appear.

1.5 Quote / unquote — quote(), bquote(), .()/..() holes (#1346, #1347)

quote({ x <- 1; library(foo); print(x) })

Nothing inside a fully quoted expression should produce uses, bindings, or
effects — no goto-def targets, no NSE recognition of the nested library().

bquote() unquote holes are the exception — code inside .() is live:

bquote(foo(.(foo)))

Here the outer foo(...) name is inert (quoted), but the .(foo) argument is
a real use of foo — goto-def/find-refs should see it.

1.6 substitute() (#1373)

foo <- function() {}
foobaz <- 1

function(arg) {
  obj <- 1
  substitute(foo(arg, obj, foobaz))
}

arg and obj (bound in the current function) should resolve normally.
foo and foobaz (from an outer scope) should read as quoted/inert — no
goto-def into them from inside the substitute() call.

1.7 evalq() (#1372) and rlang::on_load() (from #1339, refined in oak-nse/17)

x <- 1
on_load({ x <- 2 })   # deferred binding, routed to the enclosing scope
x                      # sees both `x <- 1` and `x <- 2`; doesn't get shadowed

Also check that on_load() bodies are now scanned in full — an earlier
version only recognized bare assignments inside them. This should now work
too:

local({
  f <- function() x
  rlang::on_load({ assign("x", 1) })       # or delayedAssign("x", 1)
})

f's reference to x should resolve to the on_load() binding inside
local(), not fall through to file scope.

1.8 on.exit() / defer() (#1377)

Same scoping model as rlang::on_load() — same tests apply, just swap in
on.exit({...}) or withr::defer({...}).

In particular, references in on.exit() should see forward assignments (since it's run at the end of the scope).

1.9 library(pkg, character.only = TRUE) (#1350)

library("foo", character.only = TRUE)   # recognized: literal string
x <- "foo"
library(x, character.only = TRUE)       # NOT yet recognized: needs dataflow

1.10 Import-layer shadowing (#1344)

library(stats)  # both stats and dplyr export `filter()`
library(dplyr)
filter(df, x > 1)   # resolves to dplyr's `filter` — last attached wins

filter <- function(x) x
filter(df, x > 1)   # resolves locally, not to either package

Same idea for source()/collation: a name from a file sourced later, or a
collation file later in the package, should shadow the same name from
earlier.

1.11 Attach scope — branches, loops, local() (oak-nse/20)

An attach applies only where it's known to have run. A library() inside a
branch covers that arm and stops at its closing brace:

if (cond) {
  library(shiny)
  reactive()   # shiny's
}

reactive       # nothing says the branch ran

Goto-definition on the last reactive should find no definition at all, while
the one inside the arm lands in shiny.

Attach another package unconditionally and it falls through to that one rather
than to nothing:

library(fakereactive)   # also exports `reactive`

if (cond) {
  library(shiny)
  reactive()   # shiny's
}

reactive       # fakereactive's

The sibling arm doesn't see the conditional attach either:

if (cond) {
  library(cli)
  taken        # sees cli
} else {
  other        # does not
}

An attach on both arms does survive, since either path attaches it:

if (cond) library(cli) else library(cli)
after          # sees cli

Each arm's own call still covers only that arm, so a use earlier in the second
arm doesn't see the first arm's attach:

if (cond) {
  library(cli)
} else {
  before       # does not see cli
  library(cli)
}
after          # sees cli

An else if chain follows the same rule. Every arm has to attach for the
package to survive the chain, and no arm sees another's attach.

A loop body may run zero times, so its attaches don't survive the loop:

for (i in xs) {
  library(cli)
  inside       # sees cli
}
after          # does not

local() runs at its call site, so it takes part in this sequencing instead
of being treated as deferred like a function body. A cursor inside sees only
what has run by that point, and what the block attaches is visible after it:

local({
  inside       # does not see cli, the `library()` below hasn't run yet
})
library(cli)

local({ library(cli) })
after          # sees cli

test_that() is eager too, so it behaves the same way.

A library() in a function body stays invisible outside it, since the body
may never run. Inside that body it does sequence: whenever the function runs,
a library() above a call has run before that call, so the attach applies
there and nowhere else.

f <- function() {
  library(shiny)
  reactive({ x <- 1 })   # shiny's
}

h <- function() {
  reactive({ y <- 1 })   # not shiny's
}

reactive({ z <- 1 })     # not shiny's

Check the two negatives as well as the positive: x should get its own scope
inside reactive(), while y and z should stay in h and at file scope.

Ordering inside the body cuts both ways, and a cursor on the library() call
is itself still before the attach — the call hasn't returned there:

g <- function() {
  before           # not shiny's
  library(shiny)   # cursor anywhere in this call: also not shiny's
  after            # shiny's
}

Worth trying with the call spread over several lines, since every offset inside
it should behave the same:

library(
  shiny
)

A local() block inside a body counts as that body's own attach, because
running the body runs the block:

g <- function() {
  local({ library(cli) })
  inside           # sees cli
}

Known gap (top-level attach after the call that runs the body). A body is
treated as running after the whole file has loaded, so a top-level library()
counts wherever it sits — even when the only call to that body happens first:

f <- function() {
  cli_alert("x")   # reads as cli's
}

f()
library(cli)

Knowing that f() ran before the attach needs call analysis, the same
machinery the next gap wants.

Known gap (effects of a called function). Calling the function doesn't
apply what it does:

g <- function() library(shiny)
g()
reactive({ x <- 1 })     # not shiny's

Effects come from the callee's annotation and a locally defined function
carries none, so g() contributes nothing. Inferring a function's effects
from its body and applying them at call sites isn't implemented, so anything
that reaches an effect through a user-defined wrapper reads as plain.

2. Diagnostics (experimental, off by default)

Enable with the LSP setting oak.diagnostics.experimental.enable.

2.1 Ambiguous effect

Three triggers, one diagnostic (ambiguous-effect, severity Info):

• Lazy-crossed shadow: an NSE reading inside a lazy context (function,
  reactive()) could be invalidated by a later/sibling assignment with
  undetermined timing relative to the call.

  f <- function() local({ x <- 1 })
  local <- identity

• Conditional shadow: a conditional local binding could shadow the callee
  on some path.

  if (cond) {
    local <- identity
  }
  local({ y <- 1 })

• Conditional attach: a library() attached on only some paths means the
  callee would have been effectful on that path, but reads as plain here.

  if (cond) library(shiny)
  reactive({ x <- 1 })

  The attach also stops applying past the branch (1.11), so the diagnostic and
  the resolution agree: both treat reactive here as plain.

Each diagnostic's secondary location (related_information in the LSP
response) should point at the competing site — the shadowing assignment, or
the conditional library() call.

2.2 Ambiguous attach order

Both arms attach the same packages in different orders
(ambiguous-attach-order, severity Info), the shape from 1.11:

if (cond) {
  library(cli)
  library(rlang)
} else {
  library(rlang)
  library(cli)
}

The diagnostic covers the whole if and carries no secondary location, since
both competing sites are already inside it.

It fires even though cli and rlang share no names today, because a later
package upgrade could introduce a collision with no source change. Attaching
in the same order on both arms is silent.

2.3 Uninstalled package

library(some.package.that.is.not.installed)

Should produce an uninstalled-package diagnostic (severity Warning) at the
library() call. Also explains why an ambiguity diagnostic might silently
not fire elsewhere in the file: if a package can't be resolved, its NSE
annotations are unknown, so there's nothing to flag as ambiguous.

[thomasp85]

All provided tests works as advertised