C++ Style

The C++ coding style for MBO Works repositories. It sits on top of two
machine-enforced config files and adds the human conventions below. When in doubt
the config files win; this document explains and extends them so a contributor (or
an AI assistant) can follow them without reverse-engineering the tooling.

Toolchain (machine-enforced)

• C++23, compiled with clang. Compilation with GCC is kept working on a
  best-effort basis.
• clang-format with .clang-format formats all C++ code. Run
  it; do not hand-format against it. CI rejects any reformatting diff.
• clang-tidy with .clang-tidy (WarningsAsErrors: '*') runs on
  changed C++ files through normal commit and push hooks. Changed headers select affected
  translation units. Generate the compile database with
  ./compile_commands-update.sh; the hook needs clang-tidy 22
  or newer and reports a skip when the tool or database is absent. CI supplies both and treats
  findings as errors. Checks never apply automatic fixes. A full scan is a separate manual
  invocation; see CONTRIBUTING.md for scope and parallelism.

What .clang-format decides (do not fight it)

• Google base style, column limit 120.
• AlignAfterOpenBracket: AlwaysBreak + BinPackParameters: false: when arguments
  do not fit, each goes on its own line at one indent (not aligned to the paren).
• PointerAlignment: Left (int* p), QualifierAlignment: Left (const int).
• IntegerLiteralSeparator decimal every 3: write 1'000'000.
• RemoveSemicolon, InsertBraces (always brace bodies), SeparateDefinitionBlocks.
• InsertTrailingCommas is deliberately OFF (it would force every aggregate to
  never-bin-pack). That is the lever behind the trailing-comma rule below: a manual
  trailing comma opts a single aggregate into one-element-per-line.

Source text uses ASCII except for the explicitly allowed graphical glyphs listed in RULES.md.
Spell other intentional Unicode string/character data with escapes and explain the
purpose in English. Raw strings do not decode escapes: construct Unicode expectations explicitly
when their exact code points matter. Dedicated Unicode test files may use literal visible samples.

Unicode is not inherently unwanted; unnecessary mixing of test purposes is. Tests specifically
about Unicode handling belong in separate Unicode-specific files explicitly excluded from the
ASCII-source check. Ordinary functional fixtures stay ASCII unless an approved glyph is the
actual output under test: histogram blocks and tree connectors belong in their functional tests.
Visible Unicode samples may be literal in dedicated test files; retain escapes for invisible
combining marks, joiners, or exact byte sequences, and explain the property each sample exercises.

Naming (enforced by .clang-tidy readability-identifier-naming)

• Types / classes / structs / enums / aliases / functions: CamelCase (LimitedMap).
• Variables / parameters / data members / namespaces: lower_case (flag_count).
• Private data members: lower_case with a trailing _ (flags_).
• constexpr / enum constants / global + static constants: k + CamelCase (kMaxDepth).
• Macros: UPPER_CASE, prefixed XFF_ (XFF_...). (A macro you call from the mbo
  dependency keeps its own MBO_ prefix, e.g. MBO_RETURN_IF_ERROR; the rule is about
  macros xff itself defines.)

Code organization

• All exported code lives under the top library directory (in xff: xff/).
• A cc_library target is named <thing>_cc (glob_cc, license_cc, vfs_cc), so a
  label reads as "the C++ library for <thing>" and never collides with the directory,
  header, or binary of the same name. cc_test keeps its _test suffix, and a cc_binary
  keeps the plain program name (xff). Enforced by the check-cc-target-naming pre-commit
  hook (tools/check_cc_target_naming.py), so the
  convention cannot drift back the way four extras targets did.
• Every cc_library has a test in its own package depending on it. This is the
  Testing section's "all exported code must be tested" stated as a build rule, and it is
  enforced by the check-cc-library-tested pre-commit hook
  (tools/check_cc_library_tested.py). Two things
  deliberately do not count: transitive coverage (a test of a library that merely
  deps yours) and a non-test dependent (a cc_binary, a bashtest) - neither exercises
  your unit directly. Same-package is part of the rule too: the test belongs next to the
  code it covers. A library with genuinely no testable surface goes into the tool's
  _ALLOWLIST with its reason recorded there (today's single entry is
  xff/cli:main_cc, which is main() plus argv wiring and is covered end to end by the
  xff / xff_full bashtests).
• Each subdirectory uses namespace xff::{dir} (and xff::{dir}::{sub}).
  • Internal-only code: xff::{dir}::{sub}_internal. No namespace component is ever
    just internal.
  • Exported-but-detail code may use a detail sub-namespace (fully qualify it, since
    the bare detail name can collide).
• Root globally owned namespaces explicitly in using-declarations and namespace aliases. This
  applies uniformly to the standard library, foundational dependencies, and project namespaces:
  write using ::std::size_t;, using ::absl::Status;, using ::mbo::testing::IsOk;, and
  namespace fs = ::std::filesystem;. The leading :: makes lookup unambiguously start at the
  global namespace. Keep all such declarations in one block without library-specific blank lines,
  sorted lexicographically. Relative qualification remains correct for a name owned by the current
  namespace and for dependent imports such as using Ts::operator()...;; this rule does not affect
  type aliases such as using Value = mbo::types::Value;.
• Only the standard library owns a namespace named std. Project code never declares or
  reopens ::std, never declares a nested namespace such as xff::std, and never writes
  using namespace ::std;. A standards-sanctioned customization point must instead be an explicitly
  qualified specialization at global scope, for example template <> struct std::hash<MyType>.
  Such extension points are exceptional and should be avoided where a project-owned customization
  mechanism is possible. The check-namespace-style pre-commit hook enforces namespace ownership
  and absolute qualification.
• Header guards: {PATH}_{FILE}_ (path + filename, uppercased, non-alphanumerics ->
  _, trailing _). A file may not start with a sibling directory's basename + _:
  foo/bar.h and foo_bar.h would both yield FOO_BAR_H_.
• Forward-declared symbols must be fully implemented in the same source file (except
  the Impl-in-header / implementation-in-.cc pattern).
• Macros: avoid them. An exported macro (defined in a header for other code to use) is
  prefixed XFF_ (private impl XFF_PRIVATE_...). A macro local to one translation unit
  • a .cc / test file, or a header helper you #undef right after its last use - may stay
    unprefixed (still UPPER_CASE); #undef a local macro as soon after its last use as
    possible.
• C-library names: use the std:: form; the unqualified global alias is optional. A
  <cstddef> / <cXXX> header is only required to declare size_t, int64_t, memcpy,
  ... in namespace std; whether it also injects the bare global name is
  implementation-defined, so we never rely on it.
  • In headers, always fully qualify (std::size_t); no namespace-level using.
  • In an implementation file (.cc), a using ::std::size_t; (or other using ::std::...)
    is fine where it reads better - bring the name in explicitly rather than leaning on the
    global alias. Be consistent within a file, but readability outranks consistency.
• Library flags are prefixed by their path/namespace (e.g. a flag in xff/<pkg> is
  --xff_<pkg>_...). The application entry-point binary (xff, also invoked as find) uses
  bare flag names (--config, --summary, --define).

Formatting conventions on top of clang-format

clang-format picks a layout per line; these habits steer it toward the readable one.

1. No comment at the end of a long line. Put the comment on its own line above
   the element. A trailing // ... that pushes a line past 120 makes clang-format
   explode the element across several lines.

   // -exec run in the matched entry's directory
   {.name = "-execdir", .kind = Kind::kAction, .arity = -1},

2. Trailing comma on the last field of a complex aggregate breaks it one field per
   line. Because InsertTrailingCommas is off, the comma is your per-aggregate opt-in.
   Use it for long/complex initializers; a short one that reads on a single line stays.
   This applies to named variables, return values, and structs passed directly as call arguments.
   Each multiline designated initializer needs its own trailing comma inside its closing brace.
   Review nested initializers independently: a comma after an inner closing brace separates outer
   elements or arguments and does not replace the inner trailing comma.

   Emit(Render(
       {
           .name = name,
           .options = {
               .width = width,
               .enabled = enabled,
           },
       },
       format));

   const Drop drop{
       .line = line,
       .layer = Source::kProject,
       .safety = Safety::kSecurity,
   };

   In a long array of struct literals - a registry-style table such as kGlobals or
   kDescriptors - put the trailing comma on every element, even the short ones that
   would fit on a single line, so clang-format expands the whole table uniformly, one
   field per line. A consistent table you scroll through reads better than a mix of
   one-liners and exploded rows packed to save height. This stays a deliberate, per-table
   choice made element by element: InsertTrailingCommas is off (we do not always want
   trailing commas), so clang-format never forces it for you.

3. Force a line break with a comment rather than let clang-format cram a value at the right
   margin. A long argument - especially a raw string such as a proto R"pb(...)pb" - otherwise
   gets packed onto the call line and shoved against the 120 column, unreadable. A trailing
   comment makes clang-format keep the element on its own line. Mark it // NL ("new line"):

   EXPECT_THAT(  // NL
       message,
       EqualsProto(R"pb(name: "n" value: 1)pb"));

   When there is a relevant reason, keep the prefix and add it: // NL: <short reason>, preferred
   over a bare // NL. Always keep the NL prefix - do not drop to a bare // or an unprefixed
   comment - for two reasons: it marks the comment as load-bearing for layout, so a reader knows
   that removing it re-crams the line; and the consistent marker is machine-checkable, so a
   pre-commit rule can verify these lines stay broken.

   Reserve // clang-format off / on for a genuine table or hand-aligned expression that
   clang-format cannot lay out - // NL only inserts breaks, so it cannot keep an over-120
   line whole or stop a reflow (e.g. a one-line-per-case test table, or a multi-clause
   requires(...)). Do not use it to hand-place ordinary layout, and keep the off/on pair
   tight and adjacent so the on is never forgotten: everything between the two loses every
   formatting guarantee above.

Idioms

• Pass absl::Status by value, not const&: it is a tagged pointer, and
  .clang-tidy performance-unnecessary-value-param allowlists it (with absl::StatusOr
  and std::string_view). StatusOr<T>'s cost depends on T.

• Short literal ranges may be inline when the complete for (...) header fits on one
  clang-formatted line (the 120-column limit includes indentation). For example:

  for (const auto value : {1, 2, 3}) {
    Check(value);
  }

  Name the collection when the header would wrap, when it is reused, or when a name explains
  the values' meaning. A typed wrapper such as std::to_array<T>({...}) does not name a list;
  it follows the same one-line rule. Do not compress a long list or disable formatting to evade it.
  Only the header matters: a multiline loop body does not require naming the range.

  For named constant data, prefer static constexpr std::array initialized with
  std::to_array<T>({...}). Give it a meaningful k name and a trailing comma on its final
  element. Use local scope for a single consumer, or anonymous-namespace scope when shared.
  Runtime values need an ordinary appropriately scoped collection, not forced constexpr.

  static constexpr std::array kLocaleVariablesByPrecedence = std::to_array<std::string_view>({
      "LC_ALL",
      "LC_CTYPE",
      "LANG",
  });
  for (const std::string_view variable : kLocaleVariablesByPrecedence) {
    ReadLocale(variable);
  }

  Element typing is a separate decision: std::to_array<T> explicitly chooses the stored
  element type; a braced list of string literals normally stores const char*, even if the
  loop variable is a std::string_view. Choose the type deliberately, without inventing a name
  solely for a short one-use list.

  The check-inline-range-lists hook checks multiline/overlong literal range headers, including
  common typed wrappers. Semantic reasons to name a short list (meaning or reuse) remain a review
  decision. Other braced arguments, aggregates, and ordinary computed ranges are unaffected.

• Prefer container algorithms from absl/algorithm/container.h (absl::c_contains,
  c_any_of, c_find, c_equal, c_sort, ...) or C++23 std::ranges over hand-rolled
  loops: range-based (no begin/end), well-named, constexpr-friendly. A loop that returns
  on the first match is an absl::c_any_of with a lambda; a reserve-then-push copy into a
  std::vector is range construction std::vector<T>(src.begin(), src.end()). Keep a raw
  loop only when it builds a non-trivial structure no algorithm expresses cleanly.

• When you do write a loop, it is a RANGE-BASED for; an index loop needs a reason. The
  legitimate reasons are narrow: you need the index itself as a value, or you walk two parallel
  sequences in lockstep (specs[i] with cells_per_sink[i]). "I need the elements" is not one -
  for (const Foo& foo : foos) says what the loop visits, cannot run off the end, and does not
  invite foos[i] typos. modernize-loop-convert catches the mechanical cases; the judgement
  ones are on you.

• Never index in order to look BACK at earlier elements - carry the state instead. Re-scanning
  the prefix is both the wrong shape (it hides an O(n^2) in what reads like a single pass) and
  unnecessary: a seen set or map answers "have I met this already?" in one range-based pass.

  absl::flat_hash_set<std::string_view> seen;                    // one pass, O(n), says what it means
  for (const CollectSite& site : CollectSites(expr)) {
    if (!seen.insert(site.name).second) return site.name;
  }
  for (std::size_t i = 1; i < sites.size(); ++i) {               // no: index only to re-scan [0, i)
    if (absl::c_count(names, names[i]) > 1) return names[i];     //     and quadratic while at it
  }

• Container choice: prefer the Abseil containers over the bare std:: ones. The reason is
  in their favor, not against std::: the Abseil variants are faster, support transparent
  (heterogeneous) lookup - find in a std::string-keyed map with a std::string_view, no
  temporary std::string - and flat_hash_* stores elements inline for lower memory.

  • Never std::unordered_* - no <unordered_map> / <unordered_set> include, and none
    of unordered_map / unordered_set / unordered_multimap / unordered_multiset. Use
    absl::flat_hash_map / flat_hash_set, or the node_hash_map / node_hash_set variants
    when you need pointer/reference stability. This holds in tests too - the one exception is a
    container-compatibility test that deliberately exercises a std::unordered_* input.
  • Avoid std::map / std::set (and the multi variants) in library code; prefer
    absl::btree_map / absl::btree_set, which keep the same ordered interface but are
    cache-friendlier. In tests, std::map / std::set are fine.
  • Small compile-time tables: mbo::container::LimitedMap / LimitedSet.
  • A type-detection trait may name a std:: container type freely - it must, to recognize it.

• A read-only string parameter is std::string_view (by value), not const std::string&.
  This is pretty much always: a std::string_view binds to a std::string, a string literal,
  a char*, or another view with no allocation and no .c_str() dance, so the const& only
  narrows what callers may pass. Keep const std::string& (or std::string by value) only
  when the body genuinely needs a std::string - it calls .c_str() for a C API, stores or
  moves the argument, or passes it to something that itself wants const std::string&.

• A by-value std::string_view follows ordinary const-correctness: const when you never
  mutate the view, non-const only when you do. The characters it views are already const,
  but the view itself is a normal local: const documents that you never reslice or reassign it,
  and misc-const-correctness (enabled) flags a never-mutated view that is not const. Drop the
  const exactly when you mutate the view in place - remove_prefix, remove_suffix, or
  reassignment. (This concerns the view; not std::string_view::size_type, which is a size_t,
  nor absl::Span<const std::string_view>, where const is the span's element type.)
  Corollary: when a loop view needs trimming, iterate a mutable (non-const) by-value view and
  mutate it in place - never const the loop view and then copy it to a mutable local just to
  mutate the copy.

  const std::string_view name = label.empty() ? id : label;      // read-only: const
  for (std::string_view line : absl::StrSplit(text, '\n')) {      // mutated: non-const, trim in place
    if (!line.empty() && line.back() == '\r') line.remove_suffix(1);
  }
  for (const std::string_view raw : absl::StrSplit(text, '\n')) {  // no: const + copy-to-mutate
    std::string_view line = raw;
    if (!line.empty() && line.back() == '\r') line.remove_suffix(1);
  }

• Xff-owned C++ interfaces do not use raw object pointers. A raw pointer mixes nullability,
  the pointed-at value, and ownership/lifetime into one type the caller has to second-guess. Pass a
  required observer by reference, ownership by value or smart pointer, and an optional observer as
  mbo's mbo::types::OptionalRef<T> (mbo/types/optional_ref.h, with its exact Bazel dependency)
  or a more precise value/view type. Do not return a nullable pointer for lookup; return
  OptionalRef<const T>. Do not use a pointer merely for stable identity; give the entity an
  explicit identity type. Function pointers are a distinct callable type and are not object
  ownership. Raw object pointers remain only where a C, POSIX, or third-party ABI requires their
  exact spelling (callbacks, opaque handles, buffers); confine those to the narrow adapter, convert
  on entry/return where possible, and mark each retained xff-object boundary with
  XFF_ABI_POINTER: <reason> on its declaration or immediately preceding line. The reason must name
  the external ABI; the annotation is not an allowlist for internal convenience pointers.

• Prefer side-effect-light, value-returning helpers. Do not mutate caller-owned state through
  pointer or reference output parameters. Return the value, an absl::StatusOr<T>, or a named
  result record when multiple values and diagnostics belong together. Mutation is appropriate
  when it is the explicit responsibility of a stateful abstraction or action - writers, caches,
  filesystem operations, and expression actions - but that state change must be visible in the
  abstraction rather than hidden in an otherwise value-shaped helper.

• A CHECK over a comparison uses the comparing macro: CHECK_NE(ptr, nullptr),
  CHECK_EQ(size, want), CHECK_LE(offset, end) - never CHECK(ptr != nullptr) or
  CHECK(size == want). The comparing form prints BOTH operands when it fires; the boolean form
  prints only the expression's source text, so a crash report says what was compared and never what
  the values were. Same for DCHECK / QCHECK and the ABSL_-prefixed spellings. A genuinely
  boolean condition (CHECK(matcher.ok()), CHECK(std::holds_alternative<T>(v))) stays a plain
  CHECK. Enforced by the no-comparison-in-check-macro pre-commit hook
  (tools/check_check_macros.sh).

• Mark a return value [[nodiscard]] when silently ignoring it is a bug - an
  absl::Status / StatusOr, a parsed result, a Consume... "did it match?" flag, an acquired
  handle. The exception is a function designed for its result to be optionally used: a builder
  method returning *this for chaining, or a mutator that also returns the previous value as a
  convenience. We disable modernize-use-nodiscard in .clang-tidy precisely because it would
  blanket-annotate every const method - apply [[nodiscard]] by judgment instead.

• Comparison functions name their parameters lhs and rhs - a sort comparator, an
  operator==/operator<=>, any two-things-of-one-type predicate. Never a/b (which
  readability-identifier-length rejects at under 2 characters) and not x/y either: lhs/rhs
  names the ROLE, so lhs.name < rhs.name reads as the ordering it implements.

  absl::c_sort(topics, [](const HelpTopic& lhs, const HelpTopic& rhs) { return lhs.name < rhs.name; });

• switch / case: order case labels alphabetically. Where the cases are a uniform
  mapping (key -> handler or value), prefer a constexpr mbo::container::LimitedMap
  lookup over a switch.

• No em-dashes anywhere (code, comments, docs, commit messages). Use a spaced
  hyphen -.

Error handling: absl::Status and the MBO status macros

Propagate errors with the macros from mbo/status/status_macros.h
(@mboworks_mbo//mbo/status:status_macros_cc), not a hand-written
if (!x.ok()) return x.status();.

• A "value or error" type IS absl::StatusOr<T>. Do not hand-roll a struct that bundles
  a value (or a std::vector / std::optional) with an absl::Status plus an "ok" flag:
  absl::StatusOr<T> is exactly that, enforces the not-ok-has-no-value invariant by
  construction, composes with the macros below, and matches every other API.
• MBO_RETURN_IF_ERROR(expr) evaluates a Status or StatusOr and returns early if
  it is not OK. It returns a mbo::status::StatusBuilder, which converts to the calling
  function's absl::Status or absl::StatusOr<T>, so it works in both.
• MBO_ASSIGN_OR_RETURN(Type var, expr) binds the value of a StatusOr<Type> to
  var (a new declaration - carry the type - or an existing variable) or returns the
  status. After it, var is the value (not a StatusOr); use var, not *var.
• MBO_MOVE_TO_OR_RETURN(expr, target) is the variant whose target may contain commas;
  the expression comes first so structured bindings work:
  MBO_MOVE_TO_OR_RETURN(MakePair(), auto [a, b]);.

absl::StatusOr<Report> Build(std::string_view path) {
  MBO_ASSIGN_OR_RETURN(const Config config, LoadConfig(path));  // value, or return status
  Report report = Analyze(config);
  MBO_RETURN_IF_ERROR(Persist(report));                         // Status guard
  return report;
}

• When you only need the guard but the StatusOr value is large and used later through
  *expr, guard on the status to avoid copying the value: MBO_RETURN_IF_ERROR(big.status());
  then use *big.
• Do not force the macros where they do not fit: a recovery branch (e.g.
  absl::IsNotFound(s) -> return a default value), a function that returns bool/int
  rather than a status, or a CLI path that prints a message and returns an exit code.
• Pass absl::Status by value (see Idioms).
• The hand-written shape this replaces is rejected by the no-hand-rolled-status-propagation
  pre-commit hook, which matches if (!x.ok()) { return x.status(); } and nothing else - so a
  return f().status(); that converts a StatusOr call into a Status return, and a recovery branch
  that inspects .ok() without propagating, both stay legal.

Output, logging, and AbslStringify

• Never use C strings or .c_str() except to satisfy a C / system API (e.g. building
  char* argv[] for execvp); comment why at that one boundary. Keep std::string /
  std::string_view end to end - the round-trip out to a const char* and back is
  clutter, not interop.

• Never write output through C stdio (printf / fprintf / fputs to the stdout /
  stderr FILE*). Use the C++ streams std::cout / std::cerr (or the logging library).
  The anti-pattern to delete on sight: std::fputs(absl::StrCat(...).c_str(), stderr).

• Format with Abseil: absl::StrCat for plain concatenation, absl::StrFormat for a
  formatted string, and absl::StreamFormat to write a formatted line straight to a
  stream with no temporary: std::cerr << absl::StreamFormat("wrote %d entries\n", n);.
  A plain << is fine for a simple string. Choose per line, but be consistent within a
  single function (do not mix a StreamFormat and a << chain in the same function).

• Make your types printable with AbslStringify, not a hand-rolled operator<< /
  ToString. Abseil's string-conversion extension point is a hidden-friend hook:

  struct Point {
    int x = 0;
    int y = 0;
    template<typename Sink>
    friend void AbslStringify(Sink& sink, const Point& p) {
      absl::Format(&sink, "(%d, %d)", p.x, p.y);
    }
  };

  One hook makes the type work with absl::StrCat, absl::StrFormat / StreamFormat
%v, absl::StrJoin, and Abseil logging - and gives GoogleTest readable failure output.
  absl::Status / StatusOr already stringify; print them with %v.

Concurrency and thread safety

These are the baseline for any multi-threaded code, not extra credit; threading bugs are
silent and data-dependent, so the annotations and the sanitizer are the standing guard.
Most of this repo is single-threaded today, so this section is the standard to apply when
you add multi-threaded code, not a description of current breadth.

• Use absl::Mutex + absl::MutexLock, not std::mutex / std::lock_guard or
  atomics-as-synchronization. Construct the lock from a reference: absl::MutexLock lock(mu_);
  (the pointer constructor is deprecated).
• Annotate everything (absl/base/thread_annotations.h): ABSL_GUARDED_BY(mu_) on
  every member the mutex protects, and ABSL_LOCKS_EXCLUDED /
  ABSL_EXCLUSIVE_LOCKS_REQUIRED on methods. A comment must state exactly which members a
  given mutex guards, and call out any shared state deliberately left unguarded together
  with the invariant that keeps it safe (e.g. "each index is handed to exactly one worker,
  so distinct elements never alias").
• If a type has more than one mutex, document their lock-acquisition order (which is
  taken before which) so the ordering that prevents deadlock is explicit.
• Enforce the annotations: compile first-party code with clang's -Wthread-safety
  (with -Werror an unguarded access becomes a build failure), and add a
  ThreadSanitizer CI job that runs the threaded tests so races are also caught at
  runtime. Scope the tsan job to the threaded targets; exclude heavy third-party-linked
  ones so tsan does not rebuild them.

Protocol Buffers

• Generated repeated fields and maps are STL-compatible - range-iterate them, do not
  index with field_size() + field(i). field_size() is still fine for reserve() and
  == 0 emptiness checks; compare two repeated fields with absl::c_equal.
• Edition 2024: string accessors default to string_type = VIEW and return
  std::string_view (so the by-value-string_view rule applies to them); singular fields
  have explicit presence by default, so set_x(0) serializes a 0. To mean "absent",
  leave the field unset (never call its setter) rather than setting the default.
• Build test protos from text, not setters:
  mbo::proto::ParseTextProtoOrDie(R"pb(field: 1 nested { k: "v" })pb")
  (@mboworks_proto//mbo/proto:parse_text_proto_cc). The R"pb(...)pb" raw string is what
  clang-format leaves alone, so the proto stays readable. Do not imperatively set_ / add_
  your way to a fixture.
• Assert on protos structurally with mbo::proto::EqualsProto
  (@mboworks_proto//mbo/proto:matchers_cc), which also accepts a text-proto string:
  EXPECT_THAT(msg, EqualsProto(R"pb(field: 1)pb"));. Match a subset with
  Partially(EqualsProto(...)). Never compare serialized strings.

Testing (GoogleTest / GoogleMock)

All exported code must be tested, at every level (unit, integration, and end-to-end where
it applies). A one-shot manual check or a script you ran once is planning input, never a
substitute for a committed test. Tests use GoogleTest + GoogleMock with these conventions.

Structure

• Always TEST_F with a fixture, never a bare TEST. Even an empty
  struct FooTest : ::testing::Test {}; is preferred, so shared setup has a home.
• One behaviour per test; name the test for the behaviour it asserts.

Assertions: matchers only; convenience macros are forbidden

• Assert with EXPECT_THAT / ASSERT_THAT + a matcher. The complete forbidden set is either
  EXPECT_ or ASSERT_ combined with EQ, NE, LT, LE, GT, GE, STREQ, STRNE,
  STRCASEEQ, STRCASENE, FLOAT_EQ, DOUBLE_EQ, or NEAR. There are no comparison-macro
  exceptions. EXPECT_TRUE / EXPECT_FALSE and ASSERT_TRUE / ASSERT_FALSE are also forbidden;
  use IsTrue / IsFalse so boolean checks follow the same matcher-only rule. The
  no-comparison-macros-in-cc-tests pre-commit hook
  (tools/check_test_matchers.sh) enforces this in all C++ files.
  Use NotNull for pointers, Lt / Le / Gt / Ge for ordering, StrEq where C-string value
  semantics are needed, floating-point matchers for approximate values, and EqualsText for text.

• Name matchers unqualified - never the ::testing:: prefix inline. Bring each matcher in with
  a using ::testing::Foo; (or using ::mbo::testing::Foo;) in the test file's anonymous namespace
  and use the bare name in the EXPECT_THAT / ASSERT_THAT expression; a ::testing::Foo(...)
  written inline in an assertion is the smell to fix by adding the using. (Fixture utilities such
  as ::testing::Test / ::testing::TempDir are not matchers and keep their qualification.)

• Eq is optional - a readability choice, not a rule. EXPECT_THAT(x, value) auto-wraps a
  bare value in Eq, so both forms compile. The value of EXPECT_THAT is that the line reads as
  a sentence - EXPECT_THAT(foo, Eq(25)) is "expect that foo equals 25" - so keep Eq where it
  makes the assertion read that way, and drop it where the value already carries the meaning
  (EXPECT_THAT(name, "foo")). Inside composite matchers prefer bare elements
  (ElementsAre(1, 2, 3)); Optional / Pointee are the exception and need the inner Eq (see
  below). Strings sometimes need StrEq (e.g. a char* subject, where bare Eq compares
  pointers). For booleans, IsTrue() / IsFalse() usually read better than a bare true /
  false or Eq(true): EXPECT_THAT(found, IsTrue()).

• Multi-line text: use mbo::testing::EqualsText. For a multi-line string
  (a rendered table, generated --help, file contents) prefer
  EXPECT_THAT(actual, EqualsText(golden)) (@mboworks_mbo//mbo/testing:matchers_cc): it compares
  line by line with unified-diff output, so a mismatch points at the offending line instead of
  dumping the whole blob. Use EqualsText for all text comparisons, including text that is not
  naturally line-oriented.

  • Write the golden as a DropIndent-filtered raw string, not concatenated "...\n" literals.
    clang-format shoves adjacent string literals hard against the EqualsText( bracket (aligned to
    the open paren, often at column ~35), which is unreadable and drifts with the call length. Instead
    write the expected block as an indented raw string and strip the source indent with
    WithDropIndent, which clang-format leaves untouched:

    using ::mbo::testing::EqualsText;
    using ::mbo::testing::WithDropIndent;
    EXPECT_THAT(RenderTable(Format::kAligned, header, rows), WithDropIndent(EqualsText(R"out(
        name    size
        ------  ----
        a.txt   3
        )out")));

    mbo::strings::DropIndent (which WithDropIndent applies to the expected text only) drops the
    empty first line after R"out(, strips the first content line's indent from every line, and clears
    a whitespace-only last line - so the block reads as the literal expected output. The subject stays
    as-is; only de-indent it too (EXPECT_THAT(DropIndent(actual), WithDropIndent(EqualsText(golden))),
    @mboworks_mbo//mbo/strings:indent_cc) when the actual is itself an indented literal. Use any raw
    delimiter (R"out(, R"md(); a raw string also needs no \n / \\ escaping.

  • Caveat - trailing whitespace. A raw-string golden cannot carry significant trailing spaces on
    a line: the trim trailing whitespace pre-commit hook strips them, silently changing the golden. If
    the expected output has meaningful trailing whitespace (e.g. right-padded columns), fall back to the
    concatenated-literal form for that case and accept the paren alignment.

  • DropIndentAndSplit returns the de-indented lines as a std::vector<std::string_view> for when you
    would rather match them with ElementsAre.

• Floats / doubles: never Eq / ==; use FloatEq / DoubleEq (or Near).

• Optionals: match the optional-like object itself - never reduce it to
  opt.has_value(). Use EXPECT_THAT(opt, Eq(std::nullopt)) for empty and Optional(...) for a
  value; this applies equally to std::optional and mbo::types::OptionalRef. Nested matchers do
  not auto-wrap: Optional(Eq("x")) and Pointee(Eq("x")) need the explicit inner Eq (a
  bare value fails to compile there). absl::StatusOr uses IsOkAndHolds(...) instead.

• Containers: ElementsAre(...) / UnorderedElementsAre(...) / SizeIs(n) /
  IsEmpty() cover size + order + contents in one matcher, instead of an
  ASSERT_EQ(v.size(), n) followed by indexed EXPECT_EQs. ElementsAre auto-wraps
  each bare element in Eq.

  • For a longer matcher list, prefer ElementsAreArray({m1, ..., mN,}) with a
    trailing comma over ElementsAre(m1, ..., mN). ElementsAre is variadic, so
    clang-format bin-packs it into an unreadable run; ElementsAreArray takes a
    braced init-list, so the manual trailing comma opts it into one-matcher-per-line
    (the same trailing-comma lever as any aggregate, see the Formatting section).
    A short list that reads on one line stays ElementsAre.

• Size and emptiness: match the container, never extract then match. Use
  SizeIs / IsEmpty on the container itself, not .size() / .empty() fed to a
  scalar matcher or EXPECT_TRUE - the matcher form prints the container on failure
  while the extracted form throws it away. So:

  • EXPECT_THAT(v, SizeIs(3)), not EXPECT_THAT(v.size(), 3) / EXPECT_EQ(v.size(), 3).
  • SizeIs composes with a matcher for bounds: EXPECT_THAT(v, SizeIs(Le(90))),
    not EXPECT_THAT(v.size(), Le(90)).
  • EXPECT_THAT(v, IsEmpty()) / EXPECT_THAT(v, Not(IsEmpty())), not
    EXPECT_TRUE(v.empty()) / EXPECT_FALSE(v.empty()) (this is the one place a
    .empty() boolean should still become a matcher).

• Struct elements: fold per-field checks into the element matcher with the
  3-arg, named Field("member", &T::member, m) + AllOf, ideally via a small
  testing::Matcher<T> FooIs(...) helper, so a mismatch names the field rather than
  reporting "whose given field is...".

  testing::Matcher<ResolvedFlag> FlagIs(const std::string& flag, Source source) {
    return AllOf(Field("flag", &ResolvedFlag::flag, flag), Field("source", &ResolvedFlag::source, source));
  }
  EXPECT_THAT(Resolve(in), ElementsAre(FlagIs("--color", Source::kUser), FlagIs("--sort", Source::kUser)));

Status matchers (mbo/testing)

• EXPECT_THAT(status_or, mbo::testing::IsOk()), or
  StatusIs(absl::StatusCode::kInvalidArgument) to match a specific code.
• IsOkAndHolds(m) matches an OK StatusOr whose value matches m - prefer it over
  IsOk() followed by dereferencing: EXPECT_THAT(Parse(in), IsOkAndHolds(SizeIs(3))).
• MBO_ASSERT_OK_AND_ASSIGN(const auto value, MakeThing()) asserts OK and binds in one step.
• Never ASSERT_THAT(x, IsOk()) on a StatusOr local you then dereference (*x, x->,
  x.value()): that splits one fact over two statements, and every later use re-dereferences a
  value whose validity hangs on an assertion lines above. Bind it once with
  MBO_ASSERT_OK_AND_ASSIGN(const T x, expr) - the local's type becomes the payload - or say the
  whole thing in one matcher with IsOkAndHolds(m) when the value is inspected once. In a loop
  body, put the iteration context in SCOPED_TRACE(item) (the macro takes no << stream).
  ASSERT_THAT on a plain absl::Status, or on a StatusOr never dereferenced, stays correct.
  Enforced by the no-isok-assert-then-deref pre-commit hook
  (tools/check_status_assert.py) - this held as prose-only
  guidance for exactly 78 violations before the hook existed.
• MBO_ASSERT_OK_AND_MOVE_TO(MakePair(), auto [a, b]) is the move variant whose target may
  contain commas (so the expression comes first) - the test mirror of MBO_MOVE_TO_OR_RETURN,
  for structured bindings and move-only types.

Protocol-buffer fixtures and matchers

Build proto test data with mbo::proto::ParseTextProtoOrDie(R"pb(...)pb") and assert with
mbo::proto::EqualsProto / Partially(EqualsProto(...)) - never imperative setters or
serialized-string comparison. See the Protocol Buffers section.

Shell / binary-level tests

• Use mboworks/bashtest (bazel_dep(name = "mboworks_bashtest", repo_name = "com_mboworks_bashtest"),
  whose macro emits @com_mboworks_bashtest labels), not a hand-rolled sh_test:
  load("@com_mboworks_bashtest//bashtest:bashtest.bzl", "bashtest"), then a
  script that sources "${mboworks_bashtest}", defines test::name() functions using
  the expect_* assertions, and ends with test_runner.
• Assert on captured output with bashtest's content matchers (>= 0.5.0), never a
  hand-rolled grep. expect_output_contains / expect_output_not_contains for a
  literal substring; expect_matches / expect_not_matches for an ERE. They take the
  pattern / substring first and the text second, and match via bash's built-in
  [[ =~ ]] (no subprocess), so - unlike printf ... | grep -qE - they cannot misfire
  on SIGPIPE under set -o pipefail. expect_eq / expect_ne stay for scalar checks
  (exit codes, counts); expect_contains / expect_not_contains are array-membership,
  not substring. Do not reintroduce a _has-style grep -q wrapper. This is enforced
  by the no-shell-grep-in-bashtests pre-commit hook (tools/check_bashtest_grep.sh),
  which fails on a shell grep at a command position in any *_test.sh (the -grep
  primary under test and prose are not flagged).
  • Anchoring caveat: expect_matches matches the whole text, so ^ / $ anchor
    the whole output, not a line (unlike grep). For a per-line anchor use a real newline:
    NL=$'\n', then (^|${NL})X for a line start and X($|${NL}) for a line end (\n is
    not a portable ERE escape, so embed the newline via ${NL}).
• For a whole-output-per-case golden, prefer a golden-file test over scraping. When a
  test wants to lock an entire rendered output (not just probe for substrings), commit one
  expected-output file per case and diff against it with mbo's diff.bzl diff_test
  (@mboworks_mbo//mbo/diff:diff.bzl), which fails printing the offending diff. xff's
  diff_golden.bzl macro is the pattern: a single input generator plus a {case: golden}
  dict, each case run through the built binary into a normalized .actual that diff_test
  compares. This is the output analogue of EqualsText for C++ and reads far better than a
  pile of expect_output_contains probes.
• It runs under macOS bash 3.2: no mapfile / readarray or other bash-4 features.
  Read lines with while IFS= read -r line; do arr+=("$line"); done <<< "$out".