Skip to content

Rust implementation of "rescript format" #7603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
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
69 changes: 36 additions & 33 deletions cli/rescript.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,41 +7,44 @@ import { bsc_exe, rescript_exe } from "./common/bins.js";

const args = process.argv.slice(2);

const firstPositionalArgIndex = args.findIndex(arg => !arg.startsWith("-"));

try {
if (firstPositionalArgIndex !== -1) {
const subcommand = args[firstPositionalArgIndex];
const subcommandWithArgs = args.slice(firstPositionalArgIndex);

if (
subcommand === "build" ||
subcommand === "watch" ||
subcommand === "clean" ||
subcommand === "compiler-args"
) {
child_process.execFileSync(
rescript_exe,
[...subcommandWithArgs, "--bsc-path", bsc_exe],
{
stdio: "inherit",
},
);
} else {
child_process.execFileSync(rescript_exe, [...args], {
stdio: "inherit",
});
}
} else {
// no subcommand means build subcommand
child_process.execFileSync(rescript_exe, [...args, "--bsc-path", bsc_exe], {
/**
* @param {string[]} cmdArgs
*/
function execRescript(cmdArgs) {
console.log(cmdArgs)
try {
child_process.execFileSync(rescript_exe, cmdArgs, {
stdio: "inherit",
});
} catch (err) {
if (err.status !== undefined) {
process.exit(err.status); // Pass through the exit code
} else {
process.exit(1); // Generic error
}
}
} catch (err) {
if (err.status !== undefined) {
process.exit(err.status); // Pass through the exit code
} else {
process.exit(1); // Generic error
}

const firstPositionalArgIndex = args.findIndex(arg => !arg.startsWith("-"));

if (firstPositionalArgIndex !== -1) {
const subcommand = args[firstPositionalArgIndex];
const subcommandWithArgs = args.slice(firstPositionalArgIndex);


switch (subcommand) {
case "build":
case "watch":
case "clean":
case "compiler-args":
case "format":
execRescript([...subcommandWithArgs, "--bsc-path", bsc_exe]);
break;
default:
execRescript(args);
break;
}
} else {
// no subcommand means build subcommand
execRescript([...args, "--bsc-path", bsc_exe]);
}
19 changes: 18 additions & 1 deletion rewatch/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions rewatch/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ log = { version = "0.4.17" }
notify = { version = "5.1.0", features = ["serde"] }
notify-debouncer-mini = { version = "0.2.0" }
rayon = "1.6.1"
num_cpus = "1.17.0"
regex = "1.7.1"
serde = { version = "1.0.152", features = ["derive"] }
serde_derive = "1.0.152"
Expand Down
20 changes: 16 additions & 4 deletions rewatch/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,23 @@ pub enum Command {
#[command(flatten)]
snapshot_output: SnapshotOutputArg,
},
/// Alias to `legacy format`.
#[command(disable_help_flag = true)]
/// Formats ReScript files.
Format {
#[arg(allow_hyphen_values = true, num_args = 0..)]
format_args: Vec<OsString>,
/// Read the code from stdin and print the formatted code to stdout.
#[arg(long)]
stdin: Option<String>,
/// Format the whole project.
#[arg(short = 'a', long)]
all: bool,
/// Check formatting for file or the whole project. Use `--all` to check the whole project.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--all is actually -all in bsb.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this will be a breaking changes I would say.

#[arg(short = 'c', long)]
check: bool,
/// Files to format.
files: Vec<String>,
#[command(flatten)]
bsc_path: BscPathArg,
#[command(flatten)]
folder: FolderArg,
},
/// Alias to `legacy dump`.
#[command(disable_help_flag = true)]
Expand Down
147 changes: 147 additions & 0 deletions rewatch/src/format_cmd.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
use std::sync::atomic::{AtomicUsize, Ordering};
use anyhow::{bail, Result};
use std::path::{Path, PathBuf};
use std::process::Command;
use std::io::{self, Read, Write};
use std::fs;
use rayon::prelude::*;
use crate::helpers;
use num_cpus;

pub fn run(
stdin_path: Option<String>,
all: bool,
check: bool,
files: Vec<String>,
bsc_path: Option<PathBuf>,
path: PathBuf,
) -> Result<()> {
let project_root = helpers::get_abs_path(&path);
let workspace_root = helpers::get_workspace_root(&project_root);

let bsc_exe = match bsc_path {
Some(path) => helpers::get_abs_path(&path),
None => helpers::get_bsc(&project_root, &workspace_root),
};

if check && stdin_path.is_some() {
bail!("format --stdin cannot be used with --check flag");
}

if all {
if stdin_path.is_some() || !files.is_empty() {
bail!("format --all can not be in use with other flags");
}
format_all(&bsc_exe, check)?;
} else if stdin_path.is_some() {
format_stdin(&bsc_exe, stdin_path.unwrap())?;
} else {
format_files(&bsc_exe, files, check)?;
}

Ok(())
}



fn format_all(bsc_exe: &Path, check: bool) -> Result<()> {
let output = Command::new(std::env::current_exe()?)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewatch doesn't have an info command like rescript did have. So this call won't work and it also is not future proof as this won't work for monorepos.
@jfrolich would it make sense to call packages::make() instead and iterate through all packages?

Copy link
Member Author

@cknitt cknitt Jul 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
Also realized this yesterday. Didn't test with --all before. 🤦‍♂️
Yes, this needs to be implemented on the rewatch side.

.arg("info")
.arg("-list-files")
.output()?;

if !output.status.success() {
io::stderr().write_all(&output.stderr)?;
bail!("Failed to list files");
}

let files_str = String::from_utf8_lossy(&output.stdout);
let files: Vec<String> = files_str
.split('\n')
.filter(|s| !s.trim().is_empty())
.map(|s| s.trim().to_string())
.collect();

format_files(bsc_exe, files, check)?;
Ok(())
}

fn format_stdin(bsc_exe: &Path, stdin_path: String) -> Result<()> {
let mut input = String::new();
io::stdin().read_to_string(&mut input)?;

let mut cmd = Command::new(bsc_exe);
cmd.arg("-format").arg(&stdin_path);
cmd.stdin(std::process::Stdio::piped());
cmd.stdout(std::process::Stdio::piped());
cmd.stderr(std::process::Stdio::piped());

let mut child = cmd.spawn()?;
let mut stdin = child.stdin.take().unwrap();
std::thread::spawn(move || {
stdin.write_all(input.as_bytes()).unwrap();
});

let output = child.wait_with_output()?;

if output.status.success() {
io::stdout().write_all(&output.stdout)?;
}
else {
io::stderr().write_all(&output.stderr)?;
bail!("bsc exited with an error");
}

Ok(())
}

fn format_files(bsc_exe: &Path, files: Vec<String>, check: bool) -> Result<()> {
let batch_size = 4 * num_cpus::get();
let incorrectly_formatted_files = AtomicUsize::new(0);

files.par_chunks(batch_size).try_for_each(|batch| {
batch.iter().try_for_each(|file| {
let mut cmd = Command::new(bsc_exe);
if check {
cmd.arg("-format").arg(file);
}
else {
cmd.arg("-o").arg(file).arg("-format").arg(file);
}

let output = cmd.output()?;

if output.status.success() {
if check {
let original_content = fs::read_to_string(file)?;
let formatted_content = String::from_utf8_lossy(&output.stdout);
if original_content != formatted_content {
eprintln!("[format check] {}", file);
incorrectly_formatted_files.fetch_add(1, Ordering::SeqCst);
}
}
}
else {
io::stderr().write_all(&output.stderr)?;
bail!("bsc exited with an error for file {}", file);
}
Ok(()) as Result<()>
})
})?;

let count = incorrectly_formatted_files.load(Ordering::SeqCst);
if count > 0 {
if count == 1 {
eprintln!("The file listed above needs formatting");
}
else {
eprintln!(
"The {} files listed above need formatting",
count
);
}
bail!("Formatting check failed");
}

Ok(())
}
1 change: 1 addition & 0 deletions rewatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ pub mod lock;
pub mod queue;
pub mod sourcedirs;
pub mod watcher;
pub mod format_cmd;
18 changes: 11 additions & 7 deletions rewatch/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{
path::{Path, PathBuf},
};

use rewatch::{build, cli, cmd, lock, watcher};
use rewatch::{build, cli, cmd, lock, watcher, format_cmd};

fn main() -> Result<()> {
let args = cli::Cli::parse();
Expand All @@ -20,7 +20,8 @@ fn main() -> Result<()> {
.target(env_logger::fmt::Target::Stdout)
.init();

let command = args.command.unwrap_or(cli::Command::Build(args.build_args));

let command = args.command.unwrap_or_else(|| cli::Command::Build(args.build_args));

// The 'normal run' mode will show the 'pretty' formatted progress. But if we turn off the log
// level, we should never show that.
Expand Down Expand Up @@ -112,11 +113,14 @@ fn main() -> Result<()> {
let code = build::pass_through_legacy(legacy_args);
std::process::exit(code);
}
cli::Command::Format { mut format_args } => {
format_args.insert(0, "format".into());
let code = build::pass_through_legacy(format_args);
std::process::exit(code);
}
cli::Command::Format {
stdin,
all,
check,
files,
bsc_path,
folder: path,
} => format_cmd::run(stdin, all, check, files, bsc_path.as_ref().map(|s| PathBuf::from(s.clone())), PathBuf::from(path.folder)),
cli::Command::Dump { mut dump_args } => {
dump_args.insert(0, "dump".into());
let code = build::pass_through_legacy(dump_args);
Expand Down
2 changes: 1 addition & 1 deletion scripts/format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ shopt -s extglob
dune build @fmt --auto-promote

files=$(find runtime tests -type f \( -name "*.res" -o -name "*.resi" \) ! -name "syntaxErrors*" ! -name "generated_mocha_test.res" ! -path "tests/syntax_tests*" ! -path "tests/analysis_tests/tests*" ! -path "*/node_modules/*")
./cli/rescript-legacy.js format $files
./cli/rescript.js format $files

yarn format
2 changes: 1 addition & 1 deletion scripts/format_check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ case "$(uname -s)" in

echo "Checking ReScript code formatting..."
files=$(find runtime tests -type f \( -name "*.res" -o -name "*.resi" \) ! -name "syntaxErrors*" ! -name "generated_mocha_test.res" ! -path "tests/syntax_tests*" ! -path "tests/analysis_tests/tests*" ! -path "*/node_modules/*")
if ./cli/rescript-legacy.js format -check $files; then
if ./cli/rescript.js format --check $files; then
printf "${successGreen}✅ ReScript code formatting ok.${reset}\n"
else
printf "${warningYellow}⚠️ ReScript code formatting issues found. Run 'make format' to fix.${reset}\n"
Expand Down
Loading