diff --git a/crates/ts_goport/src/effect/PORTING.md b/crates/ts_goport/src/effect/PORTING.md index 69aedf950..e58adc05b 100644 --- a/crates/ts_goport/src/effect/PORTING.md +++ b/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. diff --git a/crates/ts_goport/src/effect/rules/stability_api_usage.rs b/crates/ts_goport/src/effect/rules/stability_api_usage.rs index e71a0afc6..9bb2b14f3 100644 --- a/crates/ts_goport/src/effect/rules/stability_api_usage.rs +++ b/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) diff --git a/crates/ts_goport/src/effect/typeparser/helpers.rs b/crates/ts_goport/src/effect/typeparser/helpers.rs index 6de4fd58f..ace4fded6 100644 --- a/crates/ts_goport/src/effect/typeparser/helpers.rs +++ b/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) } diff --git a/crates/ts_goport/tests/effect_fixtures/delete-index-signature/expect.json b/crates/ts_goport/tests/effect_fixtures/delete-index-signature/expect.json new file mode 100644 index 000000000..7e283eab8 --- /dev/null +++ b/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)." +} diff --git a/crates/ts_goport/tests/effect_fixtures/delete-index-signature/src/index.ts b/crates/ts_goport/tests/effect_fixtures/delete-index-signature/src/index.ts new file mode 100644 index 000000000..08729d2b1 --- /dev/null +++ b/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; diff --git a/crates/ts_goport/tests/effect_fixtures/delete-index-signature/tsconfig.json b/crates/ts_goport/tests/effect_fixtures/delete-index-signature/tsconfig.json new file mode 100644 index 000000000..01d9cd057 --- /dev/null +++ b/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"] +} diff --git a/crates/ts_goport/tests/effect_fixtures/delete-process-env/expect.json b/crates/ts_goport/tests/effect_fixtures/delete-process-env/expect.json new file mode 100644 index 000000000..9dae411c7 --- /dev/null +++ b/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)." +} diff --git a/crates/ts_goport/tests/effect_fixtures/delete-process-env/src/index.ts b/crates/ts_goport/tests/effect_fixtures/delete-process-env/src/index.ts new file mode 100644 index 000000000..b6f368453 --- /dev/null +++ b/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 {}; diff --git a/crates/ts_goport/tests/effect_fixtures/delete-process-env/tsconfig.json b/crates/ts_goport/tests/effect_fixtures/delete-process-env/tsconfig.json new file mode 100644 index 000000000..00e3d7da5 --- /dev/null +++ b/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"] +} diff --git a/crates/ts_goport/tests/effect_fixtures/delete-required-property/expect.json b/crates/ts_goport/tests/effect_fixtures/delete-required-property/expect.json new file mode 100644 index 000000000..334f7be28 --- /dev/null +++ b/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,"] +} diff --git a/crates/ts_goport/tests/effect_fixtures/delete-required-property/src/index.ts b/crates/ts_goport/tests/effect_fixtures/delete-required-property/src/index.ts new file mode 100644 index 000000000..cca250a77 --- /dev/null +++ b/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 {}; diff --git a/crates/ts_goport/tests/effect_fixtures/delete-required-property/tsconfig.json b/crates/ts_goport/tests/effect_fixtures/delete-required-property/tsconfig.json new file mode 100644 index 000000000..db48f4198 --- /dev/null +++ b/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"] +} diff --git a/crates/ts_goport/tests/tsgo_effect_delete.rs b/crates/ts_goport/tests/tsgo_effect_delete.rs new file mode 100644 index 000000000..297062316 --- /dev/null +++ b/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) { + 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::::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); +}