Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ All notable changes to OComment will be documented here. The project follows

### Fixed

- A mise file task's header is a load-bearing directive in every language.
mise and its `usage` library read `#MISE`, `#USAGE`, `#[MISE]` and `#[USAGE]` — and the same after `//` — case-sensitively from the raw line, and `#MISE depends=`, `dir=` and `#USAGE flag` decide what the task runs and which arguments it accepts.
They were read as prose, so `ocomment fix --tidy` under `wrap = "sentence"` joined a `#MISE` line and the `#USAGE flag` line under it into one `# MISE` line, and the task stopped declaring the flag.
They are now kept by every policy, including `--policy all` without `--force-protected`, and end a paragraph as `# shellcheck` does, while `# mise installs the runtime` and `# Usage: audit` stay prose.

- The test suite runs on the systems this repository publishes a binary for.
`cargo test` ran on Linux alone while `release.yml` shipped
`x86_64-pc-windows-msvc`; what Windows CI measured was that the crate builds
Expand Down
2 changes: 2 additions & 0 deletions docs/why-kept.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ is the only way to give one up.
| `#__PURE__` | `javascript` | `/*#__PURE__*/` | `load-bearing` | required by the language or its build |
| `@__PURE__` | `javascript` | `/*@__PURE__*/` | `load-bearing` | required by the language or its build |
| `#__NO_SIDE_EFFECTS__` | `javascript` | `/*#__NO_SIDE_EFFECTS__*/` | `load-bearing` | required by the language or its build |
| `MISE` | `shell` | `#MISE description="Audit the signing posture"` | `load-bearing` | required by the language or its build |
| `USAGE` | `shell` | `#USAGE flag "--fix" help="Repair what the audit finds"` | `load-bearing` | required by the language or its build |
| `shebang` | `shell` | `#!/bin/sh` | `shebang` | required source preamble |
| `encoding` | `python` | `# -*- coding: utf-8 -*-` | `encoding` | required source preamble |
| `sourceMappingURL` | `javascript` | `//# sourceMappingURL=bundle.js.map` | `directive` | tool or language directive |
Expand Down
25 changes: 25 additions & 0 deletions ocaml/lib/ocomment_ref.ml
Original file line number Diff line number Diff line change
Expand Up @@ -589,8 +589,31 @@ let bundler_directive compact =
in
webpack || opens_with_keyword compact "vite-ignore"

(** A mise file task's header line: "#" or "//", optional whitespace, then "MISE" or "USAGE", bare or in square brackets, ending at whitespace or at the end of the comment.
mise and the usage library it hands argument lines to both match the raw line case-sensitively, so this is asked of [raw] rather than of the folded text, and "# mise installs the runtime" stays prose.
A file task is any executable file, so every language is asked. *)
let mise_task_header raw =
let length = String.length raw in
let after_marker =
if String.starts_with ~prefix:"#" raw then Some 1
else if String.starts_with ~prefix:"//" raw then Some 2
else None in
match after_marker with
| None -> false
| Some index ->
let rec skip cursor =
if cursor < length && ascii_whitespace raw.[cursor] then skip (cursor + 1) else cursor in
let start = skip index in
let rest = String.sub raw start (length - start) in
List.exists (fun word ->
let size = String.length word in
String.starts_with ~prefix:word rest &&
(String.length rest = size || ascii_whitespace rest.[size]))
["MISE"; "[MISE]"; "USAGE"; "[USAGE]"]

let is_directive language text raw =
let compact = directive_compact text in
mise_task_header raw ||
let prefixes = ["sourcemappingurl="; "sourceurl="; "#__pure__"; "@__pure__";
"__pure__"; "#__no_side_effects__"; "__no_side_effects__"; "ts-ignore";
"ts-expect-error"; "ts-nocheck"; "ts-check"; "eslint"; "prettier-ignore";
Expand Down Expand Up @@ -736,6 +759,8 @@ let bundler_is_load_bearing compact =

let is_load_bearing language text raw =
let compact = directive_compact text in
(* NOTE: A mise task's header decides what the task runs and which arguments it takes, so removing one changes the task rather than a report about it; asked of every language, as [is_directive] asks it. *)
mise_task_header raw ||
match language with
(* NOTE: The same distinction as in [directive_name], and it has to be made again here because this decides load-bearing from the text rather than from the directive name the other one returned.
A spaced "// go:generate" is prose, and prose no policy can reach is worse than prose that stays. *)
Expand Down
36 changes: 36 additions & 0 deletions rust/ocomment-core/src/scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6094,6 +6094,11 @@ const fn protected_reason(kind: CommentKind) -> Option<&'static str> {
/// The namespace is the compiler's, `//go:generate` is the only member of it a project could argue is bookkeeping, and the argument is not worth the asymmetry: a marker protected in error is one comment left behind that `--force-protected` takes away, while a marker missed in error is a silent change to the build.
/// Protect generously, and say why.
fn is_load_bearing(name: &str, language: Language) -> bool {
/* NOTE: A mise file task's header is the task's own definition: `#MISE depends=` and `dir=` decide what runs and where, and `#USAGE flag` declares an argument the task accepts, so a task stripped of one still runs and no longer does what it did.
* That is the toolchain's output changing rather than a report, and it is asked of every language because a file task can be written in any of them. */
if matches!(name, "MISE" | "USAGE") {
return true;
}
match language {
/* NOTE: `//go:build` and its retired `// +build` twin decide whether the file is compiled at all, `//go:embed` decides what a variable holds, and `//go:noescape` and its neighbours decide what the compiler is allowed to assume. */
Language::Go => matches!(name, "go:" | "+build"),
Expand Down Expand Up @@ -6929,6 +6934,9 @@ fn go_directive(compact: &str, raw: &[u8]) -> Option<&'static str> {
}

fn directive_name(text: &str, language: Language, raw: &[u8]) -> Option<&'static str> {
if let Some(name) = mise_task_header(raw) {
return Some(name);
}
let compact = text.trim_start_matches(['!', '/', '*', '#', '@', ' ']);
let common = [
"sourcemappingurl=",
Expand Down Expand Up @@ -7176,6 +7184,34 @@ fn directive_name(text: &str, language: Language, raw: &[u8]) -> Option<&'static
}
}

/// The header line a mise file task declares itself in, if `raw` is one.
///
/// mise reads a task's description, dependencies, working directory, environment and arguments out of comments at the head of the script, and the `usage` library it hands the argument lines to reads them the same way.
/// Both match the raw line, case-sensitively: `#` or `//`, optional white space, then `MISE` or `USAGE`, bare or in square brackets (mise 2026.9.12, `^(?:#|//|::)\s*(?:(USAGE|MISE)|\[(USAGE|MISE)\])(.*)$`).
/// So this reads `raw` rather than the folded text, which is what keeps `# mise installs the runtime` and `# Usage: foo` prose.
/// The word ends at white space or at the end of the comment, as the tools' own keywords do in [`opens_with_keyword`];
/// `::` is a batch file's marker, and no built-in language opens a comment with it.
///
/// Asked of every language, because a file task is any executable file — shell, Python, Ruby, a Node script — and each writes the header in its own comment marker.
fn mise_task_header(raw: &[u8]) -> Option<&'static str> {
let rest = raw
.strip_prefix(b"#")
.or_else(|| raw.strip_prefix(b"//"))?
.trim_ascii_start();
[
(&b"MISE"[..], "MISE"),
(b"[MISE]", "MISE"),
(b"USAGE", "USAGE"),
(b"[USAGE]", "USAGE"),
]
.into_iter()
.find(|(word, _)| {
rest.strip_prefix(*word)
.is_some_and(|tail| tail.first().is_none_or(u8::is_ascii_whitespace))
})
.map(|(_, name)| name)
}

/// Whether `text` opens with `keyword` and then ends it.
///
/// A directive named after the tool that reads it — `shellcheck`, `hadolint` —
Expand Down
69 changes: 69 additions & 0 deletions rust/ocomment-core/tests/languages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,75 @@ fn dockerfile_parser_and_linter_directives_are_protected() {
assert_eq!(removable(&report), 3);
}

/// A mise file task declares its description, dependencies, working directory and arguments in comments at its head, and mise 2026.9.12 and its `usage` library read them case-sensitively from the raw line: `#` or `//`, optional white space, then `MISE` or `USAGE`, bare or in square brackets.
/// A file task is any executable file, so every language is asked, and removing a line changes what the task runs or accepts, so none of them is reachable by `--policy all`.
/// The same letters in another case, running on past the word, or behind a marker mise does not read are prose.
#[test]
fn mise_task_headers_are_load_bearing_in_every_language() {
let all = ScanOptions {
policy: Policy::All,
..ScanOptions::default()
};
let cases: [(Language, &[u8]); 5] = [
(
Language::Shell,
b"#MISE description=\"audit\"\n#USAGE flag \"--fix\"\n# [MISE] dir=\"{{cwd}}\"\n#\t[USAGE] arg \"<repo>\"\n#MISE\n",
),
(Language::Python, b"#MISE depends=[\"build\"]\n# USAGE arg \"<file>\"\n"),
(Language::Ruby, b"#MISE description=\"audit\"\n#USAGE flag \"--fix\"\n"),
(Language::JavaScript, b"//MISE description=\"audit\"\n// [USAGE] flag \"--fix\"\n"),
(Language::TypeScript, b"//USAGE flag \"--fix\"\n//[MISE] alias=\"a\"\n"),
];
for (language, source) in cases {
let report = scan(source, language, all.clone());
assert!(report.valid, "{language:?}");
for comment in &report.comments {
assert_eq!(
comment.kind,
CommentKind::LoadBearing,
"{language:?}: {comment:?}"
);
assert!(!comment.action().removes(), "{language:?}: {comment:?}");
}
assert!(!report.comments.is_empty(), "{language:?}");
}

let prose = b"# mise installs the runtime\n# Usage: audit [--fix]\n#MISEish note\n##MISE doubled\n# usage flag\n";
let report = scan(prose, Language::Shell, all.clone());
assert_eq!(report.comments.len(), 5);
assert_eq!(removable(&report), 5);

let javascript = b"/// MISE doc\n/* MISE block */\nrun();\n";
let report = scan(javascript, Language::JavaScript, all);
assert_eq!(removable(&report), 2);
}

/// The header a dotfiles repository wrote, which `fix --tidy` under `wrap = "sentence"` used to join into one `# MISE` line, so mise read the `#USAGE flag` as part of the description and the task no longer took the flag.
/// A header line is a directive, and a directive ends a paragraph rather than joining one.
#[test]
fn a_sentence_wrap_leaves_a_mise_task_header_alone() {
let source = b"#!/usr/bin/env bash\n#MISE description=\"Audit signing posture across local git repos under $HOME\"\n#USAGE flag \"--fix-stale-hooks\" help=\"Remove legacy hooks\"\n#\n# Walks $HOME and audits each repository.\necho audit\n";
let options = ScanOptions {
policy: Policy::None,
style: ocomment_core::StyleRules {
wrap: ocomment_core::Wrap::Sentence,
..Default::default()
},
..ScanOptions::default()
};
let report = scan(source, Language::Shell, options);
assert!(report.valid);
assert!(report.runs.is_empty(), "{:?}", report.runs);
assert!(
report
.comments
.iter()
.all(|comment| !matches!(comment.disposition(), Disposition::Rewrite { .. })),
"{:?}",
report.comments
);
}

#[test]
fn shell_command_substitutions_are_scanned_inside_quotes() {
let source = b"value=\"$(printf ok # nested\n)\"\nold=`printf ok # legacy\n`\ntext=\"# opaque\"\n# remove\n";
Expand Down
6 changes: 5 additions & 1 deletion rust/ocomment/assets/directives.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,14 @@ load_bearing = [
"optimizer-hint", "version-comment",
# NOTE: The bundlers': each decides what comes out of the build.
"webpack", "vite-ignore",
"#__PURE__", "@__PURE__", "#__NO_SIDE_EFFECTS__"
"#__PURE__", "@__PURE__", "#__NO_SIDE_EFFECTS__",
# NOTE: A mise file task's header: `#MISE` and `#USAGE` define what the task runs and which arguments it takes.
# NOTE: A file task is any executable file, so both are read in every language and no entry below names them.
"MISE", "USAGE"
]

# NOTE: Every built-in language answers, an empty list included: surveyed and found to have none is a different claim from nobody having looked.
# NOTE: The mise task header is the one load-bearing marker read in every language, and it is left out of the entries for the reason the cross-language tool markers are left out of `[protected_by_language]`.
[load_bearing_by_language]
go = ["go:", "+build"]
swift = ["swift-tools-version:"]
Expand Down
2 changes: 1 addition & 1 deletion rust/ocomment/assets/selftest-corpus.json

Large diffs are not rendered by default.

33 changes: 28 additions & 5 deletions rust/ocomment/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
//! Every case here starts a process.
//! What a test can see is what a caller can see -- the two streams, the exit status, and the bytes on the disk afterwards -- which is the point: a promise the binary makes is a promise about those and not about a function somewhere inside it.

mod common;

use std::{
collections::BTreeSet,
fs,
Expand Down Expand Up @@ -37,7 +39,7 @@ fn binary() -> &'static str {
fn command() -> Command {
static EMPTY: std::sync::OnceLock<tempfile::TempDir> = std::sync::OnceLock::new();
let empty = EMPTY.get_or_init(|| tempfile::tempdir().expect("a temporary directory"));
let mut command = Command::new(binary());
let mut command = common::isolated(binary());
command.env("XDG_CONFIG_HOME", empty.path());
command
}
Expand Down Expand Up @@ -141,7 +143,7 @@ fn git_program() -> &'static str {
}

fn git(directory: &Path, arguments: &[&str]) -> Vec<u8> {
let output = Command::new(git_program())
let output = common::isolated(git_program())
.current_dir(directory)
.args(arguments)
.output()
Expand All @@ -157,7 +159,7 @@ fn git(directory: &Path, arguments: &[&str]) -> Vec<u8> {

#[cfg(unix)]
fn git_with_path(directory: &Path, arguments: &[&str], path: &std::ffi::OsStr) -> Vec<u8> {
let output = Command::new(git_program())
let output = common::isolated(git_program())
.current_dir(directory)
.args(arguments)
.arg(path)
Expand Down Expand Up @@ -224,6 +226,27 @@ fn check_diff_and_fix_follow_the_exit_contract() {
);
}

#[test]
#[cfg(unix)]
fn a_repository_named_by_the_callers_environment_is_left_alone() {
let sentinel = tempfile::tempdir().unwrap();
git(sentinel.path(), &["init", "-q"]);
let config = sentinel.path().join(".git/config");
let before = fs::read(&config).unwrap();

let output = std::process::Command::new(std::env::current_exe().unwrap())
.args(["--exact", "a_tidy_and_a_staged_write_both_exit_one"])
.env("GIT_DIR", sentinel.path().join(".git"))
.env("GIT_WORK_TREE", sentinel.path())
.env("GIT_INDEX_FILE", sentinel.path().join(".git/index"))
.output()
.unwrap();
let stdout = String::from_utf8_lossy(&output.stdout);
assert!(output.status.success(), "{stdout}");
assert!(stdout.contains("1 passed"), "{stdout}");
assert_eq!(fs::read(&config).unwrap(), before);
}

/// The two ways a `fix` finishes with something still to answer for.
///
/// `--tidy` writes one half of what it found and leaves the other where it was; a staged run writes the bytes the commit is about to carry.
Expand Down Expand Up @@ -314,7 +337,7 @@ fn diff_is_byte_preserving_and_git_applies_quoted_non_utf8_paths() {
String::from_utf8_lossy(&output.stdout)
);

let mut apply = Command::new(git_program())
let mut apply = common::isolated(git_program())
.current_dir(directory.path())
.args(["apply", "--whitespace=nowarn", "-"])
.stdin(Stdio::piped())
Expand Down Expand Up @@ -2364,7 +2387,7 @@ fn a_wide_transaction_completes_under_a_low_file_descriptor_limit() {
}

/* NOTE: The shell carries the isolation `command` would have given, because the binary is reached through it rather than spawned directly: a user configuration this machine really has would otherwise decide what this test observes. */
let output = Command::new("/bin/bash")
let output = common::isolated("/bin/bash")
.current_dir(directory.path())
.env("PATH", test_path())
.env("XDG_CONFIG_HOME", directory.path().join("no-user-config"))
Expand Down
11 changes: 11 additions & 0 deletions rust/ocomment/tests/common/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use std::{ffi::OsStr, process::Command};

pub fn isolated(program: impl AsRef<OsStr>) -> Command {
let mut command = Command::new(program);
for (key, _) in std::env::vars_os() {
if key.to_string_lossy().starts_with("GIT_") {
command.env_remove(key);
}
}
command
}
14 changes: 6 additions & 8 deletions rust/ocomment/tests/deadline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@
//! `[policy.allow] tags` and `[policy.allow.expiry]` differ in one thing and it is the thing that needs a repository: whether the tag runs out.
//! These tests build one, commit at a date of their choosing, and check that the run reaches the verdict the dates call for — and, just as importantly, that it leaves a comment alone when it cannot read them.

use std::{
fs,
path::Path,
process::{Command, Output},
};
mod common;

use std::{fs, path::Path, process::Output};
use tempfile::TempDir;

fn binary() -> &'static str {
Expand All @@ -26,7 +24,7 @@ fn no_user_config() -> &'static std::path::Path {
}

fn git(directory: &Path, arguments: &[&str], date: Option<&str>) {
let mut command = Command::new("git");
let mut command = common::isolated("git");
command.current_dir(directory).args(arguments);
if let Some(date) = date {
command
Expand All @@ -50,7 +48,7 @@ fn run(directory: &Path, arguments: &[&str]) -> Output {
arguments.push("--format");
arguments.push("human");
}
Command::new(binary())
common::isolated(binary())
.env("XDG_CONFIG_HOME", no_user_config())
.current_dir(directory)
.args(&arguments)
Expand Down Expand Up @@ -223,7 +221,7 @@ fn a_proposed_edit_is_judged_against_the_history_of_the_file_it_would_change() {
},
})
.to_string();
let mut child = Command::new(binary())
let mut child = common::isolated(binary())
.env("XDG_CONFIG_HOME", no_user_config())
.current_dir(path)
.args(["hook", "claude-code"])
Expand Down
12 changes: 5 additions & 7 deletions rust/ocomment/tests/gate.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
//! The three things a gate needs besides a verdict: a way to narrow itself to what a branch changed, a refusal to be silently green, and numbers a later step can read.

use std::{
fs,
path::Path,
process::{Command, Output},
};
mod common;

use std::{fs, path::Path, process::Output};
use tempfile::TempDir;

fn binary() -> &'static str {
Expand All @@ -23,7 +21,7 @@ fn no_user_config() -> &'static std::path::Path {
}

fn git(directory: &Path, arguments: &[&str]) {
let output = Command::new("git")
let output = common::isolated("git")
.current_dir(directory)
.args(arguments)
.output()
Expand All @@ -44,7 +42,7 @@ fn run(directory: &Path, arguments: &[&str]) -> Output {
arguments.push("--format");
arguments.push("human");
}
Command::new(binary())
common::isolated(binary())
.env("XDG_CONFIG_HOME", no_user_config())
.current_dir(directory)
.args(&arguments)
Expand Down
6 changes: 4 additions & 2 deletions rust/ocomment/tests/hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
//! The report is an instruction rather than a listing: the verb on each line is the rule that decided the comment, and a comment that only had to move must not be reported as one to delete.
//! The hook is silent unless it has something to say — a hook that spoke on every event would stand between an agent and every file it touched.

mod common;

use serde_json::{Value, json};
use std::{path::Path, process::Command};
use std::path::Path;

/// The `PATH` a run under test is given.
///
Expand Down Expand Up @@ -47,7 +49,7 @@ fn project() -> tempfile::TempDir {

fn run(directory: &Path, arguments: &[&str], stdin: &str) -> (String, String, i32) {
use std::io::Write;
let mut child = Command::new(binary())
let mut child = common::isolated(binary())
.env("XDG_CONFIG_HOME", no_user_config())
.current_dir(directory)
.env("PATH", test_path())
Expand Down
Loading
Loading