Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,53 @@ for historical reference.

### Fixed

- **Serialized shape.** `npm` and `npa` emission is now decided by the
space's kind alone, for every language (#1203). #1197 declared the rule
— containers and the file `unit` root carry the block, a function space
never does — but enforced it only for the ten languages it routed
through a shared predicate. The other seven kept enabling from their
own grammar node kinds and disagreed with it in both directions:

- A Go or Rust `struct` declared inside a function body put the block
on that **function** space. Across the `serde` corpus that was 39
function spaces in 10 files, most of them `#[test]` functions
declaring a local `struct`.
- A C++ `namespace`, and any file root whose only container sat inside
a function, carried **no** block. In the last case the counts were
serialized nowhere at all — they reached the root's `_sum` fields,
which nothing emitted — so merely suppressing the function-space
block would have deleted them from output rather than relocating
them.

The space kind is now the only input, recorded once per space by the
walker, so there is no per-language surface left to deviate on.
Practically: every container space and every file root of a language
with class-shaped constructs carries both blocks, and no function space
does. In this repository's integration corpora that adds a block to
1,214 file roots and 1,332 C++ namespaces, and removes one from 39 Rust
function spaces.

**No metric value changed.** The counts always rolled up through every
enclosing space regardless of which one serialized them; this moves
which space reports them. Thresholds are unaffected — `bca check` reads
`metrics.npm` directly through `MetricScope`, which never consulted the
emission gate. `STABILITY.md` already places which space carries which
block outside the shape contract.

Languages with no class-shaped construct at all — Bash, C, Lua, Perl,
Tcl, iRules — still emit neither block, rather than gaining an all-zero
one on every file root. Go remains the one language whose `npm` / `npa`
appear only on the root, because its space tree has no container kind;
since `bca check` gates both on container spaces, no `npm` or `npa`
limit can fire on Go source. Both are documented in the metrics guide.

One consequence reaches a front-end. The Python `to_sarif` binding
walks serialized JSON and skips a metric whose key is absent, so it
silently dropped every `npm` / `npa` offender on a C++ `namespace` —
a kind `MetricScope::Container` admits and the CLI has always gated,
reading the struct rather than the JSON. The two front-ends now agree,
and SARIF output may gain namespace-scoped findings it was missing.

- **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",
Expand Down
5 changes: 3 additions & 2 deletions STABILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -1076,8 +1076,9 @@ 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, 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
on the file root, and never on a function space (#1197, #1203). Since
the latter, the space's own kind is the sole input for every language,
so no grammar deviates from that rule in either direction. 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
Expand Down
35 changes: 25 additions & 10 deletions big-code-analysis-book/src/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -927,20 +927,35 @@ 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.
accessor / `init` / `static` spaces. In the same release Go, Rust,
Python, C++, Objective-C and Elixir went the other way: they decided
from their own grammar node kinds, so a `struct` declared inside a
function put the block on that **function** space, while a `namespace`
or a file root with no container at file scope carried none. Both
deviations are gone — the space's kind is now the only input, for every
language.

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.
That rule governs the *block*, not the numbers behind it: the counts
roll up through every enclosing space regardless. So a type declared
inside a function body is reported by the nearest enclosing container,
or by the file root when there is none.

Two things read differently. A Go file's NPA and NPM live on the `unit`
root and nowhere else, because Go is the one language that emits them
without having a container kind in its space tree — `type … struct` and
`type … interface` open no space of their own. (Bash, C, Lua, Perl,
Tcl and iRules have no container kind either, but they emit neither
block anywhere, so the question does not arise.) 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)).
[Threshold scope](commands/check.md#threshold-scope)). Taken with the
paragraph above, that has a consequence worth stating outright: since a
Go file's NPA and NPM are only ever reported at the root, **no `npa` or
`npm` threshold can fire on Go source**.

### How to read it

Expand Down
5 changes: 4 additions & 1 deletion big-code-analysis-book/src/recipes/thresholds.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,10 @@ is therefore sparse on codebases predating `use v5.36`, and one anonymous-sub fo
because the grammar misparses its signature.

`wmc`, `npm`, and `npa` are only produced for languages with a class-like container. They are absent
from Bash, C, Go, Lua, Perl, and Tcl output.
from Bash, C, Lua, Perl, and Tcl output. Go is a half case: it has no container space at all, so
`wmc` is absent and `npm` / `npa` are reported only on the file `unit` root. `bca check` gates both
on container spaces and never on the root, so **no `npm` or `npa` limit can fire on Go source** — see
[Which spaces carry NPA, NPM and WMC](../metrics.md#oop-emission-scope).

A per-language override cannot fix any of these. Bash `nargs` is `0` for every function, so no
limit — not even `0`, which fires only on a value *above* it — can make the gate say anything. The
Expand Down
10 changes: 6 additions & 4 deletions big-code-analysis-py/tests/test_metrics_select.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,14 @@ def test_metric_names_round_trip_through_analyze() -> None:
"""Every name in ``METRIC_NAMES`` is accepted by ``metrics=``.

Class-only metrics (``npa``, ``npm``, ``wmc``) are elided from
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.
languages with no class-like construct. The unit root of a
language that has one carries all three even when the file
declares no class (#1203), so the Java fixture is no longer
required for the key to appear — it is kept so the three are
exercised on a root that merged a real container's counts.

They are also elided from every *function* space, which is where
the unit root differs from a nested one (#1197); the assertion
the unit root differs from a nested one (#1197, #1203); the assertion
below reads the root, and ``test_class_only_metrics_skip_function_spaces``
pins the other half.
"""
Expand Down
12 changes: 9 additions & 3 deletions big-code-analysis-web/src/web/server_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,9 @@ async fn test_web_metrics_json() {
"sei": 176.151_664_509_301_33,
"visual_studio": 81.856_334_244_533_39},
"abc": {"assignments": 0, "branches": 0, "conditions": 0, "magnitude": 0.0, "value": 0.0, "assignments_average": 0.0, "branches_average": 0.0, "conditions_average": 0.0, "assignments_min": 0, "assignments_max": 0, "branches_min": 0, "branches_max": 0, "conditions_min": 0, "conditions_max": 0},
"wmc": {"class_wmc_sum": 0, "interface_wmc_sum": 0, "total": 0}},
"wmc": {"class_wmc_sum": 0, "interface_wmc_sum": 0, "total": 0},
"npm": {"class_npm_sum": 0, "interface_npm_sum": 0, "class_methods": 0, "interface_methods": 0, "class_coa": 0.0, "interface_coa": 0.0, "total": 0, "total_methods": 0, "coa": 0.0},
"npa": {"class_npa_sum": 0, "interface_npa_sum": 0, "class_attributes": 0, "interface_attributes": 0, "class_cda": 0.0, "interface_cda": 0.0, "total": 0, "total_attributes": 0, "cda": 0.0}},
"name": "test.py",
"spaces": [{"kind": "function",
"start_line": 3,
Expand Down Expand Up @@ -730,7 +732,9 @@ async fn test_web_metrics_json_unit() {
"sei": 142.643_061_717_489_76,
"visual_studio": 88.422_991_744_574_97},
"abc": {"assignments": 0, "branches": 0, "conditions": 0, "magnitude": 0.0, "value": 0.0, "assignments_average": 0.0, "branches_average": 0.0, "conditions_average": 0.0, "assignments_min": 0, "assignments_max": 0, "branches_min": 0, "branches_max": 0, "conditions_min": 0, "conditions_max": 0},
"wmc": {"class_wmc_sum": 0, "interface_wmc_sum": 0, "total": 0}},
"wmc": {"class_wmc_sum": 0, "interface_wmc_sum": 0, "total": 0},
"npm": {"class_npm_sum": 0, "interface_npm_sum": 0, "class_methods": 0, "interface_methods": 0, "class_coa": 0.0, "interface_coa": 0.0, "total": 0, "total_methods": 0, "coa": 0.0},
"npa": {"class_npa_sum": 0, "interface_npa_sum": 0, "class_attributes": 0, "interface_attributes": 0, "class_cda": 0.0, "interface_cda": 0.0, "total": 0, "total_attributes": 0, "cda": 0.0}},
"name": "test.py",
"spaces": []}
});
Expand Down Expand Up @@ -834,7 +838,9 @@ async fn test_web_metrics_plain() {
"sei": 142.643_061_717_489_76,
"visual_studio": 88.422_991_744_574_97},
"abc": {"assignments": 0, "branches": 0, "conditions": 0, "magnitude": 0.0, "value": 0.0, "assignments_average": 0.0, "branches_average": 0.0, "conditions_average": 0.0, "assignments_min": 0, "assignments_max": 0, "branches_min": 0, "branches_max": 0, "conditions_min": 0, "conditions_max": 0},
"wmc": {"class_wmc_sum": 0, "interface_wmc_sum": 0, "total": 0}},
"wmc": {"class_wmc_sum": 0, "interface_wmc_sum": 0, "total": 0},
"npm": {"class_npm_sum": 0, "interface_npm_sum": 0, "class_methods": 0, "interface_methods": 0, "class_coa": 0.0, "interface_coa": 0.0, "total": 0, "total_methods": 0, "coa": 0.0},
"npa": {"class_npa_sum": 0, "interface_npa_sum": 0, "class_attributes": 0, "interface_attributes": 0, "class_cda": 0.0, "interface_cda": 0.0, "total": 0, "total_attributes": 0, "cda": 0.0}},
"name": "test.py",
"spaces": [{"kind": "function",
"start_line": 1,
Expand Down
33 changes: 29 additions & 4 deletions src/macros/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,12 @@ macro_rules! implement_metric_trait {
);
// Internal helper: shared no-op body for traits whose `compute`
// signature is `<'a>(&Node<'a>, &'a [u8], Ancestors<'a, '_>,
// &mut Stats)` (Abc, Cyclomatic, Npa, Npm). Public arms below
// delegate here so the body is written once.
// &mut Stats)` (Abc, Cyclomatic). Public arms below delegate here
// so the body is written once. `Npa` and `Npm` share the signature
// but need `HAS_MEMBERS = false` as well, so they route through
// `@code_and_chain_taking_memberless` instead — reaching for this
// arm for a new no-op `Npa` / `Npm` impl would silently restore
// the all-zero file-root block #1203 removed.
(@code_and_chain_taking $trait:ident, $($code:ident),+) => (
$(
impl $trait for $code {
Expand All @@ -96,11 +100,32 @@ macro_rules! implement_metric_trait {
(Cyclomatic, $($code:ident),+) => (
implement_metric_trait!(@code_and_chain_taking Cyclomatic, $($code),+);
);
// `Npa` and `Npm` take the same shape as the arm above plus one
// thing: the no-op impl must also opt the language out of
// *emitting* the block, which `HAS_MEMBERS` does. Without it a shell
// script would report `class_npa_sum: 0`, because the file unit is a
// member scope like any other and the walker would record its kind
// (#1203). `wmc` reaches the same place by different means — its
// no-op `compute` simply never records a kind.
(@code_and_chain_taking_memberless $trait:ident, $($code:ident),+) => (
$(
impl $trait for $code {
const HAS_MEMBERS: bool = false;

fn compute<'a>(
_node: &Node<'a>,
_code: &'a [u8],
_ancestors: crate::Ancestors<'a, '_>,
_stats: &mut Stats,
) {}
}
)+
);
(Npa, $($code:ident),+) => (
implement_metric_trait!(@code_and_chain_taking Npa, $($code),+);
implement_metric_trait!(@code_and_chain_taking_memberless Npa, $($code),+);
);
(Npm, $($code:ident),+) => (
implement_metric_trait!(@code_and_chain_taking Npm, $($code),+);
implement_metric_trait!(@code_and_chain_taking_memberless Npm, $($code),+);
);
(Loc, $($code:ident),+) => (
$(
Expand Down
Loading