flag_cluster: fix the three findings, and pin the mordant pack by rev

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" }]

crates/scarlet/src/main.rs

@@ -86,23 +86,108 @@ struct RunArgs {
     args: Vec<String>,
 }
 
-#[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<String>,
+        action: FileAction,
+    },
+}
+
 struct FmtArgs {
-    path: Option<String>,
-    /// 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<Self, clap::Error> {
+        let target = if m.get_flag("stdin") {
+            FmtTarget::Stdin
+        } else {
+            FmtTarget::Files {
+                path: m.get_one::<String>("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 <file>`");
             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);
     }
 }

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" }

crates/scarlet_core/src/bytecode/compiler/mod.rs

@@ -396,6 +396,18 @@ struct ElabFrame {
     frame_closures: Vec<ClosureSite>,
 }
 
+// `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,