diff --git a/CLAUDE.md b/CLAUDE.md index fc846aea83..201d0dd1a3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -250,7 +250,7 @@ For path/filename ops use `fio` helpers (`base_name`/`dir_name`/`path_join`/...) **Complexity/length lint (STYLE037/STYLE038): new code meets both limits from the start.** On a hit in existing code, the suppress-vs-split resolution policy is `skills/style_lint.md` - never force a split on an honest shape. -**Comment hygiene, all languages: `skills/comment_style_hygiene.md` is the rulebook.** For `.das` outside tutorials/examples the headline is that no comment outside the skill's kept set survives to a commit - scaffolding is welcome while you work, and the PR gate drains it (`skills/internal/make_pr.md`, the Comment drain row). For C/C++ it is no NEW comments. The kept sets, teaching-code boundary, naming and code-shape rules all live in the skill; the `style-hygiene-auditor` agent applies them to every PR's new code (mandatory run in `skills/internal/make_pr.md`, findings persuade rather than block). +**Comment hygiene, all languages: `skills/comment_style_hygiene.md` is the rulebook.** For `.das` in a folder whose `.lint_config` arms `force_clean_comments` the headline is that no comment outside the skill's kept set survives to a commit - scaffolding is welcome while you work, and the PR gate drains it (`skills/internal/make_pr.md`, the Comment drain row); elsewhere the kept set still holds and every other `.das` comment answers to the skill's deletion test. For C/C++ it is no NEW comments. The kept sets, teaching-code boundary, naming and code-shape rules all live in the skill; the `style-hygiene-auditor` agent applies them to every PR's new code (mandatory run in `skills/internal/make_pr.md`, findings persuade rather than block). ## Key Directories diff --git a/CMakeLists.txt b/CMakeLists.txt index 16960cb471..0024941ddd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1854,6 +1854,8 @@ install(DIRECTORY ${PROJECT_SOURCE_DIR}/skills/ # skills/daslang_live.md (prefix match on the full path) REGEX "/skills/internal$" EXCLUDE REGEX "/skills/daslang$" EXCLUDE + # ruling provenance for this repo's rule documents, not a skill + REGEX "/LAWS\\.md$" EXCLUDE PATTERN "*.md" ) # its README is editing rules for this repo's maintainers, meaningless in the SDK @@ -1861,6 +1863,7 @@ install(DIRECTORY ${PROJECT_SOURCE_DIR}/skills/daslang/ DESTINATION ${DAS_INSTALL_DOCDIR}/skills/daslang FILES_MATCHING REGEX "/daslang/README\\.md$" EXCLUDE + REGEX "/LAWS\\.md$" EXCLUDE PATTERN "*.md" ) # The review constitution ships with the review skills that route to it. @@ -1959,6 +1962,7 @@ install(DIRECTORY ${PROJECT_SOURCE_DIR}/skills/daslang/ DESTINATION .claude/skills/daslang FILES_MATCHING REGEX "/daslang/README\\.md$" EXCLUDE + REGEX "/LAWS\\.md$" EXCLUDE PATTERN "*.md" ) install(FILES ${PROJECT_SOURCE_DIR}/.claude/agents/dragon.md DESTINATION .claude/agents) diff --git a/ci/check_shipped_skills.py b/ci/check_shipped_skills.py index 1b73b2d79d..b1e2e3d962 100644 --- a/ci/check_shipped_skills.py +++ b/ci/check_shipped_skills.py @@ -2,7 +2,8 @@ """Gate the skills shipped into an SDK bundle. The FOLDER is the shipping decision: everything under skills/ ships except -skills/internal/, and skills/daslang/ is the standalone language skill. So +skills/internal/ and the NOT_A_SKILL names below, and skills/daslang/ is the +standalone language skill. So anything a shipped skill says has to be true *from inside the bundle*. A skill that points at src/, links a file that is not there, or tells the reader to run bin/Release/daslang.exe is a dead end for an SDK user -- and for Claude running @@ -63,6 +64,10 @@ MACHINE_PATH = re.compile( r"(?:[A-Za-z]:[\\/](?:Users|Work|DASPKG)\b|/home/[A-Za-z0-9_.]+|[\\/]AppData\b)", re.I) +# A .md under skills/ that is not a skill: ruling provenance for the repo's own rule +# documents. The install rules exclude it, so the set compare below must not demand it. +NOT_A_SKILL = {"LAWS.md"} + # nested paths too -- a skills/internal/x.md reference must not sit unmarked in # a shipped file SKILL_REF = re.compile(r"skills/((?:[A-Za-z0-9_-]+/)*[A-Za-z0-9_-]+\.md)") @@ -220,6 +225,12 @@ def shipped_exists(rel): continue path = os.path.join(dirpath, name) rel = name if rel_dir == "." else rel_dir.replace(os.sep, "/") + "/" + name + # not a skill at any depth -- the install rules drop it, so a bundle + # carrying one means an install regex stopped matching + if name in NOT_A_SKILL: + problems.append(("skills/" + rel, "not a skill", + "provenance for the repo's rule documents; it must not ship")) + continue if rel_dir == ".": root_skills.append(name) scan_file(path, rel, problems, @@ -247,8 +258,8 @@ def shipped_exists(rel): repo_skills = os.path.join(repo_root, "skills") if os.path.isdir(repo_skills): for name in sorted(os.listdir(repo_skills)): - if name.endswith(".md") and not os.path.exists( - os.path.join(skills_dir, name)): + if (name.endswith(".md") and name not in NOT_A_SKILL + and not os.path.exists(os.path.join(skills_dir, name))): problems.append(("repo skills/" + name, "missing from bundle", "the install rules did not ship it")) diff --git a/ci/test_check_shipped_skills.py b/ci/test_check_shipped_skills.py index 35cae3556b..1a9518996d 100644 --- a/ci/test_check_shipped_skills.py +++ b/ci/test_check_shipped_skills.py @@ -144,6 +144,20 @@ def test_repo_skill_missing_from_bundle(self): write(self.repo, "skills/dropped.md", "# Dropped\n\nprose\n") self.assert_fires("missing from bundle", "dropped.md") + def test_laws_md_is_not_a_skill(self): + # ruling provenance: the install rules exclude it on purpose, and a bundle + # that ships it anyway is still a defect + write(self.repo, "skills/LAWS.md", "# LAWS\n\n| date | doc | ask |\n") + rc, out = self.run_gate() + self.assertEqual(rc, 0, out) + write(self.bundle, "skills/LAWS.md", "# LAWS\n\n| date | doc | ask |\n") + self.assert_fires("not a skill", "LAWS.md") + + def test_nested_laws_md_in_bundle_fires(self): + # a nested one has no trigger row to miss, so only the basename check sees it + write(self.bundle, "skills/daslang/LAWS.md", "# LAWS\n\n| date | doc | ask |\n") + self.assert_fires("not a skill", "daslang/LAWS.md") + if __name__ == "__main__": unittest.main(verbosity=1) diff --git a/dastest/README.md b/dastest/README.md index ec5942282c..b7b9f9db4b 100644 --- a/dastest/README.md +++ b/dastest/README.md @@ -85,5 +85,8 @@ Not part of the test runner: `dastest/review_gate.das` is the support library fo contract lives in `REVIEW_COMMON.md` at the repo root, vendored by repos that adopt it). It provides finding accumulation and the exit verdict (`gate_finding`, `gate_findings`, `gate_reset`, `gate_verdict`), plus tree-analysis helpers: `das_requires`, -`cmake_command_blocks`, `cmake_list_entries`, `is_kebab_case`, `find_line`. It lives under +`cmake_command_blocks`, `cmake_command_targets`, `cmake_words`, `cmake_args`, +`cmake_list_entries`, `cmake_test_labels`, `cmake_test_commands`, `is_cmake_keyword`, +`is_kebab_case`, `find_line`. The CMake helpers match command names case-insensitively, as +CMake itself does. It lives under `dastest/` so an installed SDK carries it the same way it carries the test framework. diff --git a/dastest/review_gate.das b/dastest/review_gate.das index 174f6b7c4d..739390558b 100644 --- a/dastest/review_gate.das +++ b/dastest/review_gate.das @@ -95,6 +95,31 @@ def private is_ident_byte(b : int) : bool { return is_alnum(b) || b == '_' } +def private to_lower_byte(b : int) : int { + return b >= 'A' && b <= 'Z' ? b + 32 : b +} + +def private lowered_bytes(s : string) : array { + var out : array + peek_data(s) $(d) { + out |> reserve(length(d)) + for (i in range(length(d))) { + out |> push(to_lower_byte(int(d[i]))) + } + } + return <- out +} + +// CMake command names are case-insensitive, so `Add_Test(` opens a block like `add_test(`. +def private matches_lowered(d; at : int; lowered : array) : bool { + let m = length(lowered) + return false if (at + m > length(d)) + for (i in range(m)) { + return false if (to_lower_byte(int(d[at + i])) != lowered[i]) + } + return true +} + //! `text` with every // comment cut to end-of-line (string-literal-aware, so a "//" inside //! a quoted URL survives). Use before scanning sources or descriptors: a name mentioned //! only in a comment must not count. @@ -203,9 +228,10 @@ def gate_descriptor_census(descriptor_path : string; census_dirs : array //! double-quoted strings respected. Each result is the full block text, command name included. def cmake_command_blocks(text : string; command : string) : array { var out : array + var inscope needle <- lowered_bytes(command) peek_data(text) $(d) { let n = length(d) - let m = length(command) + let m = length(needle) var i = 0 while (i < n) { if (int(d[i]) == '#') { // a commented-out invocation must not open a block @@ -214,7 +240,7 @@ def cmake_command_blocks(text : string; command : string) : array { } continue } - if (!starts_with(d, i, command)) { + if (!matches_lowered(d, i, needle)) { i++ continue } @@ -267,11 +293,155 @@ def cmake_command_blocks(text : string; command : string) : array { return <- out } +//! Words of CMake text, `#` comments dropped, whitespace treated as a separator. A +//! double-quoted run is one word: quotes removed, backslashes kept as written. `(` and `)` +//! are words of their own, so a caller can see where one command's arguments end. +def cmake_words(text : string) : array { + var out : array + peek_data(text) $(d) { + let n = length(d) + var i = 0 + while (i < n) { + let c = int(d[i]) + if (c == '#') { + while (i < n && int(d[i]) != '\n') { + i++ + } + } elif (c == '"') { + var j = i + 1 + while (j < n && int(d[j]) != '"') { + if (int(d[j]) == '\\' && j + 1 < n) { + j++ + } + j++ + } + out |> push(slice(d, i + 1, j)) + i = j + 1 + } elif (c == '(' || c == ')') { + out |> push(slice(d, i, i + 1)) + i++ + } elif (is_white_space(c)) { + i++ + } else { + var j = i + while (j < n && !is_word_break_byte(int(d[j]))) { + j++ + } + out |> push(slice(d, i, j)) + i = j + } + } + } + return <- out +} + +def private is_word_break_byte(b : int) : bool { + return is_white_space(b) || b == '(' || b == ')' || b == '#' || b == '"' +} + +//! The argument words of one `command(...)` block - the command name and the enclosing +//! parens dropped. +def cmake_args(command_block : string) : array { + var inscope words <- cmake_words(command_block) + var out : array + var depth = 0 + out |> reserve(length(words)) + for (w in words) { + if (w == "(") { + depth++ + continue if (depth == 1) + } elif (w == ")") { + break if (depth == 1) + depth-- + } + out |> push(w) if (depth >= 1) + } + return <- out +} + +//! True for a word CMake reads as a keyword rather than a value: caps, digits and +//! underscores only, with at least one letter (`PROPERTIES`, `PASS_REGULAR_EXPRESSION`). +def is_cmake_keyword(word : string) : bool { + var letters = 0 + var ok = !empty(word) + peek_data(word) $(d) { + for (i in range(length(d))) { + let b = int(d[i]) + if (b >= 'A' && b <= 'Z') { + letters++ + } elif (!is_number(b) && b != '_') { + ok = false + break + } + } + } + return ok && letters > 0 +} + +//! Every ctest label the file's `LABELS` properties name, in order, without duplicates. +//! Both spellings count: `LABELS "big;style_lint"` and the `LABELS big style_lint` an +//! unquoted list expands to. Command-agnostic on purpose: `add_test`, +//! `set_tests_properties` and a discovery macro's `PROPERTIES LABELS` all count. +def cmake_test_labels(text : string) : array { + var out : array + var seen : table + var inscope words <- cmake_words(text) + let n = length(words) + var i = 0 + while (i < n) { + if (words[i] != "LABELS") { + i++ + continue + } + // the value run ends at the next property keyword or at the end of the command + var j = i + 1 + while (j < n && words[j] != ")" && words[j] != "(" && !is_cmake_keyword(words[j])) { + for (lab in split(words[j], ";")) { + continue if (empty(lab) || key_exists(seen, lab)) + seen |> insert(lab) + out |> push(lab) + } + j++ + } + i = j + } + return <- out +} + +//! The word after `COMMAND` in every `add_test` block, verbatim - the thing that runs the +//! test, which may be a target name, a `$` expression, or an interpreter. +def cmake_test_commands(text : string) : array { + var out : array + for (blk in cmake_command_blocks(text, "add_test")) { + var inscope args <- cmake_args(blk) + for (i in range(length(args) - 1)) { + if (args[i] == "COMMAND") { + out |> push(args[i + 1]) + break + } + } + } + return <- out +} + +//! Name of the first argument of every `command(...)` block, in order - the target a +//! `add_dependencies` / `add_custom_target` / `add_executable` block names. +def cmake_command_targets(text : string; command : string) : array { + var out : array + for (blk in cmake_command_blocks(text, command)) { + var inscope args <- cmake_args(blk) + if (!empty(args)) { + out |> push(args[0]) + } + } + return <- out +} + //! Entries of a CMake list across its `set(NAME ...)` and `list(APPEND NAME ...)` blocks, //! in order, `#` comments dropped. `${...}` references come back verbatim. def cmake_list_entries(text : string; listname : string) : array { var out : array - for (cmd in ["set", "SET", "list", "LIST"]) { + for (cmd in ["set", "list"]) { for (blk in cmake_command_blocks(text, cmd)) { let open = find(blk, "(") let body = slice(blk, open + 1, length(blk) - 1) diff --git a/dastest/tests/test_review_gate.das b/dastest/tests/test_review_gate.das index 34b7194638..ad27ded8d0 100644 --- a/dastest/tests/test_review_gate.das +++ b/dastest/tests/test_review_gate.das @@ -91,6 +91,118 @@ def test_cmake_command_blocks_edges(t : T?) { t |> success(find(blocks[2], "PROGRAMS z") >= 0, "whitespace before the paren is accepted") } +[test] +def test_cmake_words(t : T?) { + let text = "add_test(NAME a # comment ) here\n COMMAND \"c:/p a t h/x.exe\")\n" + var inscope words <- cmake_words(text) + t |> equal(7, length(words)) + t |> equal("add_test", words[0]) + t |> equal("(", words[1], "parens are words of their own") + t |> equal("NAME", words[2]) + t |> equal("a", words[3]) + t |> equal("COMMAND", words[4], "the comment runs to end of line only") + t |> equal("c:/p a t h/x.exe", words[5], "a quoted run is one word, quotes dropped") + t |> equal(")", words[6]) + + var inscope args <- cmake_args(text) + t |> equal(4, length(args), "cmake_args drops the command name and the parens") + t |> equal("NAME", args[0]) + t |> equal("c:/p a t h/x.exe", args[3]) + + var inscope trailing <- cmake_words("set(A \"unterminated\\") + t |> equal(4, length(trailing), "an escape at EOF terminates the scan") + t |> equal("unterminated\\", trailing[3], "the unterminated run ends at EOF, not past it") + + var inscope escaped <- cmake_words("\"esc\\\"aped\" x") + t |> equal(2, length(escaped)) + t |> equal("esc\\\"aped", escaped[0], "an escaped quote does not end the run") + + var inscope eof_comment <- cmake_words("# trailing comment, no newline") + t |> equal(0, length(eof_comment)) + + var inscope tight <- cmake_words("a\"b\" c") + t |> equal(3, length(tight), "a quote breaks a word even with no space before it") + t |> equal("a", tight[0]) + var inscope hashed <- cmake_words("a#b\nd") + t |> equal(2, length(hashed), "a # breaks a word and opens a comment") + t |> equal("a", hashed[0]) + t |> equal("d", hashed[1]) +} + +[test] +def test_cmake_test_labels(t : T?) { + let text = "add_test(NAME one COMMAND x)\nset_tests_properties(one PROPERTIES\n LABELS \"big;style_lint\"\n PASS_REGULAR_EXPRESSION \"STYLE014\")\ndoctest_discover_tests(exe PROPERTIES LABELS \"small\")\nset_tests_properties(two PROPERTIES LABELS big)\n" + var inscope labels <- cmake_test_labels(text) + t |> equal(3, length(labels)) + t |> equal("big", labels[0]) + t |> equal("style_lint", labels[1], "a semicolon list yields every label") + t |> equal("small", labels[2], "any command's LABELS property counts") + + var inscope none <- cmake_test_labels("add_test(NAME x COMMAND y)\n# LABELS \"commented\"\n") + t |> equal(0, length(none)) + + var inscope dangling <- cmake_test_labels("set_tests_properties(x PROPERTIES LABELS") + t |> equal(0, length(dangling), "a LABELS with no value reads past nothing") + var inscope trailing_semi <- cmake_test_labels("set_tests_properties(x PROPERTIES LABELS \"big;\")") + t |> equal(1, length(trailing_semi), "a trailing semicolon yields no empty label") + t |> equal("big", trailing_semi[0]) + + // an unquoted list variable expands to one argument per label - the shape the vendored + // doctestAddTests.cmake writes as `LABELS ${labels}` + var inscope expanded <- cmake_test_labels( + "set_tests_properties(x PROPERTIES LABELS big style_lint PASS_REGULAR_EXPRESSION \"STYLE014\")\nadd_dependencies(test-big y)\n") + t |> equal(2, length(expanded), "every label of an expanded list is read") + t |> equal("big", expanded[0]) + t |> equal("style_lint", expanded[1]) + + var inscope bounded <- cmake_test_labels("set_tests_properties(x PROPERTIES LABELS big)\nadd_dependencies(test-big y)\n") + t |> equal(1, length(bounded), "the run stops at the end of its own command") + t |> equal("big", bounded[0]) +} + +[test] +def test_cmake_command_targets(t : T?) { + let text = "add_dependencies(test-big test_a test_b)\n# add_dependencies(test-small ghost)\nadd_dependencies(\n test-small test_c)\nadd_executable(other x.cpp)\n" + var inscope deps <- cmake_command_targets(text, "add_dependencies") + t |> equal(2, length(deps)) + t |> equal("test-big", deps[0]) + t |> equal("test-small", deps[1], "the target may sit on the next line") + var inscope exes <- cmake_command_targets(text, "add_executable") + t |> equal(1, length(exes)) + t |> equal("other", exes[0]) + + var inscope argless <- cmake_command_targets("add_dependencies()\nadd_dependencies(x y)", "add_dependencies") + t |> equal(1, length(argless), "an empty block names no target and does not index past it") + t |> equal("x", argless[0]) + var inscope absent <- cmake_command_targets(text, "add_library") + t |> equal(0, length(absent)) +} + +[test] +def test_cmake_commands_are_case_insensitive(t : T?) { + // CMake itself accepts any casing; a gate that only reads one spelling reports + // a wiring defect that is not there, or misses one that is + let text = "ADD_DEPENDENCIES(test-big test_a)\nAdd_Test(NAME m COMMAND test_a)\nset_tests_properties(m PROPERTIES LABELS \"big\")\n" + var inscope deps <- cmake_command_targets(text, "add_dependencies") + t |> equal(1, length(deps)) + t |> equal("test-big", deps[0], "an all-caps command opens a block") + var inscope cmds <- cmake_test_commands(text) + t |> equal(1, length(cmds)) + t |> equal("test_a", cmds[0], "mixed case opens a block too") + var inscope labels <- cmake_test_labels(text) + t |> equal(1, length(labels)) + t |> equal("big", labels[0]) +} + +[test] +def test_cmake_test_commands(t : T?) { + let text = "add_test(NAME a COMMAND test_a WORKING_DIRECTORY x)\nadd_test(\n NAME b\n COMMAND $ script.das)\nadd_test(NAME c)\n" + var inscope cmds <- cmake_test_commands(text) + t |> equal(2, length(cmds), "a block with no COMMAND contributes nothing") + t |> equal("test_a", cmds[0]) + t |> equal("$", cmds[1], "a generator expression comes back verbatim") +} + [test] def test_cmake_list_entries(t : T?) { let text = "set(DAS_UTILS\n aot\n dascov # a comment\n internal/jit\n)\nif(NOT X)\n list(APPEND DAS_UTILS benchctl) # same gate\nendif()\nset(OTHER_LIST nope)\n" @@ -108,4 +220,7 @@ def test_cmake_list_entries(t : T?) { var inscope bare <- cmake_list_entries("set()\nset(EMPTY_LIST)\n", "EMPTY_LIST") t |> equal(0, length(bare), "a set with no entries neither crashes nor yields") + + var inscope shouty <- cmake_list_entries("SET(CAPS a)\nLIST(APPEND CAPS b)\n", "CAPS") + t |> equal(2, length(shouty), "an uppercase command is read once, not twice") } diff --git a/install/CLAUDE.md b/install/CLAUDE.md index bc9a33f818..0ce47c79bc 100644 --- a/install/CLAUDE.md +++ b/install/CLAUDE.md @@ -186,7 +186,7 @@ For path/filename ops use `fio` helpers (`base_name`/`dir_name`/`path_join`/...) **`print` is fine for application scripts.** In library and tool code prefer `to_log(LOG_INFO|LOG_WARNING|LOG_ERROR)` - same stdout, but level-tagged and filterable. -**Comment hygiene (.das): ABSOLUTELY NO comments that are not documentation or lint suppression.** The kept set is exactly: `//!` docs on public API (never on private symbols), `// nolint:CODE` / `@nolint` suppressions carrying their one-line why, `//fmt:` formatter directives, and a file's leading header block. Everything else - narration, banners, section dividers, commented-out code - does not exist; the MCP `format_file` tool applies this file-wide by default, fail-closed. Teaching code (tutorials, examples) is the one exception: prose carrying the lesson stays - the folder's `.lint_config` `[format]` policy opts it out (`force_clean_comments = false`), so the same `format_file` call is safe everywhere. +**Comment hygiene (.das): the folder's `.lint_config` decides.** Where `[format] force_clean_comments = true`, only the kept set survives a format - `//!` docs on a contract a caller must know, `// nolint:CODE` / `@nolint` suppressions carrying their one-line why, `//fmt:` directives, license comments, and the file's leading header block; the MCP `format_file` tool strips everything else, fail-closed. The key defaults to `false` - teaching code (tutorials, examples) declares that default explicitly - and wherever it is false the kept set still holds and every other comment answers to the deletion test instead: remove it and re-read; if the WHY goes dark, restore it, otherwise it was noise. The same `format_file` call is safe everywhere. ## SDK Directory Layout diff --git a/skills/LAWS.md b/skills/LAWS.md new file mode 100644 index 0000000000..02bfdbd808 --- /dev/null +++ b/skills/LAWS.md @@ -0,0 +1,11 @@ +# LAWS.md - Boris's rulings + +Append-only intent provenance for rule-document edits in this folder's documents (the +mechanism: CLAUDE.md sec. "Boris's rulings get a `LAWS.md` sidecar"). Never groomed, +compacted, or cited as rules. + +| Date | Document | The ask | +|---|---|---| +| 2026-08-25 | comment_style_hygiene.md (guide self-review boundary) | "DONT PROPOSE MINOR CHANGES TO THIS ONE" - said while triaging comment findings on the vecmath backend PR; the guide invites findings against itself, and he wants that read narrowly | +| 2026-08-25 | comment_style_hygiene.md (vendored code) | "3rd party libraries go as is" - on the vendored include/vecmath copy: house comment rules do not reach code owned by an upstream project | +| 2026-08-26 | comment_style_hygiene.md (.das kept-set boundary), CLAUDE.md (repo root), install/CLAUDE.md (shipped twin), das_formatting.md | "lets fix. prorposed wording is good" - on the audit finding that the kept-set ban read as opt-out when `force_clean_comments` is opt-in; state the boundary by the mechanism, not by an example list, in every document that carried the old reading | diff --git a/skills/comment_style_hygiene.md b/skills/comment_style_hygiene.md index 775f9f34fa..a1d6a901e2 100644 --- a/skills/comment_style_hygiene.md +++ b/skills/comment_style_hygiene.md @@ -14,6 +14,10 @@ this guide.** Mark it like any other finding - a guide defect blocks nothing, bu fix (a rewrite or a removal, never silent tolerance) lands with the round's other fixes. +**DON'T PROPOSE MINOR CHANGES TO THIS ONE.** A defect in this guide that changes what a +reviewer checks or concludes is a finding; wording, ordering and coverage you would like +added are not. + ## How this guide is applied - **Findings speak plain.** A review finding - and the commit fixing one - states the @@ -24,27 +28,35 @@ fixes. carries the boundary for when a departure stands. - **The audit is not a gate.** Style review runs on every PR; findings persuade, lint compels. Fix or consciously decline - there is no re-run-until-clean loop. +- **Third-party libraries go as is.** A file this repo syncs from an upstream project + rather than authors answers to that project, not to this guide - editing it to these + rules is fork drift. A folder is vendored when it is a `3rdparty/` tree or its own + `CLAUDE.md` names an upstream and records the synced revision; inside one, the files + that `CLAUDE.md` marks as written or maintained here - fork-local hooks, this repo's + own docs - answer to these rules, and every file it leaves to upstream does not. Rules marked *(lintable)* are mechanical enough for a lint to enforce; where a rule notes an existing lint, the lint compels - an unnoted rule is the reviewer's. ## Comments -**.das outside teaching code: ABSOLUTELY NO comments that are not documentation or lint -suppression.** The kept set is exactly: `//!` docs on a contract a CALLER must know - -public API always, a private helper only when callers inside the module need the contract -(narration of an implementation goes to a name, a test, or the module's `.md`) - -`// nolint:CODE` / `@nolint` suppressions carrying their one-line why, `//fmt:` formatter -directives, license comments (Copyright / SPDX), and the file's leading header block (which -may sit below the `options` / `module` / `require` preamble). Everything else - narration, -banners, section dividers, commented-out code - does not exist. The formatter enforces this -per folder, fail-closed (a strip must compile or the file is restored); the policy mechanics -are `skills/das_formatting.md`. A `//` comment outside the kept set is therefore EPHEMERAL: -anything worth preserving becomes code (a name, a `//!` doc, an assert) or lands in an `.md` -beside the code. The ban is on what survives to a commit, not on writing - scaffold freely -while working; the PR gate drains what remains. The boundary is the folder's `.lint_config`: teaching code (tutorials, -examples) and load-bearing fixture corpora opt out there, and prose carrying the lesson -stays - comments that add nothing to their line still go. +**.das in a folder whose `.lint_config` arms `force_clean_comments`: ABSOLUTELY NO comments +that are not documentation or lint suppression.** The kept set is exactly: `//!` docs on a +contract a CALLER must know - public API always, a private helper only when callers inside +the module need the contract (narration of an implementation goes to a name, a test, or the +module's `.md`) - `// nolint:CODE` / `@nolint` suppressions carrying their one-line why, +`//fmt:` formatter directives, license comments (Copyright / SPDX), and the file's leading +header block (which may sit below the `options` / `module` / `require` preamble). Everything +else - narration, banners, section dividers, commented-out code - does not exist. The +formatter enforces this per folder, fail-closed (a strip must compile or the file is +restored); the policy mechanics are `skills/das_formatting.md`. A `//` comment outside the +kept set is therefore EPHEMERAL: anything worth preserving becomes code (a name, a `//!` +doc, an assert) or lands in an `.md` beside the code. The ban is on what survives to a +commit, not on writing - scaffold freely while working; the PR gate drains what remains. +Where the key is unset or false, the kept set above still holds and every other `//` comment +answers to the deletion test instead; in teaching folders (tutorials, examples) and +load-bearing fixture corpora the prose carrying the lesson passes that test, and a comment +that adds nothing to its line still goes. **The header block - the comments above the first declaration - describes the FILE.** A comment there that describes the next declaration moves onto that declaration: as a `//!` diff --git a/skills/das_formatting.md b/skills/das_formatting.md index 4adc187e5e..ba7d08f3ce 100644 --- a/skills/das_formatting.md +++ b/skills/das_formatting.md @@ -2,7 +2,7 @@ After creating or modifying any `.das` file that is part of the project (daslib modules, tutorials, tests, etc.), run the source formatter on it. Do NOT format temporary/scratch files that will be deleted. -**Formatter:** MCP `format_file` tool (calls `daslib/das_source_formatter` directly). Comment policy comes from the file's folder: each `[format]` key is taken from the nearest `.lint_config` that declares it, and keys cascade independently up to the `.git` root. `force_clean_comments = true` also strips comments file-wide, keeping only the set `skills/comment_style_hygiene.md` defines; fail-closed (a strip must pass compile_check or the file is restored). Teaching folders (`tutorials/`, `examples/`) declare `force_clean_comments = false`; `format_enabled = false` makes the formatter a no-op for that tree (load-bearing fixture layout). There is no per-call override. +**Formatter:** MCP `format_file` tool (calls `daslib/das_source_formatter` directly). Comment policy comes from the file's folder: each `[format]` key is taken from the nearest `.lint_config` that declares it, and keys cascade independently up to the `.git` root. `force_clean_comments = true` also strips comments file-wide, keeping only the set `skills/comment_style_hygiene.md` defines; fail-closed (a strip must pass compile_check or the file is restored). The key defaults to `false`, and teaching folders declare it so explicitly; `format_enabled = false` makes the formatter a no-op for that tree (load-bearing fixture layout). There is no per-call override. **A strip that deleted comments is reviewed before commit.** The MCP tool appends a final `rescue_advisory` entry to the batch result and the CLI prints `comments were deleted from N file(s)`; either way, read the strip diff before committing and rescue what carries lasting information. What counts as worth rescuing, and where it goes, is `skills/comment_style_hygiene.md`. diff --git a/skills/internal/skill_taxonomy.md b/skills/internal/skill_taxonomy.md index ca98d7f6a6..fe8bcd1d00 100644 --- a/skills/internal/skill_taxonomy.md +++ b/skills/internal/skill_taxonomy.md @@ -10,6 +10,9 @@ Read this before adding, moving, or shipping any skill, and before any skills cl no CI, no lint rule IDs. - `skills/internal/` - never ships: repo workflow, CI, review machinery, compiler internals. +- `LAWS.md`, in any folder - never ships: Boris's ruling provenance for that folder's rule + documents, not a skill. Anywhere under `skills/` the install rules drop it by name and + the gate knows the name. There is no shipping list. The gate (`ci/check_shipped_skills.py`) enforces the split: no shipped file links into `internal/`, no unmarked repo-only path in a shipped file diff --git a/tests-cpp/REVIEW.das b/tests-cpp/REVIEW.das new file mode 100644 index 0000000000..214d882f53 --- /dev/null +++ b/tests-cpp/REVIEW.das @@ -0,0 +1,93 @@ +options gen2 + +require strings +require daslib/fio +require dastest/review_gate + +// The mechanical half of tests-cpp/REVIEW.md (contract: REVIEW_COMMON.md at the repo root). +// Run from the repo root: bin/daslang tests-cpp/REVIEW.das — exit 0 clean, 1 with findings. + +def private lanes_of(text : string; command : string) : array { + var inscope names <- cmake_command_targets(text, command) + return <- [for (n in names); slice(n, length("test-"), length(n)); where n |> starts_with("test-")] +} + +def private cmakelists_under(folder : string) : array { + var out : array + dir(folder) $(name) { + let p = "{folder}/{name}/CMakeLists.txt" + if (name != "." && name != ".." && fexist(p)) { + out |> push(p) + } + } + out |> sort + return <- out +} + +// Targets named by `add_dependencies(test- ...)`, whichever lane. +def private lane_wired_targets(text : string) : array { + var out : array + for (blk in cmake_command_blocks(text, "add_dependencies")) { + var inscope args <- cmake_args(blk) + continue if (empty(args) || !starts_with(args[0], "test-")) + out |> reserve(length(out) + length(args) - 1) + for (i in range(1, length(args))) { + out |> push(args[i]) + } + } + return <- out +} + +// The target an `add_test` COMMAND runs, or "" when it is not a target this file builds - +// an interpreter or a plain path is nobody's build dependency. +def private runs_target(command : string; local_exes : array) : string { + let genex = "$ starts_with(genex) && command |> ends_with(">")) { + return slice(command, length(genex), length(command) - 1) + } + return find_index(local_exes, command) >= 0 ? command : "" +} + +// Every test binary a file registers is a binary its lane builds. `ctest -L ` runs +// whatever carries the label, while `test-` builds only what is wired to it, so an +// unwired target runs stale or missing while the lane reports success. +def private check_test_targets(f : string; text : string) { + var inscope local_exes <- cmake_command_targets(text, "add_executable") + var inscope wired <- lane_wired_targets(text) + for (command in cmake_test_commands(text)) { + let target = runs_target(command, local_exes) + continue if (empty(target) || find_index(wired, target) >= 0) + gate_finding(f, "test target {target} is wired into no test- here - add_dependencies(test- {target}) or the lane runs a binary it never built") + } +} + +// Every lane a file labels tests into is a lane that file also builds for. +def private check_lane_wiring { + let top_cmake = "tests-cpp/CMakeLists.txt" + var inscope aggregate_lanes <- lanes_of(fread(top_cmake), "add_custom_target") + var files <- [top_cmake] + files |> push_from(cmakelists_under("tests-cpp/big")) + for (f in files) { + let text = fread(f) + var inscope wired_lanes <- lanes_of(text, "add_dependencies") + // a file that defines an aggregate wires its dependencies at the definition site + wired_lanes |> push_from(lanes_of(text, "add_custom_target")) + var inscope labels <- cmake_test_labels(text) + for (lab in labels) { + // a label with no test- target is a ctest filter — nothing to wire to + continue if (find_index(aggregate_lanes, lab) < 0 || find_index(wired_lanes, lab) >= 0) + gate_finding(f, "labels a test \"{lab}\" but wires nothing into test-{lab} — wire it with add_dependencies(test-{lab} ), or the lane runs a binary it never built") + } + check_test_targets(f, text) + } +} + +[export] +def main() : int { + if (!fexist("tests-cpp/REVIEW.das")) { + to_log(LOG_ERROR, "tests-cpp/REVIEW.das: run from the repo root\n") + return 2 + } + check_lane_wiring() + return gate_verdict("tests-cpp") +} diff --git a/utils/REVIEW.das b/utils/REVIEW.das index f2d97a28d4..6e25d3833e 100644 --- a/utils/REVIEW.das +++ b/utils/REVIEW.das @@ -93,12 +93,8 @@ def private check_kebab_case(externals, internals : array) { } } -// CMake commands are case-insensitive and this tree spells both install( and INSTALL( def private install_blocks_of(text : string) : array { - var out <- cmake_command_blocks(text, "install") - var inscope upper <- cmake_command_blocks(text, "INSTALL") - out |> push_from(upper) - return <- out + return <- cmake_command_blocks(text, "install") } def private check_install_symmetry(externals : array) {