From 11003f4b7281706e062bdc956f8d95da98987cf9 Mon Sep 17 00:00:00 2001 From: Joao Gomes Date: Thu, 29 Oct 2026 19:32:39 -0300 Subject: [PATCH] fix(effect): rule type queries read the types the check recorded (#112) With the Effect plugin, floatingEffect asked the checker for the type of a call statement after the file's check. The checker's quick path for a call (get_quick_type_of_expression) runs check_non_null_expression on the callee, which added a TS18048 or TS2532 next to the TS2722 that tsc reports. Port Effect-TS/tsgo#860 (first released in @effect/tsgo@0.53.2) ahead of the 0.51.1 pin: exit_check_expression records the final expression types of the file's check (patch 033), after_check_source_file pauses recording while the rules run, and get_type_at_location reads the record first. PORTING.md lists the difference from 0.51.1. Co-Authored-By: Claude Opus 5.5 --- crates/ts_goport/src/checker/checker_p09.rs | 5 + crates/ts_goport/src/effect/PORTING.md | 9 ++ crates/ts_goport/src/effect/mod.rs | 8 +- .../typeparser/checked_expression_types.rs | 89 ++++++++++++++++++ .../effect/typeparser/get_type_at_location.rs | 4 + crates/ts_goport/src/effect/typeparser/mod.rs | 5 + .../ts_goport/tests/tsgo_effect_type_query.rs | 94 +++++++++++++++++++ 7 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 crates/ts_goport/src/effect/typeparser/checked_expression_types.rs create mode 100644 crates/ts_goport/tests/tsgo_effect_type_query.rs diff --git a/crates/ts_goport/src/checker/checker_p09.rs b/crates/ts_goport/src/checker/checker_p09.rs index 7cd650b50..dacd2e83d 100644 --- a/crates/ts_goport/src/checker/checker_p09.rs +++ b/crates/ts_goport/src/checker/checker_p09.rs @@ -74,6 +74,11 @@ impl Checker { self.check_const_enum_access(node, t); } self.current_node = saved.save_current_node; + // Effect-TS/tsgo#860 patch 033 `ExpressionCheckedCallback`, registered by the + // plugin. Kept only when the program has Effect diagnostics options, as patch 004. + if self.compiler_options.effect.is_some() { + crate::effect::typeparser::record_checked_expression_type(self, node, check_mode, t); + } drop(saved); t } diff --git a/crates/ts_goport/src/effect/PORTING.md b/crates/ts_goport/src/effect/PORTING.md index 69aedf950..ac3329ec5 100644 --- a/crates/ts_goport/src/effect/PORTING.md +++ b/crates/ts_goport/src/effect/PORTING.md @@ -43,6 +43,15 @@ and "Types" sections and follow them. This file adds the Effect rules. `noUnusedParameters` it adds only suggestions (Go `reportUnused`), so `tsc` output does not change, and a later suggestion request gets the TS6133 that plain tsgo gives. +- Effect-TS/tsgo#860 (first released in 0.53.2) is ported ahead of the pin + (issue #112). `exit_check_expression` records each final expression type + of the file's check (Go patch 033 `ExpressionCheckedCallback`, only with + Effect options, as patch 004), `after_check_source_file` pauses recording + while the rules run, and `get_type_at_location` reads the record first + (`typeparser/checked_expression_types.rs`). In 0.51.1 a rule's type query + of a call statement took the checker's quick path + (`get_quick_type_of_expression`), whose `check_non_null_expression` added a + TS18048 or TS2532 next to the TS2722 that `tsc` reports. - A standalone API process runs no rules unless `TSGO_EFFECT_API=1` (`rulerunner::enabled_options`). - Patch 031 (`stability-tag-fast-path`) adds a scanner token flag and a diff --git a/crates/ts_goport/src/effect/mod.rs b/crates/ts_goport/src/effect/mod.rs index 897ea1950..f3abe56dc 100644 --- a/crates/ts_goport/src/effect/mod.rs +++ b/crates/ts_goport/src/effect/mod.rs @@ -60,7 +60,9 @@ pub fn after_check_source_file(ctx: &Context, c: &mut Checker, source_file: Node }; let min = rulerunner::min_visible_severity(Some(&effect_config)); let program = c.program; - let Ok(diagnostics) = rulerunner::run( + // Rule queries must not overwrite the types the check recorded. + let pause = typeparser::pause_checked_expression_types(c); + let result = rulerunner::run( ctx, program, c, @@ -68,7 +70,9 @@ pub fn after_check_source_file(ctx: &Context, c: &mut Checker, source_file: Node Some(&effect_config), None, min, - ) else { + ); + pause.resume(c); + let Ok(diagnostics) = result else { return; }; for diag in diagnostics { diff --git a/crates/ts_goport/src/effect/typeparser/checked_expression_types.rs b/crates/ts_goport/src/effect/typeparser/checked_expression_types.rs new file mode 100644 index 000000000..d536598dc --- /dev/null +++ b/crates/ts_goport/src/effect/typeparser/checked_expression_types.rs @@ -0,0 +1,89 @@ +// Go: internal/typeparser/checked_expression_types.go +// PORT: Effect-TS/tsgo#860 (first released in @effect/tsgo@0.53.2), ported +// ahead of the 0.51.1 pin. See effect/PORTING.md. + +use crate::effect::typeparser::checker_integration::{ + get_resolved_type_from_type_node, is_checking_source_file, is_source_file_type_checked, +}; +use crate::effect::typeparser::*; +use crate::prelude::*; + +/// RecordCheckedExpressionType stores the type the checker computed for an +/// expression while checking a source file. Only final evaluations count: plain +/// Normal mode, or Contextual alone when a call argument is checked against its +/// parameter. Inference passes and type-only queries are tentative. The checker +/// may evaluate a node more than once, and the last final evaluation is the one +/// its diagnostics are based on, so later writes win. Rule runs pause recording, +/// and evaluations outside a source-file check are ignored, so only the check +/// itself writes the store. +pub fn record_checked_expression_type( + c: &mut Checker, + node: Node, + check_mode: CheckMode, + t: TypeId, +) { + if !check_mode.without(CheckMode::CONTEXTUAL).is_empty() || !is_checking_source_file(c) { + return; + } + let links = effect_links_of(c); + if links.checked_expression_types_paused { + return; + } + *links.checked_expression_type.get(node) = CachedId::new(t.0); +} + +/// PauseCheckedExpressionTypes stops recording for the checker until the +/// returned guard is dropped (Go returns the resume function, which the caller +/// defers). +pub fn pause_checked_expression_types(c: &mut Checker) -> CheckedExpressionTypesPause { + effect_links_of(c).checked_expression_types_paused = true; + CheckedExpressionTypesPause { _private: () } +} + +/// The resume function of `pause_checked_expression_types`. PORT: the guard +/// holds no borrow of the checker, so the caller resumes with `resume`. +#[must_use] +pub struct CheckedExpressionTypesPause { + _private: (), +} + +impl CheckedExpressionTypesPause { + pub fn resume(self, c: &mut Checker) { + effect_links_of(c).checked_expression_types_paused = false; + } +} + +impl TypeParser<'_> { + /// checkedExpressionType returns the type the file's check computed for an + /// expression, without evaluating anything, or nil when the check never + /// evaluated it. + pub fn checked_expression_type(&mut self, mut node: Node) -> TypeId { + if !is_expression_node(node) || is_part_of_type_node(node) { + return TypeId::NIL; + } + if is_right_side_of_qualified_name_or_property_access(node) { + node = node.parent(); + } + if !is_source_file_type_checked(self.checker, get_source_file_of_node(node)) { + return TypeId::NIL; + } + let mut t = TypeId::NIL; + if let Some(recorded) = self.links().checked_expression_type.try_get(node) { + t = TypeId(recorded.get()); + } + if t.is_nil() { + // Initializers and return expressions first evaluated while recording + // was paused are only in the checker's own cache. + t = get_resolved_type_from_type_node(self.checker, node); + } + if t.is_nil() { + return TypeId::NIL; + } + self.checker.get_regular_type_of_literal_type(t) + } +} + +// Go: internal/typeparser/type_parser.go effectLinksOf +pub fn effect_links_of(c: &mut Checker) -> &mut EffectLinks { + c.effect_links.get_or_insert_with(Box::default) +} diff --git a/crates/ts_goport/src/effect/typeparser/get_type_at_location.rs b/crates/ts_goport/src/effect/typeparser/get_type_at_location.rs index c760f1f9f..d41babd80 100644 --- a/crates/ts_goport/src/effect/typeparser/get_type_at_location.rs +++ b/crates/ts_goport/src/effect/typeparser/get_type_at_location.rs @@ -77,6 +77,10 @@ impl TypeParser<'_> { return TypeId::NIL; } + let t = self.checked_expression_type(node); + if t.is_some() { + return t; + } let c = &mut *self.checker; // Go: defer func() { if r := recover(); r != nil { result = nil } }() go_recover(|| c.get_type_at_location(node)).unwrap_or(TypeId::NIL) diff --git a/crates/ts_goport/src/effect/typeparser/mod.rs b/crates/ts_goport/src/effect/typeparser/mod.rs index 8dcf2cca6..433b72340 100644 --- a/crates/ts_goport/src/effect/typeparser/mod.rs +++ b/crates/ts_goport/src/effect/typeparser/mod.rs @@ -14,6 +14,7 @@ mod api_stability_p2; mod api_stability_p3; mod api_stability_safety; mod cause_type; +mod checked_expression_types; pub mod checker_integration; mod constant_evaluation; mod context_tag; @@ -78,6 +79,7 @@ pub use api_stability_p2::*; pub use api_stability_p3::*; pub use api_stability_safety::*; pub use cause_type::*; +pub use checked_expression_types::*; pub use constant_evaluation::*; pub use context_tag::*; pub use context_type::*; @@ -214,6 +216,9 @@ pub struct EffectLinks { pub execution_flow: FxHashMap>>, pub effect_context_flags: LinkStore, pub effect_yield_generator_function: LinkStore, + // Effect-TS/tsgo#860: the expression types the file's check recorded. + pub checked_expression_type: LinkStore, + pub checked_expression_types_paused: bool, pub discover_packages_computed: bool, pub discover_packages_value: Vec, diff --git a/crates/ts_goport/tests/tsgo_effect_type_query.rs b/crates/ts_goport/tests/tsgo_effect_type_query.rs new file mode 100644 index 000000000..8377dc91e --- /dev/null +++ b/crates/ts_goport/tests/tsgo_effect_type_query.rs @@ -0,0 +1,94 @@ +//! Issue #112: with the Effect plugin, a statement that calls a possibly +//! undefined function got a TS18048 or TS2532 next to the TS2722 of `tsc`. +//! The `floatingEffect` rule asked the checker for the statement's type +//! after the check, and the quick type of a call reports them +//! (`check_non_null_expression`). Effect-TS/tsgo#860 reads the types the check +//! recorded first. Each test runs `tsgo -p` in a new scratch dir and expects +//! the output of `tsc` (`typescript@7.1.0-beta`). A passing test deletes its +//! dir. + +use std::fs; +use std::path::PathBuf; +use std::process::Command; + +/// Writes `files` to a new scratch dir, runs +/// `tsgo -p tsconfig.json --pretty false` there and returns the output +/// (stdout, then stderr) and the dir. +fn tsc(name: &str, files: &[(&str, &str)]) -> (String, PathBuf) { + let dir = std::env::temp_dir().join(format!( + "tsgo_effect_type_query_{}_{name}", + std::process::id() + )); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + for (file, text) in files { + fs::write(dir.join(file), text).unwrap(); + } + let out = Command::new(env!("CARGO_BIN_EXE_tsgo")) + .args(["-p", "tsconfig.json", "--pretty", "false"]) + .current_dir(&dir) + .output() + .unwrap(); + let mut text = String::from_utf8_lossy(&out.stdout).into_owned(); + text.push_str(&String::from_utf8_lossy(&out.stderr)); + (text, dir) +} + +fn config(plugin: &str) -> String { + format!( + r#"{{ + "compilerOptions": {{ + "strict": true, + "noUncheckedIndexedAccess": true, + "noEmit": true, + "types": [], + "plugins": [{plugin}] + }}, + "files": ["a.ts"] +}}"# + ) +} + +/// The issue's repro, an awaited call, and an initializer, where `tsc` itself +/// reports TS18048 next to TS2722 (the check takes the quick type there). +const A_TS: &str = r"declare const f: (() => void) | undefined; +f(); + +declare const fs: Array<() => void>; +fs[0](); + +declare const g: (() => Promise) | undefined; +export async function h() { + await g(); +} + +const r = f(); +void r; + +export {}; +"; + +/// `tsc` output for `A_TS`. +const A_TS_TSC: &str = "\ +a.ts(2,1): error TS2722: Cannot invoke an object which is possibly 'undefined'. +a.ts(5,1): error TS2722: Cannot invoke an object which is possibly 'undefined'. +a.ts(9,9): error TS2722: Cannot invoke an object which is possibly 'undefined'. +a.ts(12,11): error TS2722: Cannot invoke an object which is possibly 'undefined'. +a.ts(12,11): error TS18048: 'f' is possibly 'undefined'. +"; + +fn check(name: &str, plugin: &str) { + let (output, dir) = tsc(name, &[("tsconfig.json", &config(plugin)), ("a.ts", A_TS)]); + assert_eq!(output, A_TS_TSC, "case dir {}", dir.display()); + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn floating_effect_type_query_adds_no_diagnostics() { + check("plugin", r#"{ "name": "@effect/language-service" }"#); +} + +#[test] +fn plain_output_without_plugin() { + check("plain", ""); +}