fix(npm): emit npm/npa on member scopes, not every space

.bca-baseline.toml

@@ -1031,18 +1031,6 @@ qualified = "<file>"
 metric = "loc.ploc"
 value = 547.0
 
-[[entry]]
-path = "src/metrics/npa/csharp.rs"
-qualified = "CsharpCode::compute"
-metric = "cognitive"
-value = 15.0
-
-[[entry]]
-path = "src/metrics/npa/kotlin.rs"
-qualified = "KotlinCode::compute"
-metric = "cognitive"
-value = 15.0
-
 [[entry]]
 path = "src/metrics/npa/objc.rs"
 qualified = "ObjcCode::compute"
@@ -1053,7 +1041,7 @@ value = 15.0
 path = "src/metrics/npa/php.rs"
 qualified = "PhpCode::compute"
 metric = "halstead.effort"
-value = 52227.844233305164
+value = 51274.96613103099
 
 [[entry]]
 path = "src/metrics/npa/python.rs"

CHANGELOG.md

@@ -766,6 +766,41 @@ for historical reference.
 
 ### Fixed
 
+- **Serialized shape.** `npm` and `npa` no longer emit an all-zero block
+  on function spaces (#1197). They enabled themselves from
+  `Checker::is_func_space`, which answers "does this node open a space",
+  not "is this a scope that owns methods and attributes". C#, JavaScript,
+  MozJS, TypeScript, TSX, PHP and Ruby therefore carried a block on every
+  ordinary method, and #1184 extended that to Kotlin `get()` / `set()` /
+  `init { … }`, Java and Groovy `static { … }` and the JS-family
+  `class_static_block` — which is the inconsistency the issue reports: a
+  `<get>` space carrying OOP metrics while the `m` beside it did not.
+  Kotlin, Java, Groovy, JavaScript, MozJS, TypeScript, TSX, C#, PHP and
+  Ruby are affected; Python, Rust, C, C++, Mozcpp, Go, Objective-C and
+  Elixir gate on their own node kinds and do not move.
+
+  The rule is now `SpaceKind::is_member_scope`, which `wmc` already
+  followed and which the three metrics share as a single definition:
+  containers and the whole-file `unit` root carry the block, a function
+  space never does. **The file-root roll-up is retained** — an earlier
+  draft of this fix narrowed to containers alone, which would have
+  deleted the whole-file `class_npm_sum` (7,530 fields across the
+  integration corpus, 400 of them non-zero) and left `npm` / `npa`
+  disagreeing with `wmc` about the same root.
+
+  Consumers reading `metrics.npm` off a *function* space now find the key
+  absent. Across the integration corpus that removes 6,974 blocks, of
+  which 6,969 were entirely zero. The other five belong to a function
+  that lexically contains a class — a PHP `new class { … }` or a
+  JavaScript class inside a callback — where the block was that nested
+  class's roll-up; it remains available on the class's own space and in
+  the file-root total, and `wmc` has always omitted it in the same
+  position. No metric value changed anywhere.
+
+  The CSV projection is a fixed-column format and is unaffected: it
+  writes the `npm.*` / `npa.*` columns on every row regardless of space
+  kind, carrying the real accessor values.
+
 - **Metric drift.** ABC `conditions` moves wherever a comment sits inside
   a ternary (#1181). Slots are now addressed by grammar field rather than
   by neighbouring token or fixed index, which fixes two opposite errors

STABILITY.md

@@ -1074,8 +1074,13 @@ drift-gate test, so the Python types cannot diverge from the JSON the
 CLI emits. The change is stub-only (the runtime values are plain
 dicts, byte-identical to the CLI output), so it only narrows the
 static type. Every metric block is `NotRequired` because a `metrics=`
-selection can elide blocks; under the default full suite every block
-is present. The VCS *report* dicts are now single-sourced and typed too
+selection can elide blocks, and because the object-oriented blocks are
+scope-gated: `wmc`, `npm` and `npa` are emitted on container spaces and
+on the file root, and — outside Rust and Go, which gate `npm` / `npa`
+on their own node kinds — not on a function space (#1197). A language
+with no class-like construct emits them nowhere. Which spaces carry
+which block is not part of the shape contract — the `wire` struct
+definitions are. The VCS *report* dicts are now single-sourced and typed too
 (#664): `vcs.rank` returns `VcsReportDict`, `vcs.trend` returns
 `VcsTrendDict`, `vcs.commit` returns `JitCommitReportDict`, and
 `vcs.score_diff` returns `JitDiffReportDict`. The report / trend envelope

big-code-analysis-book/src/metrics.md

@@ -917,6 +917,31 @@ decides what counts as "public" (Java `public`, C# `public`, Rust
 `pub`, Python's "no leading underscore" convention, …) and what
 counts as "attribute" rather than "method".
 
+### Which spaces carry NPA, NPM and WMC {#oop-emission-scope}
+
+The three object-oriented blocks are emitted on **container spaces** —
+`class`, `struct`, `trait`, `impl`, `namespace`, `interface` — and on
+the whole-file `unit` root, which carries the roll-up across every
+container in the file. A **function space does not carry them**: a
+method owns no methods or attributes of its own, so the block would be
+all zeros. Before big-code-analysis 2.1.0, NPA and NPM did emit that
+all-zero block on function spaces in C#, JavaScript, MozJS, TypeScript,
+TSX, PHP and Ruby, and on the Kotlin, Java, Groovy and JS-family
+accessor / `init` / `static` spaces.
+
+Two caveats. Go and Rust do not follow that rule: they set the flag
+from their own node kinds, on whichever space encloses the `struct`
+they are counting. So a `struct` declared inside a function puts the
+block on that **function** space, and a file with no `struct` at file
+scope leaves the root without one — both the opposite of what the rule
+above predicts. And the CSV projection is a fixed-column format: it
+writes the `npa.*` / `npm.*` columns on **every** row regardless of
+space kind, carrying the real accessor values rather than eliding them.
+
+Thresholds are narrower still — `bca check` gates `npa` / `npm` on
+container spaces only, never the file root (see
+[Threshold scope](commands/check.md#threshold-scope)).
+
 ### How to read it
 
 NPA is a *direct* measure of encapsulation. Every public attribute
@@ -946,7 +971,8 @@ As with NPA, big-code-analysis splits NPM by definition-site kind
 `interface_npm_sum`, `class_methods` (sum of *all* methods — public or
 not — across classes), `interface_methods`, `class_coa`,
 `interface_coa` (operation-accessibility *ratios*, not averages),
-`total`, `total_methods`, and `coa`.
+`total`, `total_methods`, and `coa`. It follows the same emission rule
+as NPA ([above](#oop-emission-scope)).
 The language-specific `Npm` trait decides what counts as public —
 for example, Rust's `pub`, Python's leading-underscore convention,
 C++'s `public:` section — and folds together regular methods,

big-code-analysis-py/src/types_codegen.rs

@@ -1056,19 +1056,30 @@ mod tests {
     /// never appear in the Rust fixture (disabled / not injected), so they
     /// are checked against a language that emits them and a synthetic
     /// `vcs` block respectively — otherwise their specs would be untested.
+    ///
+    /// `npm` / `npa` are read off the class space rather than the file
+    /// root. Both carry the block, but the class is where the counts are
+    /// actually produced — the root only rolls them up — so asserting
+    /// there keeps the spec check independent of the roll-up.
     #[test]
     fn class_only_and_vcs_specs_match_wire_json_keys() {
         use big_code_analysis::wire;
 
-        // Java emits wmc / npm / npa on a class.
+        // Java emits all three on both the root and the class; read wmc
+        // at the root and npm / npa at the class, so each is checked
+        // where the metric is defined rather than where it aggregates.
         let value = analyze_to_value(
             "java",
             "public class Foo {\n    public int a;\n    public int m() { return 1; }\n}\n",
         );
         let metrics = value["metrics"].as_object().expect("metrics");
         assert_keys_match("WmcDict", &metrics["wmc"]);
-        assert_keys_match("NpmDict", &metrics["npm"]);
-        assert_keys_match("NpaDict", &metrics["npa"]);
+
+        let class = &value["spaces"][0];
+        assert_eq!(class["kind"], "class", "fixture's first child is the class");
+        let class_metrics = class["metrics"].as_object().expect("class metrics");
+        assert_keys_match("NpmDict", &class_metrics["npm"]);
+        assert_keys_match("NpaDict", &class_metrics["npa"]);
 
         // VcsDict: serialize a default wire::Vcs directly (with the two
         // optional fields populated so they appear in the key set, which we

big-code-analysis-py/tests/test_metrics_select.py

@@ -81,6 +81,11 @@ def test_metric_names_round_trip_through_analyze() -> None:
     a unit-level space when there is no class to attach them to —
     use the Java fixture (which carries a class) for those so the
     output key actually appears.
+
+    They are also elided from every *function* space, which is where
+    the unit root differs from a nested one (#1197); the assertion
+    below reads the root, and ``test_class_only_metrics_skip_function_spaces``
+    pins the other half.
     """
     class_only = {"npa", "npm", "wmc"}
     for name in bca.METRIC_NAMES:
@@ -514,3 +519,62 @@ def test_metrics_does_not_override_skip_generated() -> None:
     """
     result = bca.analyze(FIXTURES / "generated.rs", metrics=["loc"])
     assert result is None
+
+
+def test_class_only_metrics_skip_function_spaces() -> None:
+    """``npa`` / ``npm`` never appear on a function space (#1197).
+
+    The root-level assertion in
+    ``test_metric_names_round_trip_through_analyze`` cannot see this:
+    it reads only the top-level space, which legitimately carries the
+    whole-file roll-up.
+
+    Uses **JavaScript**, not the ``Hello.java`` fixture. Java's
+    ``is_func_space`` never listed ``method_declaration``, so a Java
+    method carried no block even before the fix and asserting its
+    absence would hold either way. JavaScript's did list every function
+    form, so each of the three function spaces below — a class method,
+    a static block, and a top-level function — really did carry an
+    all-zero ``npm``/``npa`` block in 2.0.x.
+    """
+    result = bca.analyze_source(
+        "class C {\n"
+        "  a = 1;\n"
+        "  static { this.b = 2; }\n"
+        "  m() { return this.a; }\n"
+        "}\n"
+        "function top(x) { return x; }\n",
+        "javascript",
+    )
+    assert result is not None
+    assert {"npa", "npm"} <= _metrics_keys(result), "root keeps its roll-up"
+
+    spaces = _walk(result)
+    classes = [s for s in spaces if s["kind"] == "class"]
+    assert len(classes) == 1, f"expected one class space, got {_shape(spaces)}"
+    assert {"npa", "npm"} <= set(classes[0]["metrics"].keys()), (
+        "the container itself must still carry both blocks"
+    )
+
+    functions = [s for s in spaces if s["kind"] == "function"]
+    assert len(functions) == 3, (
+        f"expected m, <static-init> and top as function spaces, got {_shape(spaces)}"
+    )
+    for space in functions:
+        keys = set(space["metrics"].keys())
+        assert not ({"npa", "npm"} & keys), (
+            f"function space {space['name']!r} must not carry npa/npm; got {sorted(keys)}"
+        )
+
+
+def _shape(spaces: list[FuncSpaceDict]) -> list[tuple[str, str | None]]:
+    """``(kind, name)`` per space, for an assertion message."""
+    return [(s["kind"], s["name"]) for s in spaces]
+
+
+def _walk(space: FuncSpaceDict) -> list[FuncSpaceDict]:
+    """Every space in the tree, root first."""
+    out = [space]
+    for child in space.get("spaces", ()):
+        out.extend(_walk(child))
+    return out