fix(extract): scope this.* binding keys per enclosing class

crates/core/tests/integration_test.rs

@@ -219,6 +219,8 @@ mod issue_1640_commit_and_tag_version_updater;
 mod issue_1785_typed_property_hop;
 #[path = "integration_test/issue_1788_local_class_options.rs"]
 mod issue_1788_local_class_options;
+#[path = "integration_test/issue_1821_per_class_this_scoping.rs"]
+mod issue_1821_per_class_this_scoping;
 #[path = "integration_test/issue_1821_private_field_di.rs"]
 mod issue_1821_private_field_di;
 #[path = "integration_test/issue_346_static_factory_method.rs"]

crates/core/tests/integration_test/issue_1821_per_class_this_scoping.rs

@@ -0,0 +1,53 @@
+use super::common::{create_config, fixture_path};
+
+fn unused_member_names(root: std::path::PathBuf) -> Vec<String> {
+    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");
+    results
+        .unused_class_members
+        .iter()
+        .map(|m| format!("{}.{}", m.member.parent_name, m.member.member_name))
+        .collect()
+}
+
+#[test]
+fn per_class_this_scoping_credits_both_colliding_fields_cross_module() {
+    // Issue #1821 (Fix B): two classes in ONE consumer file both name their
+    // receiver field `dep` (`constructor(private dep: DepPrivParam)` and
+    // `readonly dep = new DepPubField()`), and a second pair collides on a
+    // `#`-private `#dep`. Before per-class scoping the module-flat `this.dep` /
+    // `this.#dep` binding key collided (last-write-wins), so only one class of
+    // each pair credited its dep's members and the other's real member was
+    // falsely reported as `unused-class-member` cross-module. Consumers live in
+    // a separate file from the dep classes so the file-level self-access map
+    // cannot mask the bug. Every dep class also carries a genuinely-dead control
+    // member that must STAY flagged, proving the fix credits precisely per class
+    // and never over-credits.
+    let unused = unused_member_names(fixture_path("issue-1821-per-class-this-scoping"));
+
+    for credited in [
+        "DepPrivParam.privParam",
+        "DepPubField.pubField",
+        "DepHashA.hashA",
+        "DepHashB.hashB",
+    ] {
+        assert!(
+            !unused.contains(&credited.to_string()),
+            "{credited} is reached through a same-named `dep` / `#dep` receiver on a \
+             sibling class and must be credited per class (issue #1821 Fix B), found: {unused:?}"
+        );
+    }
+    for control in [
+        "DepPrivParam.deadOnPrivParam",
+        "DepPubField.deadOnPubField",
+        "DepHashA.deadOnHashA",
+        "DepHashB.deadOnHashB",
+    ] {
+        assert!(
+            unused.contains(&control.to_string()),
+            "{control} has no call site and must stay flagged (per-class crediting must \
+             not blanket-credit), found: {unused:?}"
+        );
+    }
+}

crates/extract/src/cache/types.rs

@@ -791,7 +791,14 @@ use crate::MemberKind;
 /// iteration variable to the element class, so `for...of` / `.map` / `.forEach`
 /// member accesses on the item credit the class. Warm 232 caches lack the added
 /// `member_accesses`, leaving those methods falsely reported as unused.
-pub(super) const CACHE_VERSION: u32 = 233;
+///
+/// Bumped to 234 for issue #1821 (Fix B): `this.<field>` binding keys are now
+/// scoped per enclosing class during extraction, so two classes in one module
+/// that declare a same-named field (e.g. `constructor(private dep: DepA)` and
+/// `readonly dep = new DepB()`) no longer collide via last-write-wins. The
+/// resulting `member_accesses` change for such modules (the previously-losing
+/// class's members are now credited); warm 233 caches keep the collision.
+pub(super) const CACHE_VERSION: u32 = 234;
 
 /// Duplication token cache version. Bump when duplicate tokenization,
 /// normalization, or the on-disk token cache schema changes.

crates/extract/src/visitor/mod.rs

@@ -281,6 +281,19 @@ pub(crate) struct ModuleInfoExtractor {
     /// `toString` coercion for the quasi itself.
     in_tagged_template_quasi: bool,
     pub(crate) class_super_stack: Vec<Option<String>>,
+    /// Monotonic per-module class-scope id source: incremented each time a
+    /// class body is entered so every class gets a unique id, paired with
+    /// `class_scope_stack`. A `this.<field>` binding key is qualified with the
+    /// enclosing class's id (`this@<id>.<field>`) during the walk so two
+    /// classes declaring a same-named field do not collide in the module-flat
+    /// `binding_target_names` map; the qualifier is stripped back to `this.`
+    /// before ModuleInfo emission. See issue #1821 (Fix B).
+    class_scope_counter: u32,
+    /// Stack of active class-scope ids. The top is the enclosing class of the
+    /// current walk position; `this.<field>` keys and receiver spellings are
+    /// qualified with it. Empty at module scope, so module-level `this` stays
+    /// unqualified (behavior unchanged). See issue #1821 (Fix B).
+    class_scope_stack: Vec<u32>,
     pub(crate) inline_template_findings: Vec<InlineTemplateFinding>,
     pub(crate) side_effect_registered_class_names: FxHashSet<String>,
     lit_custom_element_candidates: Vec<LitCustomElementCandidate>,
@@ -743,6 +756,56 @@ impl ModuleInfoExtractor {
             .collect()
     }
 
+    /// Build the class-scoped `binding_target_names` key for a `this.<suffix>`
+    /// member. Inside a class body the key is qualified with the enclosing
+    /// class's scope id (`this@<id>.<suffix>`) so two classes in one module that
+    /// declare a same-named field do not collide in the module-flat map: without
+    /// this, last-write-wins credits only the class declared last and falsely
+    /// reports the other class's members unused. Module-level `this` (no active
+    /// class scope) keeps the plain `this.<suffix>` spelling. The qualifier is
+    /// stripped back to `this.` by `strip_this_scope_qualifiers` before any
+    /// spelling reaches `ModuleInfo`. See issue #1821 (Fix B).
+    fn this_member_key(&self, suffix: &str) -> String {
+        match self.class_scope_stack.last() {
+            Some(id) => format!("this@{id}.{suffix}"),
+            None => format!("this.{suffix}"),
+        }
+    }
+
+    /// Qualify a `this.`-rooted access / whole-object / iteration-receiver
+    /// spelling with the enclosing class scope id so it resolves against the
+    /// same class's qualified `binding_target_names` keys (issue #1821). A no-op
+    /// for the bare `this` object (single segment, so the per-file self-access
+    /// credit keyed on `object == "this"` is untouched), any non-`this`
+    /// spelling, an already-qualified `this@<id>.` spelling, and module-level
+    /// `this` (no active class scope). Paired with `strip_this_scope_qualifiers`
+    /// at emission.
+    fn qualify_this_scope(&self, spelling: &str) -> String {
+        if let Some(id) = self.class_scope_stack.last()
+            && let Some(rest) = spelling.strip_prefix("this.")
+        {
+            return format!("this@{id}.{rest}");
+        }
+        spelling.to_string()
+    }
+
+    /// Rewrite every internal `this@<id>.` scope qualifier (issue #1821) back to
+    /// a plain `this.` across the emitted `member_accesses` and
+    /// `whole_object_uses`, so no persisted spelling and no downstream consumer
+    /// (core member self-access `== "this"`, heritage `!= "this"`,
+    /// `unused_component_output` `this.<name>`, SFC template `starts_with("this.")`)
+    /// ever sees the qualifier. Called last in `finalize_resolution_phase`, after
+    /// every resolution pass that relies on the per-class keys, so the strip is
+    /// invariant across the `into_module_info` and `merge_into` (SFC) paths.
+    fn strip_this_scope_qualifiers(&mut self) {
+        for access in &mut self.member_accesses {
+            strip_this_scope_qualifier(&mut access.object);
+        }
+        for whole in &mut self.whole_object_uses {
+            strip_this_scope_qualifier(whole);
+        }
+    }
+
     fn insert_class_binding_target(&mut self, binding: String, target: String) {
         self.binding_target_names
             .insert(binding, BindingTarget::Class(target));
@@ -2101,7 +2164,12 @@ impl ModuleInfoExtractor {
         self.map_local_signature_refs_to_exports();
         self.apply_side_effect_registrations();
         self.resolve_typed_react_props();
-        self.collect_namespace_object_aliases()
+        let namespace_object_aliases = self.collect_namespace_object_aliases();
+        // Last: every resolution pass above relies on the per-class `this@<id>.`
+        // keys, so the qualifier is stripped only once they have run, before any
+        // spelling is emitted into `ModuleInfo`. See issue #1821 (Fix B).
+        self.strip_this_scope_qualifiers();
+        namespace_object_aliases
     }
 
     pub(crate) fn into_module_info(
@@ -2350,6 +2418,26 @@ pub(super) fn extract_destructured_names(obj_pat: &ObjectPattern<'_>) -> Vec<Str
         .collect()
 }
 
+/// Strip a leading `this@<id>` scope qualifier back to `this` (issue #1821),
+/// leaving any non-`this@` spelling untouched. `@` cannot appear in a JS
+/// identifier or dotted member path, so the marker is unambiguous, and it is
+/// only ever produced for multi-segment `this.<path>` spellings, so a `.`
+/// always follows the id. The `else` arm is defensive and unreachable in
+/// practice.
+fn strip_this_scope_qualifier(spelling: &mut String) {
+    let Some(rest) = spelling.strip_prefix("this@") else {
+        return;
+    };
+    if let Some(dot) = rest.find('.') {
+        let mut rebuilt = String::with_capacity("this".len() + rest.len() - dot);
+        rebuilt.push_str("this");
+        rebuilt.push_str(&rest[dot..]);
+        *spelling = rebuilt;
+    } else {
+        *spelling = "this".to_string();
+    }
+}
+
 fn try_extract_require<'a, 'b>(
     init: &'b Expression<'a>,
 ) -> Option<(&'b CallExpression<'a>, &'b str)> {

crates/extract/src/visitor/tests.rs

@@ -4307,6 +4307,155 @@ fn private_field_without_member_call_records_no_access() {
     );
 }
 
+#[test]
+fn per_class_this_scoping_credits_both_colliding_public_fields() {
+    // Issue #1821 (Fix B): two classes in one module both name their field
+    // `dep`. Before per-class scoping the module-flat `this.dep` binding key
+    // collided (last-write-wins), so only the class declared last credited its
+    // dep's members. Now BOTH `DepA.aMethod` and `DepB.bMethod` are credited.
+    let info = parse(
+        r"
+            import { DepA, DepB } from './dep';
+            class ConsumerA {
+                constructor(private dep: DepA) {}
+                run() { return this.dep.aMethod(); }
+            }
+            class ConsumerB {
+                readonly dep = new DepB();
+                run() { return this.dep.bMethod(); }
+            }
+        ",
+    );
+    assert!(
+        info.member_accesses
+            .iter()
+            .any(|a| a.object == "DepA" && a.member == "aMethod"),
+        "param-property field should credit DepA.aMethod despite the same-named \
+         `dep` field on ConsumerB, found: {:?}",
+        info.member_accesses
+    );
+    assert!(
+        info.member_accesses
+            .iter()
+            .any(|a| a.object == "DepB" && a.member == "bMethod"),
+        "public field should credit DepB.bMethod despite the same-named `dep` \
+         field on ConsumerA, found: {:?}",
+        info.member_accesses
+    );
+}
+
+#[test]
+fn per_class_this_scoping_is_declaration_order_independent() {
+    // Issue #1821 (Fix B): reversing the class declaration order must not flip
+    // which class's members are credited (the tell that exposed the collision).
+    let info = parse(
+        r"
+            import { DepA, DepB } from './dep';
+            class ConsumerB {
+                readonly dep = new DepB();
+                run() { return this.dep.bMethod(); }
+            }
+            class ConsumerA {
+                constructor(private dep: DepA) {}
+                run() { return this.dep.aMethod(); }
+            }
+        ",
+    );
+    assert!(
+        info.member_accesses
+            .iter()
+            .any(|a| a.object == "DepA" && a.member == "aMethod"),
+        "DepA.aMethod must be credited regardless of declaration order, found: {:?}",
+        info.member_accesses
+    );
+    assert!(
+        info.member_accesses
+            .iter()
+            .any(|a| a.object == "DepB" && a.member == "bMethod"),
+        "DepB.bMethod must be credited regardless of declaration order, found: {:?}",
+        info.member_accesses
+    );
+}
+
+#[test]
+fn per_class_this_scoping_credits_both_colliding_private_fields() {
+    // Issue #1821 (Fix B): the same collision through `#`-private fields (the
+    // Fix A receiver shape). Both classes name the field `#dep`; both dep
+    // classes' members must be credited.
+    let info = parse(
+        r"
+            import { DepA, DepB } from './dep';
+            class ConsumerA {
+                readonly #dep = new DepA();
+                run() { return this.#dep.aMethod(); }
+            }
+            class ConsumerB {
+                readonly #dep = new DepB();
+                run() { return this.#dep.bMethod(); }
+            }
+        ",
+    );
+    assert!(
+        info.member_accesses
+            .iter()
+            .any(|a| a.object == "DepA" && a.member == "aMethod"),
+        "private-field DepA.aMethod must be credited despite the same-named \
+         `#dep` on ConsumerB, found: {:?}",
+        info.member_accesses
+    );
+    assert!(
+        info.member_accesses
+            .iter()
+            .any(|a| a.object == "DepB" && a.member == "bMethod"),
+        "private-field DepB.bMethod must be credited despite the same-named \
+         `#dep` on ConsumerA, found: {:?}",
+        info.member_accesses
+    );
+}
+
+#[test]
+fn per_class_this_scoping_strips_internal_qualifier_before_emission() {
+    // Issue #1821 (Fix B): the internal `this@<id>.` per-class qualifier is an
+    // extraction-only disambiguator and must never reach an emitted spelling.
+    // Assert no emitted `member_accesses` or `whole_object_uses` object carries
+    // it, and that the plain `this.dep` receiver spelling survives.
+    let info = parse(
+        r"
+            import { DepA, DepB } from './dep';
+            class ConsumerA {
+                constructor(private dep: DepA) {}
+                run() { return this.dep.aMethod(); }
+            }
+            class ConsumerB {
+                readonly dep = new DepB();
+                run() {
+                    Object.keys(this.dep);
+                    return this.dep.bMethod();
+                }
+            }
+        ",
+    );
+    assert!(
+        info.member_accesses.iter().all(|a| !a.object.contains('@')),
+        "no emitted member-access object may carry the internal per-class \
+         qualifier, found: {:?}",
+        info.member_accesses
+    );
+    assert!(
+        info.whole_object_uses.iter().all(|w| !w.contains('@')),
+        "no emitted whole-object use may carry the internal per-class \
+         qualifier, found: {:?}",
+        info.whole_object_uses
+    );
+    assert!(
+        info.member_accesses
+            .iter()
+            .any(|a| a.object == "this.dep" && a.member == "bMethod"),
+        "the plain `this.dep` receiver spelling must survive the strip, found: {:?}",
+        info.member_accesses
+    );
+}
+
 #[test]
 fn module_exports_object_extracts_keys() {
     let info = parse("module.exports = { foo: 1, bar: 2 };");