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

crates/ts_goport/src/effect/PORTING.md

@@ -77,6 +77,12 @@ and "Types" sections and follow them. This file adds the Effect rules.
   nil and `[]` change the compare after a round trip (the override lists and
   `skipLeadingPath`), the port keeps them apart as Go
   (`configraw::clone_effect_options`, `etscore::SlicePtr`).
+- Effect-TS/tsgo#828 (first released in 0.52.0) is ported ahead of the pin
+  (issue #52): the delete guard of `TypeParser::get_symbol_at_location`, and
+  `stability_api_usage.rs` now calls it. See "TS2790 with the stability
+  rules". The rest of #828 (lint tooling, CI, moving the other raw checker
+  calls) is not ported: those calls take type names, declaration names,
+  heritage expressions and module specifiers, which are never a delete target.
 
 ## Go shim -> Rust
 
@@ -239,8 +245,8 @@ the 3 rules on, `--pretty false` and `--pretty true`), and the fixtures
 With `unstableApiUsage` or `experimentalApiUsage` on (the default), a
 `delete` of a property that an index signature gives, under
 `exactOptionalPropertyTypes`, gets TS2790 ("The operand of a 'delete'
-operator must be optional"). effect-tsgo 0.51.1 prints it too, and plain
-TypeScript-Go does not. On T3 Code `apps/server` these are the 14 TS2790
+operator must be optional") in effect-tsgo 0.51.1. Plain TypeScript-Go does
+not report it. On T3 Code `apps/server` these are the 14 TS2790
 lines (`delete process.env.X`). The cause, in rule order:
 
 1. The two usage rules come before `floatingEffect` in Go `rules.All`
@@ -259,9 +265,22 @@ lines (`delete process.env.X`). The cause, in rule order:
    TS2790: with `exactOptionalPropertyTypes` only the `Optional` flag counts.
 
 The first check of the file found no symbol there, so plain TypeScript-Go and
-a run with the two rules off report nothing. The port makes the same calls in
-the same order (`rules/stability_api_usage.rs`, `rules/floating_effect.rs`),
-so it prints the same lines. A small case: `declare const env: { [key:
+a run with the two rules off report nothing.
+
+The port takes the fix of Effect-TS/tsgo#828, so here it prints what 0.52.0
+prints, not 0.51.1: `TypeParser::get_symbol_at_location` returns nil for a
+delete target (the access or its property name, also through parentheses and
+`?.`), and the usage rules call it, so the checker stores no symbol. The fixtures
+`delete-index-signature`, `delete-process-env` and `delete-required-property`
+hold the 0.51.1 output as their `ref`. A small case: `declare const env: { [key:
 string]: string | undefined }; delete env.FOO;` with `strict`,
-`exactOptionalPropertyTypes` and the plugin. `scripts/effect/t3-rules-off.sh`
-runs T3 Code with the rules on and off against effect-tsgo 0.51.1.
+`exactOptionalPropertyTypes` and the plugin.
+
+The guard has one more effect, as in 0.52.0: the usage rules find no symbol on
+a deleted name, so `delete cfg.beta` no longer gets TS377135 or TS377136 when
+`beta` is tagged. `cfg.beta` elsewhere still does, and so does a tagged receiver
+(`delete box.a` on a tagged `box`).
+
+`scripts/effect/t3-rules-off.sh` runs T3 Code with the rules on and off against
+effect-tsgo 0.51.1, which still prints these lines, so its rules-on runs now
+report DIFF on them.

crates/ts_goport/src/effect/rules/stability_api_usage.rs

@@ -129,7 +129,7 @@ fn visit(
                 }
             }
         }
-        let symbol = ctx.tp.checker.get_symbol_at_location_exported(node);
+        let symbol = ctx.tp.get_symbol_at_location(node);
         let resolved_symbol = ctx.tp.reference_symbol_at_node(node);
         let use_symbol = selected_declaration.is_nil()
             || !symbol_has_declaration(ctx.tp.checker, symbol, selected_declaration)

crates/ts_goport/src/effect/typeparser/helpers.rs

@@ -66,8 +66,8 @@ impl TypeParser<'_> {
         self.checker.get_type_of_property_of_type_exported(t, name)
     }
 
-    /// GetSymbolAtLocation wraps checker.GetSymbolAtLocation with a meta-property
-    /// guard. Meta properties (import.meta, import.defer, new.target) never
+    /// GetSymbolAtLocation guards meta properties and delete targets.
+    /// Meta properties (import.meta, import.defer, new.target) never
     /// reference a symbol rules care about, and the checker debug-asserts (panics)
     /// when asked for `import.defer` used as an import-call callee. Always use
     /// this instead of the raw checker call.
@@ -78,6 +78,19 @@ impl TypeParser<'_> {
         if node.kind() == SyntaxKind::MetaProperty {
             return SymbolId::NIL;
         }
+        // Symbol lookup can cache an index symbol that makes a later delete check
+        // incorrectly require an optional property.
+        let mut target = node;
+        let parent = target.parent();
+        if parent.is_some() && is_property_access_expression(parent) && parent.name() == target {
+            target = parent;
+        }
+        if is_access_expression(target) {
+            let parent = walk_up_parenthesized_expressions(target.parent());
+            if parent.is_some() && parent.kind() == SyntaxKind::DeleteExpression {
+                return SymbolId::NIL;
+            }
+        }
         self.checker.get_symbol_at_location_exported(node)
     }
 

crates/ts_goport/tests/effect_fixtures/delete-index-signature/expect.json

@@ -0,0 +1,7 @@
+{
+  "fails": false,
+  "has": [],
+  "lacks": ["TS2790"],
+  "ref": { "fails": true, "has": ["src/index.ts(2,8): error TS2790"], "lacks": [] },
+  "refNote": "effect-tsgo 0.51.1 reports these lines (Effect-TS/tsgo#820). The fix is Effect-TS/tsgo#828, first released in 0.52.0, and tsc-rs ports it ahead of its pin (PORTING.md)."
+}

crates/ts_goport/tests/effect_fixtures/delete-index-signature/src/index.ts

@@ -0,0 +1,21 @@
+declare const env: { [key: string]: string | undefined };
+delete env.FOO;
+delete env["BAR"];
+delete (env.PAREN);
+delete (((env.PARENS)));
+delete env?.OPTIONAL;
+delete (env?.OPTIONAL_PAREN);
+
+declare const numbers: { [key: string]: number };
+delete numbers.x;
+
+declare const nested: { inner: { [key: string]: string | undefined } };
+delete nested.inner.x;
+
+declare const optional: { a?: string };
+delete optional.a;
+
+declare const el: HTMLElement;
+delete el.dataset.x;
+
+export const value = delete env.USED;

crates/ts_goport/tests/effect_fixtures/delete-index-signature/tsconfig.json

@@ -0,0 +1,10 @@
+{
+  "compilerOptions": {
+    "strict": true, "noEmit": true, "module": "NodeNext", "moduleResolution": "NodeNext", "target": "ESNext", "skipLibCheck": true,
+    "lib": ["ESNext", "DOM"],
+    "types": [],
+    "exactOptionalPropertyTypes": true,
+    "plugins": [{ "name": "@effect/language-service" }]
+  },
+  "include": ["src"]
+}

crates/ts_goport/tests/effect_fixtures/delete-process-env/expect.json

@@ -0,0 +1,7 @@
+{
+  "fails": false,
+  "has": [],
+  "lacks": ["TS2790"],
+  "ref": { "fails": true, "has": ["src/index.ts(1,8): error TS2790"], "lacks": [] },
+  "refNote": "effect-tsgo 0.51.1 reports these lines (Effect-TS/tsgo#820). The fix is Effect-TS/tsgo#828, first released in 0.52.0, and tsc-rs ports it ahead of its pin (PORTING.md)."
+}

crates/ts_goport/tests/effect_fixtures/delete-process-env/src/index.ts

@@ -0,0 +1,6 @@
+delete process.env.HOME;
+
+const environment: NodeJS.ProcessEnv = { ...process.env };
+delete environment.GIT_DIR;
+
+export {};

crates/ts_goport/tests/effect_fixtures/delete-process-env/tsconfig.json

@@ -0,0 +1,9 @@
+{
+  "compilerOptions": {
+    "strict": true, "noEmit": true, "module": "NodeNext", "moduleResolution": "NodeNext", "target": "ESNext", "skipLibCheck": true,
+    "types": ["node"],
+    "exactOptionalPropertyTypes": true,
+    "plugins": [{ "name": "@effect/language-service" }]
+  },
+  "include": ["src"]
+}

crates/ts_goport/tests/effect_fixtures/delete-required-property/expect.json

@@ -0,0 +1,10 @@
+{
+  "fails": true,
+  "has": [
+    "src/index.ts(2,8): error TS2790",
+    "src/index.ts(3,9): error TS2790",
+    "src/index.ts(4,8): error TS2790",
+    "src/index.ts(8,8): error TS2704"
+  ],
+  "lacks": ["src/index.ts(5,"]
+}

crates/ts_goport/tests/effect_fixtures/delete-required-property/src/index.ts

@@ -0,0 +1,10 @@
+declare const required: { a: string; b: string; c: string; d?: string };
+delete required.a;
+delete (required.b);
+delete required?.c;
+delete required.d;
+
+declare const readonlyOptional: { readonly a?: string };
+delete readonlyOptional.a;
+
+export {};

crates/ts_goport/tests/effect_fixtures/delete-required-property/tsconfig.json

@@ -0,0 +1,9 @@
+{
+  "compilerOptions": {
+    "strict": true, "noEmit": true, "module": "NodeNext", "moduleResolution": "NodeNext", "target": "ESNext", "skipLibCheck": true,
+    "types": [],
+    "exactOptionalPropertyTypes": true,
+    "plugins": [{ "name": "@effect/language-service" }]
+  },
+  "include": ["src"]
+}

crates/ts_goport/tests/tsgo_effect_delete.rs

@@ -0,0 +1,51 @@
+//! Issue #52: with the Effect plugin and `exactOptionalPropertyTypes`, `delete`
+//! of a property from an index signature got a false TS2790. Effect-TS/tsgo#828
+//! fixes it in `TypeParser.GetSymbolAtLocation`. Each test checks a fixture of
+//! `tests/effect_fixtures`, which `scripts/effect/focused-fixtures.mjs` also
+//! runs against the reference. These two need no packages, so they run here too.
+
+use std::path::Path;
+use std::process::Command;
+
+/// Runs `tsgo -p tsconfig.json --pretty false` in the fixture dir (each fixture
+/// has `noEmit`, so nothing is written) and returns success and the stdout lines.
+fn tsc(fixture: &str) -> (bool, Vec<String>) {
+    let dir = Path::new(env!("CARGO_MANIFEST_DIR"))
+        .join("tests/effect_fixtures")
+        .join(fixture);
+    let out = Command::new(env!("CARGO_BIN_EXE_tsgo"))
+        .args(["-p", "tsconfig.json", "--pretty", "false"])
+        .current_dir(dir)
+        .output()
+        .unwrap();
+    let lines = String::from_utf8_lossy(&out.stdout)
+        .lines()
+        .map(str::to_string)
+        .collect();
+    (out.status.success(), lines)
+}
+
+#[test]
+fn index_signature_delete_is_clean() {
+    let (success, lines) = tsc("delete-index-signature");
+    assert_eq!(lines, Vec::<String>::new());
+    assert!(success);
+}
+
+#[test]
+fn required_and_readonly_delete_still_report() {
+    let optional = "error TS2790: The operand of a 'delete' operator must be optional.";
+    let readonly =
+        "error TS2704: The operand of a 'delete' operator cannot be a read-only property.";
+    let (success, lines) = tsc("delete-required-property");
+    assert_eq!(
+        lines,
+        [
+            format!("src/index.ts(2,8): {optional}"),
+            format!("src/index.ts(3,9): {optional}"),
+            format!("src/index.ts(4,8): {optional}"),
+            format!("src/index.ts(8,8): {readonly}"),
+        ]
+    );
+    assert!(!success);
+}