Code health: ban char*, prefix overloads, and too many header names

AGENTS.md

@@ -314,6 +314,9 @@ so keep it where it clearly pays off. `std::string_view` is the exception (see S
   ids (`for (std::string_view slug : {...})`). Never store it past the call (TotW #1, #180).
   The user's call over upstream's `const std::string&` habit (2026-11-02): fix it across our PRs
   before an upstream handoff. Existing upstream APIs we don't otherwise touch keep their signature.
+- **No `char*` / `char[]` in new code.** Use `std::string`, or `std::string_view`
+  (`inline constexpr std::string_view` for constants); `unsigned char` bytes and `main(argc, argv)`
+  are fine (the user, review of #312). Code Health fails added lines.
 - **`std::format`** for messages with several parts, instead of `+` / `std::to_string`
   chains or `ostringstream` (TotW #3, #124). Raw string literals for embedded JSON (TotW #64).
 
@@ -355,12 +358,19 @@ so keep it where it clearly pays off. `std::string_view` is the exception (see S
   `/*save=*/false` at the call site.
 - **No sentinel values in new APIs** (`-1`, `""`, `0` meaning "none"). Return
   `std::optional` or a result enum like `WildBattleExitResult` (TotW #171). For an optional
-  input, overload or take `const T*`, not `const std::optional<T>&` or `= ""` (TotW #163).
+  input, use a default argument with a real value, `std::optional<T>` by value or `const T*`,
+  not `const std::optional<T>&` or `= ""` (TotW #163).
 - **No top-level `const` on by-value parameters in declarations** (TotW #109).
 - **File-local helpers go in an unnamed `namespace{`**, not `static` and not the header
   (TotW #186).
 - **Overloads only when every overload means the same thing**
   (`move_cursor_to_option(..., BattleMenuOption / BattleMoveOption)`) (TotW #148).
+- **Default arguments over overloads.** When one overload's parameters are the other's plus more,
+  write one function with a default argument or a `std::optional` parameter (the user, review of
+  #312). Code Health fails such a pair in a header when either is new.
+- **Few names per header:** at most 8 public namespace-scope names (types, aliases, free functions
+  with their overloads, constants); 7-8 is a hint to split. `detail`/anonymous namespaces and class
+  members don't count. Code Health holds new headers to 8 and existing ones to their base count.
 - **Lambdas, not `std::bind`** (TotW #108). Free functions over extra public methods
   (TotW #99).
 

devtools/code_health/README.md

@@ -49,6 +49,7 @@ are included. Missing base revisions and analysis errors fail the command.
 | Public methods per class/struct | 7 (excluding ctors, dtors, operators, deleted and Qt signals); 6-7 advisory |
 | Public non-static data in a class with mutating methods | 0 (exempt: plain structs, value types with only const/static methods, const fields, `*Option`/`*Cell` fields, test files) |
 | Single-method interface (one pure virtual + optional virtual dtor, no bases) | not allowed: prefer a callable (`MoveOnlyFunction` / `std::function`) |
+| Public names per header: distinct namespace-scope types, enums, aliases, free functions (overloads count once) and constants/variables, outside anonymous, `detail`, `internal` and `*Internal` namespaces; class members, operators, forward declarations and macros excluded (`header_api.py`) | 8; 7-8 advisory |
 
 New files/functions must meet these limits. In modified files, compare each
 metric with its value at the base revision: existing violations may remain or
@@ -94,11 +95,14 @@ void load_move_table(){
 `// code-health: allow <check>[,<check>] -- <justification>` lets the listed metrics of one
 function or class exceed their limit. Checks are the metric names in the table above:
 `file_lines`, `function_lines`, `nesting`, `parameters`, `public_methods`, `public_data`,
-`single_method_interface`. Style and layering findings have no bypass.
+`single_method_interface`, `header_symbols`. Two style findings take a tag too: `char_pointer` and
+`overload_prefix`, for a C API or a fixed interface that needs them. Other style and layering
+findings have no bypass.
 
 - **Place it** on the function's or class's first line, or in the comment block directly above
   it (only `//` comment and `template` lines between). A `file_lines` tag goes in the first 20
-  lines of the file. A tag that attaches to nothing fails.
+  lines of the file, and so does a `header_symbols` tag. A `char_pointer` or `overload_prefix` tag goes on
+  the flagged line or in the comment block directly above it. A tag that attaches to nothing fails.
 - **Justify it:** the text after `--`, on the tag line itself, must be at least 40 characters and 6 words,
   and say why splitting would make the code worse. Unknown checks, a missing or short
   justification, or a malformed tag fail.
@@ -148,6 +152,8 @@ lexer, not a parser: rules that need types stay in review.
 | A defaulted `bool` parameter, or 2+ `bool` parameters (not `override`/`final`, not an unchanged signature) | error | TotW #94 |
 | `using namespace` at namespace scope in a header | error | TotW #153 |
 | Forward `class`/`struct` declaration in a header: new header / existing header | error / advisory | fork AGENTS.md "Forward declarations" |
+| `char*` / `const char*` / `char**` types and named `char` arrays (`char buf[8]`); not `unsigned`/`signed char`, `char8_t`-`char32_t`, `wchar_t`, a plain `char`, `*_cast<char*>`, or `main(argc, argv)` | error (tag: `char_pointer`) | fork AGENTS.md "Strings" |
+| In a header, two declarations of one function in the same scope (namespace or class) whose parameter types are a strict prefix of each other, one of them on an added line; not constructors, operators, `= delete`, variadic packs, or overloads that differ in a parameter's type | error (tag: `overload_prefix`) | fork AGENTS.md "Functions" |
 | A comment whose first sentence describes the neighbouring function, none of its own | advisory | CONTRIBUTING; Google "Function Comments" |
 
 Advisories print as `Advisory:` lines and `::warning` annotations and never fail.

devtools/code_health/__main__.py

@@ -5,7 +5,7 @@
 import sys
 from pathlib import Path
 
-from . import allow, layering, style
+from . import allow, header_api, layering, style
 from .classes import measure_classes
 from .findings import annotations
 from .git_files import changed_files, default_base, read_revision
@@ -14,7 +14,7 @@
 
 
 def analyze(path, source):
-    return measure(path, source) + measure_classes(path, source)
+    return measure(path, source) + measure_classes(path, source) + header_api.measure_header(path, source)
 
 
 def run(root, base_ref=None, github=False):
@@ -39,7 +39,10 @@ def run(root, base_ref=None, github=False):
         bypassed.extend(notices)
         notes.extend(unneeded)
         style_errors, style_notes = style.check(current, source, previous_source)
-        failures.extend(style_errors)
+        style_errors += header_api.prefix_overloads(current, source, previous_source)
+        style_errors, waived, unattached = allow.waive(current, source, allowances, style_errors)
+        failures.extend(style_errors + unattached)
+        bypassed.extend(waived)
         notes.extend(style_notes)
         advisory += sum(m.check == 'file_lines' and 500 < m.value <= 1000 for m in metrics)
     print(f'Scanned {len(changes)} changed C++ files against {base_ref}.')

devtools/code_health/allow.py

@@ -2,15 +2,22 @@
 
 A tag on a function's or class's first line, or in the comment block directly above it, lets
 that function's or class's listed metrics exceed their limit up to a hard ceiling. A
-`file_lines` tag goes in the first FILE_TAG_LINES lines. Every bypass is printed, quoting
+`file_lines` or `header_symbols` tag goes in the first FILE_TAG_LINES lines. A `char_pointer` or
+`overload_prefix` tag waives that style finding on its own line or the line below its comment
+block. Every bypass is printed, quoting
 its justification, so reviewers see it; malformed, unknown or misplaced tags fail."""
 import re
 from dataclasses import dataclass
 
-from .config import CLASS_LIMITS
+from .config import CLASS_LIMITS, HEADER_LIMITS
 from .findings import Finding
 
-CHECKS = ('file_lines', 'function_lines', 'nesting', 'parameters') + tuple(CLASS_LIMITS)
+# Style findings a tag can bypass; they attach like a function's tag, to the finding's line.
+STYLE_CHECKS = ('char_pointer', 'overload_prefix')
+# File-level metrics: the tag goes in the first FILE_TAG_LINES lines.
+FILE_CHECKS = ('file_lines',) + tuple(HEADER_LIMITS)
+CHECKS = (('file_lines', 'function_lines', 'nesting', 'parameters') + tuple(CLASS_LIMITS)
+          + tuple(HEADER_LIMITS) + STYLE_CHECKS)
 FILE_TAG_LINES = 20
 MIN_JUSTIFICATION_CHARS = 40
 MIN_JUSTIFICATION_WORDS = 6
@@ -67,9 +74,11 @@ def attach(path, source, allowances, metrics):
     allowed, errors = {}, []
     for allowance in allowances:
         for check in allowance.checks:
+            if check in STYLE_CHECKS:
+                continue
             targets = [m for m in metrics if m.check == check and covers(allowance, m, lines)]
             if not targets:
-                where = (f'in the first {FILE_TAG_LINES} lines' if check == 'file_lines'
+                where = (f'in the first {FILE_TAG_LINES} lines' if check in FILE_CHECKS
                          else 'on or directly above the function or class signature')
                 errors.append(Finding(path, allowance.line, f'code-health tag: {check} attaches to '
                                       f'nothing; put the tag {where}'))
@@ -78,8 +87,30 @@ def attach(path, source, allowances, metrics):
     return allowed, errors
 
 
+def waive(path, source, allowances, findings):
+    """(findings left, bypass notices, errors for style tags that attach to nothing).
+
+    A STYLE_CHECKS tag goes on the finding's line or in the comment block directly above it."""
+    lines = normalized_lines(source)
+    kept, notices, used = [], [], set()
+    for finding in findings:
+        tag = next((a for a in allowances if finding.check in a.checks
+                    and covers(a, finding, lines)), None)
+        if tag is None:
+            kept.append(finding)
+            continue
+        used.add((tag.line, finding.check))
+        notices.append(Finding(path, finding.line, f'{finding.check} bypassed by the tag at line '
+                               f'{tag.line}: "{tag.justification}"'))
+    errors = [Finding(path, a.line, f'code-health tag: {check} attaches to nothing; put the tag '
+                      'on or directly above the flagged line')
+              for a in allowances for check in a.checks
+              if check in STYLE_CHECKS and (a.line, check) not in used]
+    return kept, notices, errors
+
+
 def covers(allowance, metric, lines):
-    if metric.check == 'file_lines':
+    if metric.check in FILE_CHECKS:
         return allowance.line <= FILE_TAG_LINES
     if allowance.line == metric.line:
         return True

devtools/code_health/config.py

@@ -6,13 +6,19 @@
     'single_method_interface': 0,
 }
 
+# Distinct public namespace-scope names (types, aliases, functions, constants) per header.
+HEADER_LIMITS = {'header_symbols': 8}
+
 # Advisory floors: new or changed values above these (and within the limit) are reported.
 # Changed files above their floor are always reported.
-ADVISORY = {'file_lines': 500, 'function_lines': 25, 'public_methods': 5}
+ADVISORY = {'file_lines': 500, 'function_lines': 25, 'public_methods': 5,
+            'header_symbols': 6}
 
 HINTS = {
     'public_methods': 'split the class or move helpers into free functions',
     'public_data': 'make data private, or split it into a plain struct without methods',
+    'header_symbols': 'split the header by concern, or move helpers into a detail namespace '
+                      'or the .cpp',
     'single_method_interface': 'prefer a callable (MoveOnlyFunction / std::function) '
                                'over a single-method interface',
 }

devtools/code_health/findings.py

@@ -12,6 +12,7 @@ class Finding:
     path: str
     line: int
     message: str
+    check: str = ''  # set on findings an allow tag can bypass (allow.STYLE_CHECKS)
 
     def __str__(self):
         return f'{self.path}:{self.line}: {self.message}'

devtools/code_health/header_api.py

@@ -0,0 +1,149 @@
+"""Header API shape: prefix overloads and the number of public namespace-scope names.
+
+Both read the statements `style.py` lexes. An overload pair whose parameter types are a
+strict prefix of each other is what a default argument or a `std::optional` parameter
+replaces. The header symbol count is a Metric, compared with the base like function length.
+"""
+import re
+
+from .config import HEADER_LIMITS
+from .findings import Finding
+from .metrics import Metric
+from .style import (HEADERS, added_lines, default_split, internal, parameter_type, rule, scopes, signature,
+                    statements, strip_code, without_templates)
+
+SYMBOL = '<header>'
+TYPE_HEAD = re.compile(r'(?:template\s*<.*>\s*)?(?:enum(?:\s+(?:class|struct))?|class|struct|union)\b(.*)$')
+DECLARATION_KEYWORDS = re.compile(r'(using|typedef|namespace|template|class|struct|union|enum|friend'
+                                  r'|static_assert|return|extern\s*")\b')
+VALUE = re.compile(r'(?:[\w:]+(?:<.*>)?[\s\*&]+)+?[\*&\s]*([A-Za-z_]\w*)\s*(?:\[[^\]]*\])?')
+
+
+def public(enclosing):
+    """Namespace scope outside anonymous, `detail`, `internal` and `*Internal` namespaces."""
+    for kind, name in enclosing:
+        if kind == 'linkage':
+            continue
+        if kind != 'namespace' or not name or internal(name.split('::')[-1]):
+            return False
+    return True
+
+
+def normalized(text):
+    return ' '.join(text.replace('*', ' * ').replace('&', ' & ').split())
+
+
+# ---- Prefix overloads ---------------------------------------------------------------
+
+def overload_candidate(statement, enclosing):
+    """(scope, name, parameter types) of a declaration that may pair with a prefix overload."""
+    sig = signature(statement)
+    if sig is None or '::' in sig.name or sig.name.startswith('~') or 'operator' in statement.text:
+        return None
+    if re.search(r'=\s*delete\b', sig.tail) or any('...' in p for p in sig.params):
+        return None
+    if enclosing and enclosing[-1][0] == 'class' and sig.name == enclosing[-1][1]:
+        return None  # constructor
+    if not all(kind in ('namespace', 'linkage', 'class') for kind, _ in enclosing):
+        return None
+    params = tuple(normalized(parameter_type(p)) for p in sig.params if p.strip() not in ('', 'void'))
+    return enclosing, sig.name, params
+
+
+def prefix_overloads(path, source, previous_source=None):
+    """Errors for same-scope overloads whose parameter types are a strict prefix of another's.
+
+    At least one of the pair must be on a line added since `previous_source`."""
+    if not path.endswith(HEADERS):
+        return []
+    previous = normalize(previous_source).split('\n') if previous_source is not None else None
+    added = added_lines(normalize(source).split('\n'), previous)
+    found = []
+    for statement, enclosing in scopes(statements(strip_code(normalize(source)))):
+        candidate = overload_candidate(statement, enclosing)
+        if candidate:
+            found.append((candidate, statement.line))
+    errors, reported = [], set()
+    for (scope, name, short), line in found:
+        for (other_scope, other_name, long), other_line in found:
+            if (scope, name) != (other_scope, other_name) or len(short) >= len(long) \
+                    or long[:len(short)] != short:
+                continue
+            if line not in added and other_line not in added:
+                continue
+            at = max(line, other_line)
+            if (at, name) in reported:
+                continue
+            reported.add((at, name))
+            errors.append(Finding(path, at, rule(
+                f'{name}: overload ({", ".join(short)}) is a prefix of ({", ".join(long)}) at line '
+                f'{other_line if at == line else line}; use one function with a default argument '
+                'or a std::optional parameter', 'fork AGENTS.md "Functions"'),
+                check='overload_prefix'))
+    return sorted(errors, key=lambda f: f.line)
+
+
+# ---- Public names per header --------------------------------------------------------
+
+def type_name(text):
+    head = TYPE_HEAD.match(text)
+    if not head:
+        return None
+    rest = re.split(r'(?<!:):(?!:)', head.group(1))[0]
+    if '<' in without_templates(rest) or '<' in rest:
+        return None  # specialization of a template declared elsewhere
+    words = [w for w in re.findall(r'[A-Za-z_]\w*', rest) if w != 'final']
+    return words[-1] if words else None
+
+
+def declared_name(statement):
+    """The namespace-scope name a statement declares, or None."""
+    text = statement.text
+    if statement.end == '}' or not text or (statement.follows == 'class' and statement.end == ';'):
+        return None  # the variable in `struct{...} name;`, rare at namespace scope
+    alias = re.match(r'(?:template\s*<.*>\s*)?using\s+([A-Za-z_]\w*)\s*=', text)
+    if alias:
+        return alias.group(1)
+    if text.startswith('typedef'):
+        names = re.findall(r'([A-Za-z_]\w*)\s*\)?\s*(?:\(.*\))?\s*(?:\[[^\]]*\])?$', text)
+        return names[-1] if names else None
+    if statement.end == '{':
+        named = type_name(text)
+        if named:
+            return named
+    elif TYPE_HEAD.match(text):
+        return None  # forward or opaque declaration
+    sig = signature(statement)
+    if sig is not None:
+        if '::' in sig.name or 'operator' in text:
+            return None
+        return sig.name
+    head = re.sub(r'^template\s*<.*?>\s*', '', text)
+    head = default_split(head.rstrip('=').strip() if statement.end == '{' else head)[0]
+    if DECLARATION_KEYWORDS.match(head) or '(' in without_templates(head):
+        return None
+    value = VALUE.fullmatch(head)
+    return value.group(1) if value else None
+
+
+def public_names(source):
+    """{name: first line} of distinct public namespace-scope names in a header."""
+    names = {}
+    for statement, enclosing in scopes(statements(strip_code(normalize(source)))):
+        if not statement.namespace_scope or not public(enclosing):
+            continue
+        name = declared_name(statement)
+        if name and name not in names:
+            names[name] = statement.line
+    return names
+
+
+def measure_header(path, source):
+    if not path.endswith(HEADERS):
+        return []
+    count = len(public_names(source))
+    return [Metric(path, SYMBOL, 'header_symbols', count, HEADER_LIMITS['header_symbols'], 1)]
+
+
+def normalize(source):
+    return source.replace('\r\n', '\n').replace('\r', '\n')

devtools/code_health/style.py

@@ -274,6 +274,22 @@ def rule(message, *cites):
 RETURN_MOVE = re.compile(r'\breturn\s+std::move\s*\(\s*([A-Za-z_]\w*)\s*\)\s*;')
 QT_INCLUDE = re.compile(r'^\s*#\s*include\s*<Q', re.M)
 CONTROL = re.compile(r'\b(if(?:\s+constexpr)?|for|while|switch)\s*\(')
+# `char` followed by a pointer or a named array: `const char*`, `char const*`, `char**`, `char buf[8]`.
+CHAR_TYPE = re.compile(r'\bchar\b(?=\s*(?:const\s*)?\*|\s+(?:const\s+)?[A-Za-z_]\w*\s*\[)')
+MAIN = re.compile(r'\bmain\s*\(')
+
+
+def char_pointers(line):
+    """Columns where `line` names a `char*` / `char[]` type; not `unsigned char`, casts or main()."""
+    if MAIN.search(line):
+        return []
+    found = []
+    for match in CHAR_TYPE.finditer(line):
+        before = line[:match.start()]
+        if re.search(r'\b(un)?signed\s+$', before) or re.search(r'_cast\s*<[^<>]*$', before):
+            continue
+        found.append(match.start())
+    return found
 
 
 def function_start(bodies, line):
@@ -296,6 +312,11 @@ def line_checks(path, raw, code, added, statements_):
         for pattern, exempt, message, *cites in BANS:
             if pattern.search(line) and not path.startswith(exempt or ('\0',)):
                 errors.append(Finding(path, number, rule(message, *cites)))
+        if char_pointers(line):
+            errors.append(Finding(path, number, rule(
+                'no char* / char[] in new code; use std::string, or std::string_view '
+                '(inline constexpr std::string_view for constants)', 'fork AGENTS.md "Strings"'),
+                check='char_pointer'))
         if BIND.search(line):
             errors.append(Finding(path, number, rule('use a lambda, not std::bind', 'TotW #108')))
         if not allocator and (NEW.search(line) or DELETE.search(line)):