Skip to content

chore(scripts): CheckStyle CS0016/CS0014 -- decide the two-line-body exemption, sweep the ~400 tree hits, consider a free-function rule #120

Description

@relmer

What

scripts/CheckStyle.ps1's CS0016 ("exactly 3 blank lines after the declaration block") was found to be blind in three ways on branch 019-assembler-dialects, and was corrected there. The corrected gate now reports far more than it used to across the tree, and two decisions were deliberately left open rather than made inside a feature branch.

The three blindnesses (fixed on 019, commits 0755f997, 03c24191)

  1. The declaration regex did not recognize the constructor form Type name (args);, so std::ifstream file (path); ended the block a line early.
  2. The rule exempted any block followed by zero blank lines -- which is exactly the shape of the violation it exists to catch (isOpen = file.is_open(); sitting where the three blanks belong).
  3. A function whose parameter list wrapped was never checked at all, by CS0016 or CS0014, because the line above the { was an indented continuation rather than the signature.

Also fixed: wrapped initializers, trailing // comments, comment lines between declarations, and a "--" inside a string literal all used to end the block early.

What the corrected gate reports tree-wide (as of 019 at 97f9a4ee)

Rule Count Shape
CS0016 ~351 175 with 1 blank, 99 with 0, 77 with 2
CS0014 ~54 functions with wrapped signatures and no //// banner

The gate is diff-scoped (origin/master..HEAD), so none of this blocks a push today; it surfaces only when a touched function is one of them. Everything on 019's own added lines was fixed there.

Decisions to make

  1. Is the two-line body exempt? Nearly all of the 0-blank hits are this shape:

    Byte *  base = m_ramRd ? m_auxRam.data() : m_mainRamPtr;
    return base + (page * kPageSize);

    .github/copilot-instructions.md line 318 says "EXACTLY 3 blank lines between variable definitions at the top of a function/block and the first real statement", and return is the first real statement -- so as written this is a violation. If the intent is otherwise, that is a one-line change to the gate and one to the doc.

  2. Sweep or live with it. Once (1) is settled, either sweep the backlog (mechanical; scripts/FixDeclAlign.ps1 does not cover it, so a small fixer would help) or leave it to be fixed as files are touched.

  3. Should there be a rule for file-scope static functions? The constitution says helpers are class statics and a free function "needs a very convincing justification"; CS0002 catches anonymous namespaces but nothing catches static ReturnType Fn (...) at file scope. CassoCli/CommandLine.cpp had accumulated 34 of them before review caught it; a grep at the time found 112 across 40 other files.

Not for 019

Deliberately not done on the feature branch: the sweep would touch ~100 files unrelated to the feature, and the two-liner question is a rule decision, not a code fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    impact: engineeringInternal only: testability, architecture, maintainabilitypriority: lowDev / power-user / infrastructure (end-user-value triage)

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions