Skip to content

fix(effect): no false TS2790 on delete of index-signature properties - #151

Open
IAmJSD wants to merge 1 commit into
pingdotgg:mainfrom
IAmJSD:fix/effect-delete-index-signature
Open

IAmJSD wants to merge 1 commit into
pingdotgg:mainfrom
IAmJSD:fix/effect-delete-index-signature

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #52. Upstream: Effect-TS/tsgo#820, fixed by Effect-TS/tsgo#828 (merge 7d051dd5, first released in @effect/tsgo@0.52.0).

Problem

With the Effect plugin and exactOptionalPropertyTypes, delete env.FOO on an index signature reports a false TS2790. The stability usage rules look up the symbol of each identifier, which saves an index-signature symbol on the node. floatingEffect then asks for the type of the delete expression, which reruns the delete check against that symbol.

Confirmed on main (91f3d021c) before the change.

What is ported

Only the behaviour fix from #828, ahead of the 0.51.1 pin:

  • typeparser/helpers.rs: get_symbol_at_location returns nil for a delete target (the access or its property name, including through parentheses and ?.).
  • rules/stability_api_usage.rs: calls that guarded lookup instead of the raw checker call.
  • PORTING.md: records the difference from 0.51.1 and updates the TS2790 section.
  • Three fixtures, plus tests/tsgo_effect_delete.rs so that CI runs the two that need no packages. focused-fixtures.mjs does not run in CI.

What is left out

  • The custom lint tooling, CI changes and changeset.
  • The other 14 commits between the pin and #828.
  • The Go test files, as PORTING.md says to skip _test.go. The fixtures and the comparison below cover those cases.
  • Moving the other raw checker calls behind the guard (api_stability_safety, api_stability_leak, pipeable_signature_shape). They take type names, declaration names, heritage expressions and module specifiers, which are never a delete target.

For maintainers to decide

  1. Lost warnings on deleted names. The guard also removes TS377135 and TS377136 on a deleted name, so delete cfg.beta no longer warns when beta is tagged. cfg.beta elsewhere still warns, and so does a tagged receiver. This matches 0.52.0, so I kept it. Please confirm you want it.
  2. Recording a cherry-pick. PORTING.md has no convention for a fix taken ahead of the pin. I added a bullet to "Where the port differs from the reference", as open PR fix(effect): rule type queries read the types the check recorded (#112) #113 does, and left the stated pin at 0.51.1. Please say if you want something else.
  3. t3-rules-off.sh. Its rules-on runs now report DIFF against 0.51.1, which still prints these lines. I did not change the script.
  4. PR fix(effect): rule type queries read the types the check recorded (#112) #113. It ports a related upstream fix to the rules' type queries. The two PRs edit different parts of PORTING.md, but I have not tested them together.

Checks

Built with Rust 1.95.0 on darwin-arm64. The reference is the npm @effect/tsgo-darwin-arm64 package.

Check main This branch
Repro from #52 TS2790 at (2,8) clean
reference-cases.mjs (0.51.1) 541/542 541/542, same mismatch
go-unit-cases.mjs (0.51.1) 501/501 501/501
incremental-check.mjs (0.51.1) pass pass
focused-fixtures.mjs, effect 4.0.0-rc.118 new delete-index-signature and delete-process-env fail all pass, 2 skipped (need effect 4.0.0)
focused-fixtures.mjs, effect 4.0.0 not run all 22 pass
effect_cases, tsgo_effect_delete not run 1,060 and 2 pass
cargo fmt --check, clippy -D warnings not run pass
T3 Code v0.0.45, TS2790 in apps/server / apps/web 24 / 3 0 / 0, no other line changed

The reference-cases mismatch is missingReturnYieldStarFnUntraced: exit code 2 against 1, with the same output as on main.

Further comparisons:

  • 58 delete cases (29 sources, exactOptionalPropertyTypes on and off): this branch's output is identical to 0.52.0, and its non-Effect lines equal a run with the plugin off.
  • Against main on those cases: 29 TS2790 and 2 TS2704 are gone and nothing is added. The TS2704 is the same bug, on a readonly index signature.
  • Genuine errors still report: TS2790 on a required property and TS2704 on a readonly optional property, both with the plugin on.
  • delete process.env.X with @types/node and delete el.dataset.x: clean.
  • Language server: a hover on the deleted name followed by a diagnostics pull is clean.

Not run

  • cargo test --workspace and go_baselines: the local workspace run was stopped partway by memory pressure on my machine. It had logged no failures, but it is not a pass.
  • The goport protected set (goport tests, gate, bound runs, LSP and API oracles, np-suite): it needs hosts a fork cannot use.
  • incremental-check.mjs --plain, t3-parity.mjs and t3-rules-off.sh: for T3 Code I compared main and this branch directly instead.
  • scripts/run-cargo-capped.sh: it needs systemd-run, so I used plain cargo with CI's toolchain and flags.

🤖 Generated with Claude Code

Port the behaviour fix of Effect-TS/tsgo#828 ahead of the 0.51.1 pin:
TypeParser::get_symbol_at_location returns nil for a delete target, and
the stability usage rules call it instead of the raw checker lookup. The
checker then saves no index-signature symbol for a later delete check.

Fixes pingdotgg#52.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b44d0a77-57d9-4d80-8ffe-78b437ded348

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0.3.0 regression: TS2790 on delete of index-signature properties (ProcessEnv, DOMStringMap); 0.2.0 and tsc 7.1.0-dev report none

1 participant