diff --git a/Cargo.toml b/Cargo.toml index ac8cb685..09b9e8a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,4 +28,19 @@ strip = false debug = true [workspace.metadata.dylint] -libraries = [{ git = "https://github.com/scarletindustries/mordant" }] +# Pinned to a rev. Unpinned, `cargo dylint` resolves whatever the remote's HEAD +# is at the moment each machine last fetched, so the mordant gate silently +# changes underneath a tree that did not move: `flag_cluster` landing on mordant +# master turned this repo's CI red with no commit here at all. It also +# accumulates one `~/.cargo/git/checkouts` directory per rev, which is how the +# same documented command comes to run two different lint sets in two worktrees +# minutes apart. +# +# Bumping is now a deliberate act, and a fix on mordant master does not reach +# this repo until someone moves this line. +# +# **Changing this line rebuilds nothing.** With a library already built, moving +# the rev and re-running `cargo dylint` loads the dylib already in +# `target/dylint`, so the bump leaves every run using the old pack. +# `rm -rf target/dylint` is what makes it take, once per worktree. +libraries = [{ git = "https://github.com/scarletindustries/mordant", rev = "eba57c339ebdfc7f2ce9a814507e028db0d21867" }] diff --git a/crates/scarlet/src/main.rs b/crates/scarlet/src/main.rs index fc3a09b0..1df1e8e9 100644 --- a/crates/scarlet/src/main.rs +++ b/crates/scarlet/src/main.rs @@ -86,23 +86,108 @@ struct RunArgs { args: Vec, } -#[derive(Args)] +/// What `al fmt` does with each file it formatted. +#[derive(Clone, Copy)] +enum FileAction { + /// Rewrite the file, when formatting changed it. + WriteBack, + /// `--stdout`: print the formatted text and leave the file alone. + Print, + /// `--check`: name the files that need formatting and exit 1 if any do. + Check, +} + +/// What `al fmt` was pointed at, and what to do with it. +/// +/// The three flags that select these are mutually exclusive, so the parse +/// boundary is the last place a combination of them can be expressed: +/// [`FmtArgs::target`] is what `cmd_fmt` reads. `--stdin --stdout` used to +/// parse and then silently ignore `--stdout`, and `path` was only excluded +/// alongside `--stdin` by a clap attribute; neither has a spelling here. +enum FmtTarget { + /// `--stdin`: format stdin and print the result. Takes no path. + Stdin, + /// Walk `path` (default `.`) for `.scrl` files and apply `action` to each. + Files { + path: Option, + action: FileAction, + }, +} + struct FmtArgs { - path: Option, - /// Print formatted output instead of writing to files - #[arg(long)] - stdout: bool, - /// Read input from stdin instead of a file - #[arg(long, conflicts_with_all = ["check", "path"])] - stdin: bool, - /// Check if files are formatted (exit 1 if not) - #[arg(long, conflicts_with = "stdout")] - check: bool, - /// Print debug information about tokens - #[arg(long)] + target: FmtTarget, + /// `--debug`: dump the token stream of each input before formatting it. debug: bool, } +/// Hand-written because `FmtArgs` holds [`FmtTarget`] rather than one bool per +/// flag, which no `#[derive(Args)]` spelling produces. The `Command` this +/// builds is what `--help` and `scarlet man` render, so each arg keeps its +/// help text. +impl clap::FromArgMatches for FmtArgs { + fn from_arg_matches(m: &clap::ArgMatches) -> Result { + let target = if m.get_flag("stdin") { + FmtTarget::Stdin + } else { + FmtTarget::Files { + path: m.get_one::("path").cloned(), + action: if m.get_flag("check") { + FileAction::Check + } else if m.get_flag("stdout") { + FileAction::Print + } else { + FileAction::WriteBack + }, + } + }; + Ok(FmtArgs { + target, + debug: m.get_flag("debug"), + }) + } + + fn update_from_arg_matches(&mut self, m: &clap::ArgMatches) -> Result<(), clap::Error> { + *self = Self::from_arg_matches(m)?; + Ok(()) + } +} + +impl Args for FmtArgs { + fn augment_args(cmd: clap::Command) -> clap::Command { + cmd.arg(clap::Arg::new("path").value_name("PATH").index(1)) + .arg( + clap::Arg::new("stdout") + .long("stdout") + .help("Print formatted output instead of writing to files") + .action(clap::ArgAction::SetTrue) + .conflicts_with_all(["check", "stdin"]), + ) + .arg( + clap::Arg::new("stdin") + .long("stdin") + .help("Read input from stdin instead of a file") + .action(clap::ArgAction::SetTrue) + .conflicts_with_all(["check", "path", "stdout"]), + ) + .arg( + clap::Arg::new("check") + .long("check") + .help("Check if files are formatted (exit 1 if not)") + .action(clap::ArgAction::SetTrue), + ) + .arg( + clap::Arg::new("debug") + .long("debug") + .help("Print debug information about tokens") + .action(clap::ArgAction::SetTrue), + ) + } + + fn augment_args_for_update(cmd: clap::Command) -> clap::Command { + Self::augment_args(cmd) + } +} + /// Resolve a diagnostic's module provenance to (path, text) so it renders /// against the file its span actually points into. fn resolve_diagnostic_source(key: &scarlet::module::ModuleKey) -> Option<(PathBuf, String)> { @@ -329,10 +414,10 @@ fn main() -> process::ExitCode { Some(Commands::Build { entrypoint }) => { eprintln!("warning: `al build` is deprecated; use `al fmt --stdout `"); cmd_fmt(FmtArgs { - path: Some(entrypoint), - stdout: true, - stdin: false, - check: false, + target: FmtTarget::Files { + path: Some(entrypoint), + action: FileAction::Print, + }, debug: false, }); } @@ -536,47 +621,53 @@ fn compile_one( } } -fn cmd_fmt(args: FmtArgs) { - if args.stdin { - let mut content = String::new(); - if let Err(e) = io::stdin().read_to_string(&mut content) { - die(format!("Error reading stdin: {e}")); - } - if args.debug { - dump_tokens(&content); +/// `al fmt --stdin`: format stdin and print the result. Separate from the file +/// walk because there is no file to write back to, check, or name in an error. +fn fmt_stdin(debug: bool) { + let mut content = String::new(); + if let Err(e) = io::stdin().read_to_string(&mut content) { + die(format!("Error reading stdin: {e}")); + } + if debug { + dump_tokens(&content); + } + match formatter::format(&content) { + formatter::FormatResult::Formatted { output } => { + print!("{output}"); + let _ = io::stdout().flush(); } - match formatter::format(&content) { - formatter::FormatResult::Formatted { output } => { - print!("{output}"); - let _ = io::stdout().flush(); - } - formatter::FormatResult::ParseFailed { errors } => { - for d in &errors { - eprintln!("{}", render_fmt_diagnostic("stdin", d)); - } - process::exit(1); - } - formatter::FormatResult::CommentsLost { comment } => { - // Pass the input through untouched so no comment is deleted. - eprintln!( - "formatter bug: formatting would delete the comment `{comment}`; input left unchanged" - ); - print!("{content}"); - let _ = io::stdout().flush(); - process::exit(1); - } - formatter::FormatResult::OutputInvalid { detail } => { - // Pass the input through so valid source is never replaced. - eprintln!("formatter bug: {detail}; input left unchanged"); - print!("{content}"); - let _ = io::stdout().flush(); - process::exit(1); + formatter::FormatResult::ParseFailed { errors } => { + for d in &errors { + eprintln!("{}", render_fmt_diagnostic("stdin", d)); } + process::exit(1); + } + formatter::FormatResult::CommentsLost { comment } => { + // Pass the input through untouched so no comment is deleted. + eprintln!( + "formatter bug: formatting would delete the comment `{comment}`; input left unchanged" + ); + print!("{content}"); + let _ = io::stdout().flush(); + process::exit(1); + } + formatter::FormatResult::OutputInvalid { detail } => { + // Pass the input through so valid source is never replaced. + eprintln!("formatter bug: {detail}; input left unchanged"); + print!("{content}"); + let _ = io::stdout().flush(); + process::exit(1); } - return; } +} + +fn cmd_fmt(args: FmtArgs) { + let (path, action) = match args.target { + FmtTarget::Stdin => return fmt_stdin(args.debug), + FmtTarget::Files { path, action } => (path, action), + }; - let path = args.path.as_deref().unwrap_or("."); + let path = path.as_deref().unwrap_or("."); let files = find_scarlet_files(path).unwrap_or_else(|e| die(e)); @@ -626,20 +717,24 @@ fn cmd_fmt(args: FmtArgs) { } formatter::FormatResult::Formatted { output } => { let changed = output != content; - if args.check { - if changed { - println!("{} needs formatting", file.display()); - needs_formatting = true; + match action { + FileAction::Check => { + if changed { + println!("{} needs formatting", file.display()); + needs_formatting = true; + } } - } else if args.stdout { - print!("{output}"); - } else if changed { - if let Err(e) = fs::write(file, &output) { - eprintln!("Error writing {}: {e}", file.display()); - has_errors = true; - continue; + FileAction::Print => print!("{output}"), + FileAction::WriteBack => { + if changed { + if let Err(e) = fs::write(file, &output) { + eprintln!("Error writing {}: {e}", file.display()); + has_errors = true; + continue; + } + println!("Formatted {}", file.display()); + } } - println!("Formatted {}", file.display()); } } } @@ -649,7 +744,7 @@ fn cmd_fmt(args: FmtArgs) { process::exit(1); } - if args.check && needs_formatting { + if matches!(action, FileAction::Check) && needs_formatting { process::exit(1); } } diff --git a/crates/scarlet_core/Cargo.toml b/crates/scarlet_core/Cargo.toml index 9e824702..7c3811e2 100644 --- a/crates/scarlet_core/Cargo.toml +++ b/crates/scarlet_core/Cargo.toml @@ -8,6 +8,12 @@ edition = "2024" # the counter on through either crate. alloc-counter = ["scarlet_vm/alloc-counter"] +# `cargo dylint` sets `--cfg dylint_lib="mordant"` and plain cargo does not, so +# the `#[cfg_attr(dylint_lib = "mordant", ...)]` suppression on `Compiler` is an +# unexpected cfg to every other build — and `clippy -- -D warnings` fails on it. +[lints.rust] +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(dylint_lib, values("mordant"))'] } + [dependencies] scarlet_syntax = { path = "../scarlet_syntax" } scarlet_types = { path = "../scarlet_types" } diff --git a/crates/scarlet_core/src/bytecode/compiler/mod.rs b/crates/scarlet_core/src/bytecode/compiler/mod.rs index a5da1d6d..7f783270 100644 --- a/crates/scarlet_core/src/bytecode/compiler/mod.rs +++ b/crates/scarlet_core/src/bytecode/compiler/mod.rs @@ -396,6 +396,18 @@ struct ElabFrame { frame_closures: Vec, } +// `flag_cluster` fires on the four bool fields below, and the answer is that +// all 16 states are legal: there is no illegal combination for an enum to +// name. They are knobs on four unrelated subsystems — pipeline truncation, +// hover-fact collection, namespace scoping, and a marker for one transient +// walk — and both corners are reachable. All unset is an ordinary `compile`; +// all set is `IncrementalSession::new_from_source`, which builds a +// `check_only` compiler, sets `collect_hover_facts`, then calls +// `register_prelude`, which runs under `with_retained_namespaces`, inside +// which `analyse_module` sets `walking_module_statements`. The mode bools in +// this struct that did carry an invariant are already enums — see +// [`UnusedBindings`] and [`ModuleScope`]; these four carry none between them. +#[cfg_attr(dylint_lib = "mordant", allow(flag_cluster))] pub struct Compiler { // --- Codegen state --- pub(super) program: Program, diff --git a/crates/scarlet_vm/src/vm/http.rs b/crates/scarlet_vm/src/vm/http.rs index ced7d599..10f9ff9d 100644 --- a/crates/scarlet_vm/src/vm/http.rs +++ b/crates/scarlet_vm/src/vm/http.rs @@ -211,8 +211,8 @@ fn parse_head_window(t: &H1, a: &mut ProcHeap, win: &ByteWindow, off: i64) -> Va t.head_flags.instantiate( a, &[ - Value::bool(flags.conn_close), - Value::bool(flags.conn_keep_alive), + Value::bool(flags.conn.has_close()), + Value::bool(flags.conn.has_keep_alive()), Value::bool(flags.expect_100_continue), ], ) @@ -230,14 +230,60 @@ fn parse_head_window(t: &H1, a: &mut ProcHeap, win: &ByteWindow, off: i64) -> Va ) } +/// Which of the two persistence options the head's `Connection` fields named. +/// All four are observable: `Connection` is a token list that may also be +/// repeated, so a peer can genuinely send both, and `Both` records that +/// instead of resolving it. RFC 9112 §9.3 gives `close` precedence over +/// `keep-alive`, and that decision stays in `scarlet/http/h1.should_close`. +/// +/// One field rather than two bools because the pair is one header's token +/// set: neither half is separately assignable, and the four cases are the +/// four the parser can witness. +#[derive(Default, Clone, Copy, PartialEq)] +enum ConnTokens { + #[default] + Neither, + Close, + KeepAlive, + Both, +} + +impl ConnTokens { + const fn new(close: bool, keep_alive: bool) -> Self { + match (close, keep_alive) { + (false, false) => Self::Neither, + (true, false) => Self::Close, + (false, true) => Self::KeepAlive, + (true, true) => Self::Both, + } + } + + const fn has_close(self) -> bool { + matches!(self, Self::Close | Self::Both) + } + + const fn has_keep_alive(self) -> bool { + matches!(self, Self::KeepAlive | Self::Both) + } + + /// A repeated `Connection` field means the same list split across lines, + /// so the tokens accumulate. Union, never replacement: `close` on a later + /// field must not unsee `keep-alive` on an earlier one. + const fn union(self, other: Self) -> Self { + Self::new( + self.has_close() | other.has_close(), + self.has_keep_alive() | other.has_keep_alive(), + ) + } +} + /// The `Connection`/`Expect` token-list answers an HTTP/1.1 server needs from /// every request head, recorded by `parse_header_block` while it already has /// each field's trimmed name and value in hand. Raw findings, not decisions: /// precedence lives in `scarlet/http/h1.should_close`. #[derive(Default, Clone, Copy, PartialEq)] struct HeadFlags { - conn_close: bool, - conn_keep_alive: bool, + conn: ConnTokens, expect_100_continue: bool, } @@ -331,8 +377,10 @@ fn parse_header_block( let name_bytes = &bytes[pos..colon]; if name_bytes.eq_ignore_ascii_case(b"connection") { let value_bytes = &bytes[vstart..vend]; - flags.conn_close |= has_token(value_bytes, b"close"); - flags.conn_keep_alive |= has_token(value_bytes, b"keep-alive"); + flags.conn = flags.conn.union(ConnTokens::new( + has_token(value_bytes, b"close"), + has_token(value_bytes, b"keep-alive"), + )); } else if name_bytes.eq_ignore_ascii_case(b"expect") { flags.expect_100_continue |= has_token(&bytes[vstart..vend], b"100-continue"); }