diff --git a/.claude/rules/detection.md b/.claude/rules/detection.md index 0ae0489c7..1bf084662 100644 --- a/.claude/rules/detection.md +++ b/.claude/rules/detection.md @@ -80,7 +80,7 @@ Non-obvious implementation details for each detection feature. These are NOT dis - **Constructor-rooted fluent chains and `this` return types** (issue #953): `new Class().a().b().done()` credits `b` and `done` only when every prior chain step is self-returning. `extract_class_members` treats both explicit same-class return annotations (`a(): Class`) and TypeScript polymorphic `this` return annotations (`a(): this`) as self-returning, matching the compiler contract even when the method body returns through a helper such as `return this.setIfProvided(...)`. A bare body heuristic still recognizes an unannotated final `return this`. Terminal methods such as `build()` do not need to be self-returning themselves, but all intermediate methods before them do. Bumps the extract `CACHE_VERSION` because the cached `is_self_returning` flags change. - **Typed instance chains** (issues #386, #388): `ClassHeritageInfo.instance_bindings` are extracted for all classes, not only Angular-decorated classes. The binding set includes non-private typed constructor parameters, typed properties, nested object-type properties, and typed getters, so a non-Angular chain like `factory.service.query()` credits `Service.query` when `factory` is a `Factory` instance and `Factory.service` is declared as `Service`. Reflective whole-object uses such as `Object.keys(factory.service)` propagate through the same typed chain and mark the target class as whole-object used. This covers getter-backed Playwright fixture teardown factories without suppressing genuinely unused service methods. **Generic-constraint substitution** (#388): on `visit_class` enter, the visitor pushes a `>` map collected via `collect_class_type_param_constraints` onto `class_type_param_constraints` (popped on exit). `record_typed_binding` consults the top of the stack: a constrained parameter substitutes its constraint (`class BaseService` with `constructor(client: TClient)` registers `this.client -> BaseClient`); an unconstrained parameter drops the binding (no resolvable class). The substitution flows through `extract_nested_type_bindings` so nested object-type properties typed as a generic parameter also resolve correctly. `extract_class_instance_bindings` applies the same substitution when populating `ClassHeritageInfo.instance_bindings`. - **Typed destructure bindings** (issue #752): a destructured `let`/`const` binding or destructured function parameter that carries a type annotation populates `binding_target_names` so subsequent `.` accesses resolve onto the referenced class. Covers the Svelte typed-prop shape `let { resultState }: Props = $props()` where `interface Props { resultState: ResultState }`, the inline-literal form `let { resultState }: { resultState: ResultState } = ...`, object type-alias references, rename forms (`let { resultState: rs }: Props = ...` binds `rs`), and destructured params (`function render({ resultState }: Props) { resultState.pin() }`). Interface / object-type-alias property types are collected at module scope (`collect_object_type_property_types`) and resolved in the `resolve_typed_destructure_bindings` finalize step so the type may be declared after the binding (interfaces hoist); inline literals resolve in place during the walk. Only bare `TSTypeReference`-to-single-name property types are recorded (`extract_type_annotation_name`); union / array / generic-wrapped property types, nested destructures with dotted keys, and rest elements do not resolve (conservative, no false negatives in the over-crediting direction). The finalize step runs first in `into_module_info` / `merge_into` and is also called in `sfc.rs::merge_script_into_module` before the template-visible `binding_target_names()` read so the Svelte/Vue template scanner credits markup member access (`{#if resultState.labelMessage}`, `bind:value={resultState.labelInput}`, `onclick={() => resultState.pin(...)}`). Resolution is type-driven, so it is framework-agnostic (plain TS, Vue, Svelte). Cross-file crediting works through type-only imports because `build_local_to_export_keys` includes resolved type-only imports. Bumps the extract `CACHE_VERSION` (the resulting `member_accesses` change). See issue #752. -- **Typed-property-hop member credit** (issue #1785): a dotted member chain whose receiver's type hop goes through a NAMED interface or type-literal alias (`interface Opts { c: OptDep }`, `constructor(private opts: Opts)`, `this.opts.c.optM()`) credits the terminal property type's class member, same-file and cross-module. Two cooperating mechanisms. **Part A (extract-local)**: `resolve_bound_member_accesses` expands a compound binding target (`Opts.c`, produced by `resolve_bound_object_name`'s longest-prefix match) through the file's own `interface_property_types` map (`expand_typed_property_compound` in `crates/extract/src/visitor/mod.rs`), one path segment per hop (terminates by segment count, so self-referential types cannot loop), rewriting the access onto the terminal local name (`OptDep`), which the analyze layer's `build_local_to_export_keys` already resolves through imports (incl. type-only) cross-module. A hop through a locally-declared CLASS continues via that class's own typed-property bindings (`local_class_exports[..].instance_bindings`, issue #1788), so an UNEXPORTED options class (`class Opts { constructor(public c: ImportedDep) {} }`, which never resolves through `local_to_export_keys`) credits its imported property types; exported classes keep the analyze-side `instance_bindings` path too (the extract-side credit is additive). A property that is not a named-reference-typed member (union / generic / unharvested / untyped class property) abstains. Whole-object uses (`Object.keys(this.opts.c)`) expand the same way (local hops only). **Part B (cross-module)**: the per-file interface/alias property types persist as `ModuleInfo.type_member_types` (`TypeMemberTypeEntry { type_name, property, property_type }`, names local to the declaring module, resolution deferred like `FactoryReturnExport.class_local_name`), and a hop whose type name is IMPORTED emits a `SemanticFact::TypedPropertyMemberAccess { type_name, property_path, member }` fact. The analyze join (`propagate_typed_property_accesses` in `unused_members.rs`) mirrors `propagate_factory_fn_accesses`'s chain of gates: consumer `local_to_export_keys` + `walk_re_export_origins` to the declaring module (which must actually declare the type in `type_member_types`, over-credit gate 1), per-segment map walk with imported mid-chain types re-resolving through their own module (depth bounded by segment count, levels deduped), terminal name resolved through the declaring module's own imports and gated on `export_is_class_with_members` (gate 2, reusing `credit_factory_return_class_member`). A wrong/unresolvable annotation is a false-negative no-op, never a false positive. Bumps the extract `CACHE_VERSION` (224 to 225: new persisted field + fact). Out of scope: generic type parameters, indexed-access / mapped / union property types, `declare module` augmentation. See issue #1785. +- **Typed-property-hop member credit** (issue #1785): a dotted member chain whose receiver's type hop goes through a NAMED interface or type-literal alias (`interface Opts { c: OptDep }`, `constructor(private opts: Opts)`, `this.opts.c.optM()`) credits the terminal property type's class member, same-file and cross-module. Two cooperating mechanisms. **Part A (extract-local)**: `resolve_bound_member_accesses` expands a compound binding target (`Opts.c`, produced by `resolve_bound_object_name`'s longest-prefix match) through the file's own `interface_property_types` map (`expand_typed_property_compound` in `crates/extract/src/visitor/mod.rs`), one path segment per hop (terminates by segment count, so self-referential types cannot loop), rewriting the access onto the terminal local name (`OptDep`), which the analyze layer's `build_local_to_export_keys` already resolves through imports (incl. type-only) cross-module. A hop through a locally-declared CLASS continues via that class's own typed-property bindings (`local_class_exports[..].instance_bindings`, issue #1788), so an UNEXPORTED options class (`class Opts { constructor(public c: ImportedDep) {} }`, which never resolves through `local_to_export_keys`) credits its imported property types; exported classes keep the analyze-side `instance_bindings` path too (the extract-side credit is additive). A property that is not a named-reference-typed member (union / generic / unharvested / untyped class property) abstains. Whole-object uses (`Object.keys(this.opts.c)`) expand the same way (local hops only). **Part B (cross-module)**: the per-file interface/alias property types persist as `ModuleInfo.type_member_types` (`TypeMemberTypeEntry { type_name, property, property_type }`, names local to the declaring module, resolution deferred like `FactoryReturnExport.class_local_name`), and a hop whose type name is IMPORTED emits a `SemanticFact::TypedPropertyMemberAccess { type_name, property_path, member }` fact. The analyze join (`propagate_typed_property_accesses` in `unused_members.rs`) mirrors `propagate_factory_fn_accesses`'s chain of gates: consumer `local_to_export_keys` + `walk_re_export_origins` to the declaring module (which must actually declare the type in `type_member_types`, over-credit gate 1), per-segment map walk with imported mid-chain types re-resolving through their own module (depth bounded by segment count, levels deduped), terminal name resolved through the declaring module's own imports and gated on `export_is_class_with_members` (gate 2, reusing `credit_factory_return_class_member`). **Interface terminal** (issue #1863, the ports-and-adapters / hexagonal DI shape `deps.greeter.greet()` where `deps: Deps`, `Deps.greeter: GreeterPort`, `class GreeterAdapter implements GreeterPort`): when the hop's terminal type is an INTERFACE rather than a class, gate 2 above drops it (an interface is not a class-with-members), so `propagate_typed_property_accesses` additionally resolves the terminal to its canonical export key (same `local_keys` + `export_key_with_origins` resolution the `interface_to_implementers` map is keyed by) and, when that key is an interface with `implements` implementers, inserts the member on the interface's export key. The typed-property pass runs inside `propagate_common_member_accesses`, BEFORE `propagate_interface_member_accesses`, so the interface access is carried to every implementing class by that later pass, making the property-hop terminal behave exactly like the direct-parameter case (`useIt(g: GreeterPort) { g.greet() }`, which already worked). Additive / false-negative-only: a terminal that is not an interface with implementers credits nothing, so it can only suppress a false positive, never create one, and the all-implementers over-credit matches the documented `interface_to_implementers` semantics. No `CACHE_VERSION` bump (analyze-only, reads existing `type_member_types` facts + `class_heritage.implements`). A wrong/unresolvable annotation is a false-negative no-op, never a false positive. Bumps the extract `CACHE_VERSION` (224 to 225: new persisted field + fact). Out of scope: generic type parameters, indexed-access / mapped / union property types, `declare module` augmentation, structural (non-`implements`) interface conformance. See issues #1785 and #1863. - **Object-literal factory-return member credit** (issue #1858): a factory function returning an inferred OBJECT LITERAL whose property values are class instances (`export function createUi() { const factory = new InvokerFactory(); return { orders: factory.ordersPage } }`), consumed cross-module or same-file as `const ui = createUi(); ui.orders.member()`, credits the property class's member. This is the general root cause behind the Playwright page-object-factory pattern (a factory hands back page objects wrapped in a plain object), NOT Playwright-specific. EXTRACTION (`crates/extract/src/visitor/visit_impl_factory_returns.rs::function_body_returns_object_shape`): a classifier captures the returned object literal's shape as `(dotted_property_path, value_ref)`, where each property value is `Class(name)` (a `new Class()` leaf) or `Path(str)` (a flattened identifier / member-expression leaf); nested object literals recurse with a dotted prefix (`invoke.orders`), spread / computed-key / dynamic-value props are skipped, and the returned literal is reached through a direct `return { ... }`, a `satisfies` / `as` / parenthesized wrapper, or an assigned-then-returned identifier (`const ui = {...}; return ui`). At FINALIZE time (`resolve_factory_return_object_shapes`, after `enrich_local_class_exports` and before `resolve_factory_return_candidates`) each value ref is resolved to a terminal LOCAL class name by REUSING `resolve_bound_object_name` + `expand_typed_property_compound` (the proven #1785/#1788 receiver-type resolution): a bare local bound to `new X()` resolves to `Class("X")` (no dot, taken directly, since piping a no-dot name through `expand_typed_property_compound` returns `Opaque`); a compound (`OrdersInvokerFactory.ordersPage`) hops through the class's typed-property binding (field or getter). A `CrossModule` (factory class in another file), opaque, or unresolvable value drops the property (conservative false negative). The resolved shape persists as `ModuleInfo.exported_factory_return_object_shapes` (strict / exported form, joined against exports like `exported_factory_returns`, honoring `export { x as y }`) plus a loose same-file map. CONSUMER (`resolve_factory_return_candidates`): for `const ui = createUi()` (a factory-return candidate) an access `ui..member` (segment-boundary strip on `ui.`, so `uiExtra` is not captured) emits a `SemanticFact::FactoryReturnObjectPropertyAccess { callee_name, property_path, member }` for an imported callee, or resolves against the same-file loose shape map directly. ANALYZE JOIN (`propagate_factory_return_object_accesses` in `members/factory.rs`) mirrors `propagate_factory_fn_accesses`: resolve the callee through `local_keys` + `walk_re_export_origins` to the factory export, match `property_path` in its `exported_factory_return_object_shapes`, then `credit_factory_return_class_member` resolves the class through the factory module's own imports and gates on `export_is_class_with_members`. The change is false-negative-only by construction (it only INSERTS credit); a wrong or unresolvable chain is a silent no-op, never a false positive. Bumps the extract `CACHE_VERSION` (234 to 235: new persisted field + fact). Out of scope (documented follow-ups): a factory CLASS defined in a different file than the factory function, a property value that is itself a factory CALL (`{ orders: makeOrders() }`), and an UNBOUND consumer call-chain (`createUi().orders.member()` with no `const ui =`; fallow's visitor only credits the first member off an unnamed factory result). Validated on the svelte benchmark: the analyze-result factory `{ root: scope_root }` consumed as `analysis.root.unique(...)` credits `ScopeRoot.unique` (a real false-positive removal), all other benchmarks byte-identical, zero over-credit. See issue #1858 (follow-up to #1785, #1788, #1744, #1441). - **Iteration-binding element-type member credit** (issue #1707 follow-up): an iteration variable whose type is the element class of a typed array / reactive array (`Util[]` / `computed(() => Util[])` etc., resolved by the same `infer_array_binding_element_type` + `array_binding_element_types` map as the Vue v-for fix) is bound to that element class so member accesses on the iteration variable credit the class instead of reporting `unused-class-member`. Two arms. **JS (framework-agnostic)**: `bind_iterable_callback_parameter` types the FIRST callback parameter of the element-first-param array methods (`map` / `forEach` / `filter` / `find` / `findLast` / `findIndex` / `findLastIndex` / `flatMap` / `some` / `every`; `reduce` / `reduceRight` are excluded because their first callback parameter is the accumulator, not an element), and `bind_for_of_element` types a `for (const util of utils)` bare-identifier loop variable. Both resolve the receiver's element class via `iterable_element_type_for` (the Angular query-list map first, then `array_binding_element_types`), for a bare-identifier module-scope receiver. This also covers React / Preact JSX `.map` (`{utils.map(u => {u.getter})}`) since those are ordinary visitor-parsed callbacks. Explicitly-typed callback params (`(u: Util) => ...`) already credit via `record_typed_binding`; the new code only adds the implicit-element-type case. **Svelte `{#each}`**: the Svelte template scanner types a `{#each utils as util}` item (mirroring the Vue v-for pre-scan + locals-exclusion): `augment_bound_targets_with_each` binds the bare-identifier item to its element class and it is dropped from the block locals so it remaps onto the class. The `index` alias and destructured items (`{ id }`) stay untyped. Over-credit only (never introduces a false positive: a module-wide name collision credits the wrong class's member, a false negative, never a false positive); the Svelte `{#each}` augmentation resolves collisions first-write-wins (`entry().or_insert()`) while the JS arm is last-write-wins (`insert_class_binding_target`), both over-credit-safe. Bumps the extract `CACHE_VERSION` (216 to 217). Vue `v-for` over a member-expression `props.` source is now covered (issue #1711): the `defineProps<{ items: Util[] }>()` inline-type harvest records each array-typed prop field's element class as `props.` into the same `array_binding_element_types` map, which the existing v-for scanner matches (only the inline-TS-literal `defineProps` form; the `.value` / `store.` sources stay deferred, issue #1716). Angular `@for` / `*ngFor` over a component-field array (`utils: Util[]`) in an INLINE `template:` is now covered (issue #1712): the Angular template scanner (`sfc_template/angular.rs`) types a bare-identifier loop var to the field's element class via `collect_component_field_array_types` + `infer_array_binding_element_type`, keeps the item out of the block locals, and remaps `util.member` -> `Util.member` in a finalize pass (external `templateUrl` templates remain deferred, issue #1717, since the HTML scanner has no access to the component's field types at extract time). Astro `.map` in the template `{...}` region is now covered (issue #1713): template expression regions are re-parsed and run through the same member-recording visitor pass (`extend_template_expression_member_accesses` in astro.rs, seeded with the frontmatter's `array_binding_element_types`), so a template `.map()` / `.forEach()` / `for...of` iteration binding credits the element class the same as the frontmatter. Deferred (still over-credit gaps, not false positives): function-local receivers for the JS arm (issue #1718), and Angular external `templateUrl` templates (issue #1717). See issues #1707, #1711, #1712, #1713, #1716, #1717, and #1718. - **`instanceof` narrowing member credit** (issue #845): an `if ( instanceof )` guard populates `binding_target_names` with ` -> ` so method calls on the narrowed local inside (or after) the guard body (`.()`) resolve onto the class via the same `resolve_bound_member_accesses` path as typed-destructure bindings. Recorded in `visit_if_statement` via `collect_instanceof_narrowings`, which walks the test condition, recurses through `&&`-chained `LogicalExpression`s and `ParenthesizedExpression`s, and collects only simple `Identifier instanceof Identifier` pairs (complex left-hand expressions and member-expression class operands are skipped conservatively). Insertion uses `entry(local).or_insert(class)`, so a pre-existing binding for the same local (a real instance binding, typed-destructure target, etc.) wins over the narrowing. Bindings are module-scoped rather than strictly block-scoped, which can credit a member outside the guard's true narrowed region: this over-credits in the false-negative direction only (a member that is genuinely unused but shares a local name with an instanceof guard could be missed), never the false-positive direction, matching fallow's conservative posture. Genuinely-unused members on the same class still report. Bumps the extract `CACHE_VERSION` (107 to 108) because the resulting `member_accesses` change, structurally identical to the #752 typed-destructure bump. See issue #845. diff --git a/CHANGELOG.md b/CHANGELOG.md index a98af7ebc..55a876cbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`unused-class-members` no longer false-flags a method dispatched through an interface-typed property (ports-and-adapters / hexagonal DI).** A method reached through a property whose declared type is an interface, on a class that `implements` that interface (`useIt(deps: Deps) { deps.greeter.greet() }` where `Deps.greeter: GreeterPort` and `class GreeterAdapter implements GreeterPort`), now credits `GreeterAdapter.greet`. The interface dispatch already worked through a direct parameter or variable (`useIt(g: GreeterPort) { g.greet() }`); this closes the remaining gap where the receiver is reached via an interface property hop, which dominates hexagonal-architecture findings. A genuinely-unused method on the implementing class still reports. Thanks [@lukeramsden](https://github.com/lukeramsden) for the clean minimal reduction. (Closes [#1863](https://github.com/fallow-rs/fallow/issues/1863)) - **`unused-class-members` no longer false-flags a method reached through a factory that returns an object literal.** A factory function returning an inferred object literal whose property values are class instances (`export function createUi() { const factory = new InvokerFactory(); return { orders: factory.ordersPage } }`), consumed cross-module as `const ui = createUi(); ui.orders.placeOrder()`, now credits `OrdersPage.placeOrder`. Every property-value shape resolves: a direct `new Class()`, a local `const` alias to one, and a member read of a separately-constructed instance (`factory.ordersPage`, whether a typed field or a getter). Nested object literals (`ui.invoke.dashboard.method()`), the assigned-then-returned form (`const ui = {...}; return ui`), and same-file consumption are all covered. A genuinely-unused method on the returned class still reports. This is the general root cause behind the Playwright page-object-factory pattern; it is not Playwright-specific. Thanks [@committedpazz](https://github.com/committedpazz) for the precise bisection. (Closes [#1858](https://github.com/fallow-rs/fallow/issues/1858)) - **Star re-exports no longer make a source module's default export appear diff --git a/crates/core/src/analyze/members/mod.rs b/crates/core/src/analyze/members/mod.rs index 8e5444433..451bb83c4 100644 --- a/crates/core/src/analyze/members/mod.rs +++ b/crates/core/src/analyze/members/mod.rs @@ -1157,6 +1157,7 @@ fn collect_propagated_member_accesses( propagate_common_member_accesses( input, indexes, + &heritage_context.interface_to_implementers, &mut accessed_members, &mut whole_object_used_exports, ); @@ -1191,6 +1192,7 @@ fn collect_propagated_member_accesses( fn propagate_common_member_accesses( input: UnusedMemberScanInput<'_>, indexes: &MemberPassIndexes<'_>, + interface_to_implementers: &FxHashMap>, accessed_members: &mut FxHashMap>, whole_object_used_exports: &mut FxHashSet, ) { @@ -1222,6 +1224,7 @@ fn propagate_common_member_accesses( input.graph, input.resolved_modules, indexes, + interface_to_implementers, accessed_members, ); propagate_fluent_chain_accesses( diff --git a/crates/core/src/analyze/members/typed_property.rs b/crates/core/src/analyze/members/typed_property.rs index 657fcb2eb..643467b44 100644 --- a/crates/core/src/analyze/members/typed_property.rs +++ b/crates/core/src/analyze/members/typed_property.rs @@ -66,13 +66,17 @@ pub(super) fn typed_property_declaring_sites<'a>( /// 3. the terminal property type resolves through the last declaring /// module's own imports/exports and must be a class with members /// (`export_is_class_with_members`, reused via -/// `credit_factory_return_class_member`). +/// `credit_factory_return_class_member`), OR (issue #1863) an INTERFACE +/// with implementers, in which case the member is credited on the +/// interface's export key so the later `propagate_interface_member_accesses` +/// pass carries it to every implementing class. /// -/// See issue #1785. +/// See issues #1785 and #1863. pub(super) fn propagate_typed_property_accesses( graph: &ModuleGraph, resolved_modules: &[ResolvedModule], indexes: &MemberPassIndexes<'_>, + interface_to_implementers: &FxHashMap>, accessed_members: &mut FxHashMap>, ) { // Phase 1: walk every fact's property path to its terminal @@ -143,5 +147,52 @@ pub(super) fn propagate_typed_property_accesses( terminal_name.as_str(), member.as_str(), ); + credit_typed_property_interface_terminal( + &mut credit_context, + interface_to_implementers, + declaring_file_id, + terminal_name.as_str(), + member.as_str(), + ); + } +} + +/// Credit a typed-property-hop TERMINAL whose type is an INTERFACE (issue #1863). +/// +/// `credit_factory_return_class_member` above resolves only class-with-members +/// terminals; an interface terminal (`deps.greeter: GreeterPort` where +/// `GreeterPort` is an interface) is filtered out by `export_is_class_with_members` +/// and would credit nothing. Resolving the interface to its canonical export key +/// (the same `local_keys` + `export_key_with_origins` resolution the +/// `interface_to_implementers` map is keyed by) and inserting the member there +/// makes the property-hop terminal behave exactly like the direct-parameter case: +/// the interface access is carried to every implementing class by the later +/// `propagate_interface_member_accesses` pass, which runs after this one (both +/// sit inside `collect_propagated_member_accesses`, the typed-property pass in +/// `propagate_common_member_accesses` first, then the interface pass). Purely +/// additive; a terminal that is not an interface with implementers credits +/// nothing, so it can only suppress a false positive, never create one. +fn credit_typed_property_interface_terminal( + context: &mut FactoryReturnCreditContext<'_, '_>, + interface_to_implementers: &FxHashMap>, + declaring_file_id: FileId, + terminal_name: &str, + member: &str, +) { + let graph = context.graph; + let indexes = context.indexes; + let Some(seed_keys) = indexes.local_keys(declaring_file_id).get(terminal_name) else { + return; + }; + for seed_key in seed_keys { + for interface_origin in export_key_with_origins(graph, seed_key) { + if interface_to_implementers.contains_key(&interface_origin) { + context + .accessed_members + .entry(interface_origin) + .or_default() + .insert(member.to_string()); + } + } } } diff --git a/crates/core/tests/integration_test.rs b/crates/core/tests/integration_test.rs index d11e23c9b..5dd73c3c3 100644 --- a/crates/core/tests/integration_test.rs +++ b/crates/core/tests/integration_test.rs @@ -225,6 +225,8 @@ mod issue_1821_per_class_this_scoping; mod issue_1821_private_field_di; #[path = "integration_test/issue_1858_object_literal_factory_return.rs"] mod issue_1858_object_literal_factory_return; +#[path = "integration_test/issue_1863_interface_property_dispatch.rs"] +mod issue_1863_interface_property_dispatch; #[path = "integration_test/issue_346_static_factory_method.rs"] mod issue_346_static_factory_method; #[path = "integration_test/issue_604_vite_rollup_path_helpers.rs"] diff --git a/crates/core/tests/integration_test/issue_1863_interface_property_dispatch.rs b/crates/core/tests/integration_test/issue_1863_interface_property_dispatch.rs new file mode 100644 index 000000000..5f4c51593 --- /dev/null +++ b/crates/core/tests/integration_test/issue_1863_interface_property_dispatch.rs @@ -0,0 +1,68 @@ +use super::common::{create_config, fixture_path}; + +#[test] +fn interface_typed_property_dispatch_credits_implementer_member() { + // Issue #1863: a member reached through a PROPERTY whose declared type is an + // interface (`deps.greeter.greet()` where `deps: Deps`, `Deps.greeter: + // GreeterPort`) must credit the member on every class that `implements` + // GreeterPort. The #1785 typed-property hop resolves the terminal to the + // interface, but the terminal credit only handled classes; routing an + // interface terminal through the existing interface->implementer propagation + // carries the member to `GreeterAdapter`, exactly as the direct-parameter + // case already works. A genuinely-dead method on the same class stays + // flagged (non-vacuous control). + let root = fixture_path("issue-1863-interface-property-dispatch"); + let mut config = create_config(root); + config.rules.unused_class_members = fallow_config::Severity::Error; + let results = fallow_core::analyze(&config).expect("analysis should succeed"); + + let unused: Vec = results + .unused_class_members + .iter() + .map(|m| format!("{}.{}", m.member.parent_name, m.member.member_name)) + .collect(); + + assert!( + !unused.contains(&"GreeterAdapter.greet".to_string()), + "GreeterAdapter.greet is reached via deps.greeter.greet() through the GreeterPort \ + interface property and must be credited (issue #1863), found: {unused:?}" + ); + assert!( + unused.contains(&"GreeterAdapter.deadOnAdapter".to_string()), + "GreeterAdapter.deadOnAdapter has no call site and must stay flagged, found: {unused:?}" + ); +} + +#[test] +fn interface_property_hop_credits_every_implementer_cross_file() { + // Issue #1863 (fan-out + cross-file): the port interface (`GreeterPort`), the + // interface holding the typed property (`Deps` in a separate file), and each + // implementing adapter all live in different files, the realistic hexagonal + // layout. `deps.greeter.greet()` must credit `greet` on EVERY class that + // implements `GreeterPort` (the documented interface->implementer over-credit + // direction), while a differently-named dead method on each adapter stays + // flagged. + let root = fixture_path("issue-1863-multi-implementer"); + let mut config = create_config(root); + config.rules.unused_class_members = fallow_config::Severity::Error; + let results = fallow_core::analyze(&config).expect("analysis should succeed"); + + let unused: Vec = results + .unused_class_members + .iter() + .map(|m| format!("{}.{}", m.member.parent_name, m.member.member_name)) + .collect(); + + assert!( + !unused.contains(&"AdapterA.greet".to_string()) + && !unused.contains(&"AdapterB.greet".to_string()), + "greet must be credited on both implementers of GreeterPort via the interface \ + property hop (issue #1863), found: {unused:?}" + ); + assert!( + unused.contains(&"AdapterA.deadOnA".to_string()) + && unused.contains(&"AdapterB.deadOnB".to_string()), + "the uncalled methods on each adapter must stay flagged (non-vacuous control), \ + found: {unused:?}" + ); +} diff --git a/tests/fixtures/issue-1863-interface-property-dispatch/package.json b/tests/fixtures/issue-1863-interface-property-dispatch/package.json new file mode 100644 index 000000000..ade767045 --- /dev/null +++ b/tests/fixtures/issue-1863-interface-property-dispatch/package.json @@ -0,0 +1,5 @@ +{ + "name": "issue-1863-interface-property-dispatch", + "private": true, + "main": "src/index.ts" +} diff --git a/tests/fixtures/issue-1863-interface-property-dispatch/src/adapter.ts b/tests/fixtures/issue-1863-interface-property-dispatch/src/adapter.ts new file mode 100644 index 000000000..c7102a53b --- /dev/null +++ b/tests/fixtures/issue-1863-interface-property-dispatch/src/adapter.ts @@ -0,0 +1,8 @@ +import type { GreeterPort } from './port'; + +export class GreeterAdapter implements GreeterPort { + greet(name: string): string { + return `hi ${name}`; + } + deadOnAdapter(): void {} +} diff --git a/tests/fixtures/issue-1863-interface-property-dispatch/src/consumer.ts b/tests/fixtures/issue-1863-interface-property-dispatch/src/consumer.ts new file mode 100644 index 000000000..4252f56c8 --- /dev/null +++ b/tests/fixtures/issue-1863-interface-property-dispatch/src/consumer.ts @@ -0,0 +1,5 @@ +import type { Deps } from './port'; + +export function useIt(deps: Deps): string { + return deps.greeter.greet('x'); +} diff --git a/tests/fixtures/issue-1863-interface-property-dispatch/src/index.ts b/tests/fixtures/issue-1863-interface-property-dispatch/src/index.ts new file mode 100644 index 000000000..33a3fe2cb --- /dev/null +++ b/tests/fixtures/issue-1863-interface-property-dispatch/src/index.ts @@ -0,0 +1,6 @@ +import { GreeterAdapter } from './adapter'; +import { useIt } from './consumer'; + +export function run(): string { + return useIt({ greeter: new GreeterAdapter() }); +} diff --git a/tests/fixtures/issue-1863-interface-property-dispatch/src/port.ts b/tests/fixtures/issue-1863-interface-property-dispatch/src/port.ts new file mode 100644 index 000000000..eed0ad696 --- /dev/null +++ b/tests/fixtures/issue-1863-interface-property-dispatch/src/port.ts @@ -0,0 +1,6 @@ +export interface GreeterPort { + greet(name: string): string; +} +export interface Deps { + greeter: GreeterPort; +} diff --git a/tests/fixtures/issue-1863-multi-implementer/package.json b/tests/fixtures/issue-1863-multi-implementer/package.json new file mode 100644 index 000000000..3c962c83f --- /dev/null +++ b/tests/fixtures/issue-1863-multi-implementer/package.json @@ -0,0 +1,5 @@ +{ + "name": "issue-1863-multi-implementer", + "private": true, + "main": "src/index.ts" +} diff --git a/tests/fixtures/issue-1863-multi-implementer/src/adapter-a.ts b/tests/fixtures/issue-1863-multi-implementer/src/adapter-a.ts new file mode 100644 index 000000000..b9edd7827 --- /dev/null +++ b/tests/fixtures/issue-1863-multi-implementer/src/adapter-a.ts @@ -0,0 +1,8 @@ +import type { GreeterPort } from './port'; + +export class AdapterA implements GreeterPort { + greet(name: string): string { + return `A ${name}`; + } + deadOnA(): void {} +} diff --git a/tests/fixtures/issue-1863-multi-implementer/src/adapter-b.ts b/tests/fixtures/issue-1863-multi-implementer/src/adapter-b.ts new file mode 100644 index 000000000..e6816f0f2 --- /dev/null +++ b/tests/fixtures/issue-1863-multi-implementer/src/adapter-b.ts @@ -0,0 +1,8 @@ +import type { GreeterPort } from './port'; + +export class AdapterB implements GreeterPort { + greet(name: string): string { + return `B ${name}`; + } + deadOnB(): void {} +} diff --git a/tests/fixtures/issue-1863-multi-implementer/src/consumer.ts b/tests/fixtures/issue-1863-multi-implementer/src/consumer.ts new file mode 100644 index 000000000..1e9299c2c --- /dev/null +++ b/tests/fixtures/issue-1863-multi-implementer/src/consumer.ts @@ -0,0 +1,5 @@ +import type { Deps } from './deps'; + +export function useIt(deps: Deps): string { + return deps.greeter.greet('x'); +} diff --git a/tests/fixtures/issue-1863-multi-implementer/src/deps.ts b/tests/fixtures/issue-1863-multi-implementer/src/deps.ts new file mode 100644 index 000000000..fc5f5e96a --- /dev/null +++ b/tests/fixtures/issue-1863-multi-implementer/src/deps.ts @@ -0,0 +1,5 @@ +import type { GreeterPort } from './port'; + +export interface Deps { + greeter: GreeterPort; +} diff --git a/tests/fixtures/issue-1863-multi-implementer/src/index.ts b/tests/fixtures/issue-1863-multi-implementer/src/index.ts new file mode 100644 index 000000000..0c4cfd95e --- /dev/null +++ b/tests/fixtures/issue-1863-multi-implementer/src/index.ts @@ -0,0 +1,7 @@ +import { AdapterA } from './adapter-a'; +import { AdapterB } from './adapter-b'; +import { useIt } from './consumer'; + +export function run(): string { + return useIt({ greeter: new AdapterA() }) + useIt({ greeter: new AdapterB() }); +} diff --git a/tests/fixtures/issue-1863-multi-implementer/src/port.ts b/tests/fixtures/issue-1863-multi-implementer/src/port.ts new file mode 100644 index 000000000..7ab8c7055 --- /dev/null +++ b/tests/fixtures/issue-1863-multi-implementer/src/port.ts @@ -0,0 +1,3 @@ +export interface GreeterPort { + greet(name: string): string; +}