Skip to content

docs(cognitive): pin the Sonar paper revision and reconcile the remaining appendix citations #1765

Closed

Description

@dekobon

Found by the six-reviewer pass over #1730's fix (fix/batch-2026-11-01), which relabelled 29 labelled-jump citations from Sonar appendix B2 to B1. Every relabel was correct; these are the things the review turned up beside them, each pre-existing and each outside a comment-only fix.

1. The tree dates the paper to a revision that has no Appendix B

big-code-analysis-book/src/metrics.md:35,299 and README.md:19 call it "the 2017 white paper". The paper's own change log (page 18 of the current PDF): v1.0 is 19 April 2017; Appendix A arrived 9 July 2021; Appendix B (Specification) and C (Examples) arrived 29 August 2023. So every §B1 citation in the tree — the 29 from #1730, the recursion relabel from a3c8ebdd, and the boolean-sequence citations at cognitive.rs:1994, 8025, 8532, … — names a section that did not exist in the revision the book dates it to. The URL sonarsource.com/docs/CognitiveComplexity.pdf is unversioned, so the next relabel is the same guess.

Fix: record the revision once beside the link in metrics.md ("v1.7, 29 August 2023") and state the appendix mapping (§1 Increments, §2 Nesting level, §3 Nesting increments) in one paragraph at the top of src/metrics/cognitive.rs beside the recursion note at :35, so a per-site §B1 can be checked locally. Keep the per-site citations — they sit beside // expected: derivations and are load-bearing there. Do not add a source-grepping test (.claude/rules/testing.md forbids that shape). The README.ja.md rule applies if README.md changes.

2. PHP break N / continue N is +0, and B1 says otherwise

src/metrics/cognitive/php.rs:75-82 scores the numeric level form break 2; at +0 ("a structured loop-level exit"), pinned by a test at cognitive.rs:9883-9902. Appendix B §1 lists break NUMBER, continue NUMBER as increments, and the v1.5 change log says "all multi-level break/continue receive a fundamental increment". Under the old wrong B2 label the conflict was hidden; now that the comment correctly cites B1 it contradicts the section it cites. This is a metric policy question (cross-language: does any other grammar have a numeric-level jump?), not a comment fix — decide it, then either move the metric with a snapshot refresh or document the deliberate divergence beside the citation.

3. Smaller citation gaps

  • cognitive.rs:8054-8055, 8146, 8184 (and two more "flat +1 (no nesting penalty) per Sonar B1" sites): the increment is B1's; the absence of a nesting penalty is B3's exhaustive list. Cite both.
  • cognitive.rs:10243: Ruby redo / retry are scored "+1 per §B1", but B1's jump bullet lists only goto/break/continue with LABEL or NUMBER; the comment extrapolates. Say so ("by analogy with", not "per").
  • big-code-analysis-book/po/ja.po:2141-2166: the two msgids still carry (B3) / (B2) with empty msgstr; make book-po-update fixes that whenever next run — nothing to do, noted so nobody reads it as drift.
  • CHANGELOG.md:2162, 7712, 9065, 9302, 11322: five released entries cite §B2. Historical record; leave them.

Labels: documentation. Items 1 and 3 are one small docs PR; item 2 needs its own decision.

Partial resolution

Status: items 1 and 3 addressed (pending merge) in d3059dea on fix/batch-2026-11-03-2 (Refs #1765). Item 2 (PHP break N / continue N) remains open.

  • Item 1: the book and src/metrics/cognitive.rs now name the cited revision (v1.7, 29 August 2023) and summarise B1/B2/B3. Correction to the premise: per the paper's change log, Appendix B was added in v1.2 (19 April 2017), so "the 2017 white paper" is accurate. The pin is needed because the URL is unversioned and v1.5 reworded the rules.
  • Item 3: the five flat-+1 sites cite B1 and B3. Ruby redo/retry is "by analogy with §B1". The ja.po and CHANGELOG.md bullets were left as stated.
  • New: the paper ignores null-coalescing operators, so scoring ?? / Elvis is a deliberate divergence. That is tracked in docs(cognitive): scoring ?? / Elvis diverges from Sonar's null-coalescing rule #1800.

Resolution

All three items are now addressed.

  • Items 1 and 3: d3059dea (docs(cognitive): pin the Sonar paper revision, on origin/main via fix: batch 2026-11-03-2 (8 issues) #1804).
  • Item 2 (PHP break N / continue N): resolved in 5d631672 on fix/batch-2026-11-04 (PHP, together with docs(cognitive): scoring ?? / Elvis diverges from Sonar's null-coalescing rule #1800) and a4864a95 (Bash, found by the whole-branch review). The numeric form now pays B1's flat +1 for any explicit argument, break 1; included — sonar-php increments on tree.argument() != null (php-frontend/src/main/java/org/sonar/php/metrics/CognitiveComplexityVisitor.java, visitBreakStatement / visitContinueStatement). The gate is a named, non-comment child of the jump (the grammar admits any expression there), so break /* c */; stays +0. Bash's POSIX break n / continue n is the one sibling with a numeric level — a plain command with no jump kind, read by its leading word — and scores the same way (bash_jump_pays_for_any_explicit_level_1765); Tcl, Lua and Ruby break take no level, Perl's last LABEL was already scored. Pinned by php_numeric_break_counted_1765 (the fix(metrics): cognitive jump-statement increments inconsistent across languages #435 fixture, 6 → 7) and php_jump_pays_for_any_explicit_level_1765 (bare / 1 / 2 for both keywords, plus the comment case), each verified by deleting the arm and by weakening its gate. The book's PHP deviation bullet now states the increment rather than a pending deviation.

Commit body carries Fixes #1765; the batch integrator closes on merge.

Activity

  1. dekobon commented on Nov 4, 2026

    @dekobon
    OwnerAuthor

    Items 1 and 3 addressed (comment/doc only); item 2 still open

    Item 2 (PHP break N / continue N) is deliberately excluded. It is a metric-policy decision, not a citation fix, and this issue stays open for it. The commit uses Refs #1765, not Fixes.

    The premise of item 1 was wrong, but the fix still holds

    I read the paper itself: https://www.sonarsource.com/docs/CognitiveComplexity.pdf, fetched 2026-11-03. It is 22 pages, and its cover says "29 August 2023, Version 1.7". Its change log says:

    Version Date Relevant change
    1.1 6 Feb 2017 Add Appendix A (Compensating Usages)
    1.2 19 Apr 2017 Add Appendix B (Specification) and Appendix C (Examples)
    1.3 15 Mar 2018 Extend Appendix A (Python decorators)
    1.4 10 Sep 2018 Clarify which method nesting raises the nesting level
    1.5 5 Apr 2021 Make explicit that all multi-level break/continue receive a fundamental increment
    1.6 9 Jul 2021 Remove author's title
    1.7 29 Aug 2023 Branding and layout only

    So Appendix B has existed since April 2017, and "the 2017 white paper" in the book is accurate. The issue body's mapping (Appendix A in 2021, Appendix B in 2023) appears to read the change-log columns off by several rows. The underlying point still holds: the URL is unversioned and the rules were reworded (v1.5), so a per-site citation needs a named revision to be checkable. metrics.md:35 ("Campbell / SonarSource, 2017", a first-defined year) is correct and unchanged. README.md never said "2017", so README / README.ja.md are untouched.

    Appendix B text as verified at v1.7:

    • B1 Increments: if, else if, else, ternary; switch; for, foreach; while, do while; catch; goto LABEL, break LABEL, continue LABEL, break NUMBER, continue NUMBER; sequences of binary logical operators; each method in a recursion cycle.
    • B2 Nesting level: if, else if, else, ternary; switch; for, foreach; while, do while; catch; nested methods and lambdas.
    • B3 Nesting increments: if, ternary; switch; for, foreach; while, do while; catch.

    Changes

    • big-code-analysis-book/src/metrics.md: one paragraph after the paper links (the citation lines only; the lambda paragraph and ABC section are not touched). It says the link serves the latest revision, the rules were reworded since 2017 (v1.5), and the chapter and source cite v1.7, 29 August 2023. It also gives the §B1/§B2/§B3 mapping. The ja.po entry for that paragraph falls back to English until the next make book-po-update.
    • src/metrics/cognitive.rs module header, beside the recursion note: the revision, plus a summary of B1/B2/B3 at v1.7, so a per-site §B1 can be checked locally. Per-site citations are kept. No source-grepping test was added.
    • Item 3, five flat-+1 sites (typescript_nullish_coalescing_with_if_230, csharp_null_coalescing_cognitive_230, php_null_coalescing_cognitive_230, groovy_elvis_inside_if_246, kotlin_elvis_inside_if_239): each now cites B1 for the increment and B3 for the absent nesting increment.
    • Item 3, ruby_redo_retry_counted: "+1 per §B1" is now "+1 by analogy with §B1", and the comment notes that B1's jump bullet names only goto and a break/continue to a LABEL or NUMBER. The production comment in cognitive/ruby.rs never claimed B1 for redo/retry and is unchanged.
    • Item 3 ja.po and CHANGELOG.md bullets: left as the issue said.

    New finding: null-coalescing is a deviation, not a B1 reading

    All five item-3 sites are ?? / Elvis tests. The paper's "Ignore shorthand" section (page 6) says "Cognitive Complexity ignores null-coalescing operators". B1's "sequences of binary logical operators" does not mention them either. So citing B1 and B3 alone would have dressed a deliberate divergence (#230, #239, #246, plus Perl's //) as conformance. The module header and the TypeScript site now say it is this crate's choice. Filed as #1800 for the policy decision and the user-facing deviation note.

    Validation

    make pre-commit: BCA_GATE: pass (gate=pre-commit). cognitive.rs carries no .bca-baseline.toml entry (rg 'src/metrics/cognitive.rs"' .bca-baseline.toml is empty, and exclude_tests = true keeps the test-comment edits out of the measured lines). make self-scan-baseline-fresh: 0 added, 0 removed, 0 worsened, 0 improved, so no refresh was needed.

    Commit on fix/batch-2026-11-03-2: bc68f525 (docs(cognitive): pin the Sonar paper revision).

  2. dekobon commented on Nov 4, 2026

    @dekobon
    OwnerAuthor

    The batch branch fix/batch-2026-11-03-2 was rebased to fold the whole-branch review's fixes into the commits they correct, so the commit cited above has a new hash: bc68f525 → d3059de. The branch has since been rebased onto main for PR #1804; the hashes here are the current ones. The final tree is unchanged apart from main's own changes and a one-line doc pointer fix in the #1649 commit, and the Resolution section is updated.

  3. dekobon commented on Nov 5, 2026

    @dekobon
    OwnerAuthor

    Resolution

    All three items are now addressed.

    • Items 1 and 3: 1d84960d (docs(cognitive): pin the Sonar paper revision, on fix/batch-2026-11-04).
    • Item 2 (PHP break N / continue N): resolved in 081a6bd8 on fix/issue-1800, together with docs(cognitive): scoring ?? / Elvis diverges from Sonar's null-coalescing rule #1800. The numeric form now pays B1's flat +1 for any explicit argument, break 1; included — sonar-php increments on tree.argument() != null (php-frontend/src/main/java/org/sonar/php/metrics/CognitiveComplexityVisitor.java, visitBreakStatement / visitContinueStatement). The gate is a named, non-comment child of the jump (the grammar admits any expression there), so break /* c */; stays +0. No other grammar in the tree has a numeric-level jump. Pinned by php_numeric_break_counted_1765 (the fix(metrics): cognitive jump-statement increments inconsistent across languages #435 fixture, 6 → 7) and php_jump_pays_for_any_explicit_level_1765 (bare / 1 / 2 for both keywords, plus the comment case), each verified by deleting the arm and by weakening its gate. The book's PHP deviation bullet now states the increment rather than a pending deviation.

    Commit body carries Fixes #1765; the batch integrator closes on merge.

  4. dekobon commented on Nov 5, 2026

    @dekobon
    OwnerAuthor

    Correction to the resolution record (whole-branch review)

    Two things the batch's review pass found in this issue's record:

    1. "No other grammar in the tree has a numeric-level jump" was wrong. Bash has POSIX break n / continue n, parsed by tree-sitter-bash as a plain command (command_name: break, number: 2) with no jump kind — the grammar-dispatch §9 shape. It scored +0 while PHP's was flipped to +1. Fixed in a4864a95 on fix/batch-2026-11-04: the Bash cognitive arm reads the leading word off the command's name field and a named level after it, +1 for any written level (break 1 included), +0 for the bare word — the same reading as PHP. Pinned by bash_jump_pays_for_any_explicit_level_1765 (cognitive 5 → 7 on the fixture; cyclomatic 6 either way, anchoring that the jumps are still spelled). Tcl, Lua and Ruby break take no level; Perl's last LABEL was already scored.
    2. The Resolution section cited 1d84960d for items 1 and 3; that hash is a pre-rebase copy no branch contains. The change is on origin/main as d3059dea (docs(cognitive): pin the Sonar paper revision). Item 2 is 5d631672 on fix/batch-2026-11-04 (PHP) plus a4864a95 (Bash).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentation

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions