Skip to content

ci: add clippy #7675

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Jul 20, 2025
Merged

ci: add clippy #7675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
037c59f
ci: add clippy
Bushuo Jul 19, 2025
6db30a2
fix: warning to show ci failure
Bushuo Jul 19, 2025
3abe002
fix: run `cargo clippy --fix`
Bushuo Jul 19, 2025
91822a1
fix: rogue comment
Bushuo Jul 19, 2025
976aad1
fix: redundant match
Bushuo Jul 19, 2025
2a540e5
fix: too many args by introducing a wrapper struct
Bushuo Jul 19, 2025
ff9cdae
chore: run cargo fmt
Bushuo Jul 19, 2025
5ff6bac
refactor: get_source_files split match
Bushuo Jul 19, 2025
118335e
refactor: get_source_files use `PackageSource.is_type_dev()`
Bushuo Jul 19, 2025
fb89e8c
refactor: simplify get_source_files
Bushuo Jul 19, 2025
0754018
chore: CHANGELOG
Bushuo Jul 20, 2025
dffc928
fix: warnings after rebase
Bushuo Jul 20, 2025
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
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ jobs:
OPAM_VERSION: 2.3.0
DUNE_PROFILE: release
RUST_BACKTRACE: "1"
RUSTFLAGS: "-Dwarnings"

steps:
- name: "Windows: Set git config"
Expand Down Expand Up @@ -131,6 +132,11 @@ jobs:
run: |
cargo build --manifest-path rewatch/Cargo.toml --target ${{ matrix.rust-target }} --release

- name: Lint rewatch
if: steps.rewatch-build-cache.outputs.cache-hit != 'true'
run: |
cargo clippy --manifest-path rewatch/Cargo.toml --all-targets --all-features

- name: Run rewatch unit tests
if: steps.rewatch-build-cache.outputs.cache-hit != 'true'
run: |
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@

- Configuration fields `bs-dependencies`, `bs-dev-dependencies` and `bsc-flags` are now deprecated in favor of `dependencies`, `dev-dependencies` and `compiler-flags`. https://github.com/rescript-lang/rescript/pull/7658

#### :house: Internal

- Add rust linting to CI with `clippy`. https://github.com/rescript-lang/rescript/pull/7675

# 12.0.0-beta.2

#### :boom: Breaking Change
Expand Down
35 changes: 17 additions & 18 deletions rewatch/src/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub struct CompilerArgs {
pub fn get_compiler_args(path: &Path) -> Result<String> {
let filename = &helpers::get_abs_path(path);
let package_root =
helpers::get_abs_path(&helpers::get_nearest_config(&path).expect("Couldn't find package root"));
helpers::get_abs_path(&helpers::get_nearest_config(path).expect("Couldn't find package root"));
let workspace_root = get_workspace_root(&package_root).map(|p| helpers::get_abs_path(&p));
let root_rescript_config =
packages::read_config(&workspace_root.to_owned().unwrap_or(package_root.to_owned()))?;
Expand All @@ -77,7 +77,7 @@ pub fn get_compiler_args(path: &Path) -> Result<String> {
let (ast_path, parser_args) = parser_args(
&rescript_config,
&root_rescript_config,
&relative_filename,
relative_filename,
&workspace_root,
workspace_root.as_ref().unwrap_or(&package_root),
&contents,
Expand All @@ -94,7 +94,7 @@ pub fn get_compiler_args(path: &Path) -> Result<String> {
&rescript_config,
&root_rescript_config,
&ast_path,
&relative_filename,
relative_filename,
is_interface,
has_interface,
&package_root,
Expand Down Expand Up @@ -216,7 +216,7 @@ pub fn initialize_build(

if show_progress {
if snapshot_output {
println!("Cleaned {}/{}", diff_cleanup, total_cleanup)
println!("Cleaned {diff_cleanup}/{total_cleanup}")
} else {
println!(
"{}{} {}Cleaned {}/{} {:.2}s",
Expand All @@ -234,7 +234,7 @@ pub fn initialize_build(
}

fn format_step(current: usize, total: usize) -> console::StyledObject<String> {
style(format!("[{}/{}]", current, total)).bold().dim()
style(format!("[{current}/{total}]")).bold().dim()
}

#[derive(Debug, Clone)]
Expand All @@ -254,23 +254,23 @@ impl fmt::Display for IncrementalBuildError {
match &self.kind {
IncrementalBuildErrorKind::SourceFileParseError => {
if self.snapshot_output {
write!(f, "{} Could not parse Source Files", LINE_CLEAR,)
write!(f, "{LINE_CLEAR} Could not parse Source Files",)
} else {
write!(f, "{} {}Could not parse Source Files", LINE_CLEAR, CROSS,)
write!(f, "{LINE_CLEAR} {CROSS}Could not parse Source Files",)
}
}
IncrementalBuildErrorKind::CompileError(Some(e)) => {
if self.snapshot_output {
write!(f, "{} Failed to Compile. Error: {e}", LINE_CLEAR,)
write!(f, "{LINE_CLEAR} Failed to Compile. Error: {e}",)
} else {
write!(f, "{} {}Failed to Compile. Error: {e}", LINE_CLEAR, CROSS,)
write!(f, "{LINE_CLEAR} {CROSS}Failed to Compile. Error: {e}",)
}
}
IncrementalBuildErrorKind::CompileError(None) => {
if self.snapshot_output {
write!(f, "{} Failed to Compile. See Errors Above", LINE_CLEAR,)
write!(f, "{LINE_CLEAR} Failed to Compile. See Errors Above",)
} else {
write!(f, "{} {}Failed to Compile. See Errors Above", LINE_CLEAR, CROSS,)
write!(f, "{LINE_CLEAR} {CROSS}Failed to Compile. See Errors Above",)
}
}
}
Expand Down Expand Up @@ -312,7 +312,7 @@ pub fn incremental_build(
Ok(_ast) => {
if show_progress {
if snapshot_output {
println!("Parsed {} source files", num_dirty_modules)
println!("Parsed {num_dirty_modules} source files")
} else {
println!(
"{}{} {}Parsed {} source files in {:.2}s",
Expand Down Expand Up @@ -370,7 +370,7 @@ pub fn incremental_build(
if log_enabled!(log::Level::Trace) {
for (module_name, module) in build_state.modules.iter() {
if module.compile_dirty {
println!("compile dirty: {}", module_name);
println!("compile dirty: {module_name}");
}
}
};
Expand Down Expand Up @@ -411,7 +411,7 @@ pub fn incremental_build(
if !compile_errors.is_empty() {
if show_progress {
if snapshot_output {
println!("Compiled {} modules", num_compiled_modules)
println!("Compiled {num_compiled_modules} modules")
} else {
println!(
"{}{} {}Compiled {} modules in {:.2}s",
Expand Down Expand Up @@ -439,7 +439,7 @@ pub fn incremental_build(
} else {
if show_progress {
if snapshot_output {
println!("Compiled {} modules", num_compiled_modules)
println!("Compiled {num_compiled_modules} modules")
} else {
println!(
"{}{} {}Compiled {} modules in {:.2}s",
Expand Down Expand Up @@ -485,9 +485,8 @@ fn log_deprecations(build_state: &BuildState) {

fn log_deprecated_config_field(package_name: &str, field_name: &str, new_field_name: &str) {
let warning = format!(
"The field '{}' found in the package config of '{}' is deprecated and will be removed in a future version.\n\
Use '{}' instead.",
field_name, package_name, new_field_name
"The field '{field_name}' found in the package config of '{package_name}' is deprecated and will be removed in a future version.\n\
Use '{new_field_name}' instead."
);
println!("\n{}", style(warning).yellow());
}
Expand Down
153 changes: 72 additions & 81 deletions rewatch/src/build/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,8 @@ pub fn compiler_args(

specs
.iter()
.map(|spec| {
return vec![
.flat_map(|spec| {
vec![
"-bs-package-output".to_string(),
format!(
"{}:{}:{}",
Expand All @@ -437,9 +437,8 @@ pub fn compiler_args(
},
root_config.get_suffix(spec),
),
];
]
})
.flatten()
.collect()
};

Expand Down Expand Up @@ -614,8 +613,7 @@ fn compile_file(
Err(stderr.to_string() + &stdout)
}
Err(e) => Err(format!(
"Could not compile file. Error: {}. Path to AST: {:?}",
e, ast_path
"Could not compile file. Error: {e}. Path to AST: {ast_path:?}"
)),
Ok(x) => {
let err = std::str::from_utf8(&x.stderr)
Expand All @@ -633,15 +631,15 @@ fn compile_file(
// because editor tooling doesn't support namespace entries yet
// we just remove the @ for now. This makes sure the editor support
// doesn't break
.join(format!("{}.cmi", module_name)),
ocaml_build_path_abs.join(format!("{}.cmi", module_name)),
.join(format!("{module_name}.cmi")),
ocaml_build_path_abs.join(format!("{module_name}.cmi")),
);
let _ = std::fs::copy(
package
.get_build_path()
.join(dir)
.join(format!("{}.cmj", module_name)),
ocaml_build_path_abs.join(format!("{}.cmj", module_name)),
.join(format!("{module_name}.cmj")),
ocaml_build_path_abs.join(format!("{module_name}.cmj")),
);
let _ = std::fs::copy(
package
Expand All @@ -650,98 +648,91 @@ fn compile_file(
// because editor tooling doesn't support namespace entries yet
// we just remove the @ for now. This makes sure the editor support
// doesn't break
.join(format!("{}.cmt", module_name)),
ocaml_build_path_abs.join(format!("{}.cmt", module_name)),
.join(format!("{module_name}.cmt")),
ocaml_build_path_abs.join(format!("{module_name}.cmt")),
);
} else {
let _ = std::fs::copy(
package
.get_build_path()
.join(dir)
.join(format!("{}.cmti", module_name)),
ocaml_build_path_abs.join(format!("{}.cmti", module_name)),
.join(format!("{module_name}.cmti")),
ocaml_build_path_abs.join(format!("{module_name}.cmti")),
);
let _ = std::fs::copy(
package
.get_build_path()
.join(dir)
.join(format!("{}.cmi", module_name)),
ocaml_build_path_abs.join(format!("{}.cmi", module_name)),
.join(format!("{module_name}.cmi")),
ocaml_build_path_abs.join(format!("{module_name}.cmi")),
);
}

match &module.source_type {
SourceType::SourceFile(SourceFile {
interface: Some(Interface { path, .. }),
..
}) => {
// we need to copy the source file to the build directory.
// editor tools expects the source file in lib/bs for finding the current package
// and in lib/ocaml when referencing modules in other packages
let _ = std::fs::copy(
Path::new(&package.path).join(path),
package.get_build_path().join(path),
)
.expect("copying source file failed");

let _ = std::fs::copy(
Path::new(&package.path).join(path),
package
.get_ocaml_build_path()
.join(std::path::Path::new(path).file_name().unwrap()),
)
.expect("copying source file failed");
}
_ => (),
if let SourceType::SourceFile(SourceFile {
interface: Some(Interface { path, .. }),
..
}) = &module.source_type
{
// we need to copy the source file to the build directory.
// editor tools expects the source file in lib/bs for finding the current package
// and in lib/ocaml when referencing modules in other packages
let _ = std::fs::copy(
Path::new(&package.path).join(path),
package.get_build_path().join(path),
)
.expect("copying source file failed");

let _ = std::fs::copy(
Path::new(&package.path).join(path),
package
.get_ocaml_build_path()
.join(std::path::Path::new(path).file_name().unwrap()),
)
.expect("copying source file failed");
}
match &module.source_type {
SourceType::SourceFile(SourceFile {
implementation: Implementation { path, .. },
..
}) => {
// we need to copy the source file to the build directory.
// editor tools expects the source file in lib/bs for finding the current package
// and in lib/ocaml when referencing modules in other packages
let _ = std::fs::copy(
Path::new(&package.path).join(path),
package.get_build_path().join(path),
)
.expect("copying source file failed");

let _ = std::fs::copy(
Path::new(&package.path).join(path),
package
.get_ocaml_build_path()
.join(std::path::Path::new(path).file_name().unwrap()),
)
.expect("copying source file failed");
}
_ => (),
if let SourceType::SourceFile(SourceFile {
implementation: Implementation { path, .. },
..
}) = &module.source_type
{
// we need to copy the source file to the build directory.
// editor tools expects the source file in lib/bs for finding the current package
// and in lib/ocaml when referencing modules in other packages
let _ = std::fs::copy(
Path::new(&package.path).join(path),
package.get_build_path().join(path),
)
.expect("copying source file failed");

let _ = std::fs::copy(
Path::new(&package.path).join(path),
package
.get_ocaml_build_path()
.join(std::path::Path::new(path).file_name().unwrap()),
)
.expect("copying source file failed");
}

// copy js file
root_package.config.get_package_specs().iter().for_each(|spec| {
if spec.in_source {
match &module.source_type {
SourceType::SourceFile(SourceFile {
implementation: Implementation { path, .. },
..
}) => {
let source = helpers::get_source_file_from_rescript_file(
&Path::new(&package.path).join(path),
&root_package.config.get_suffix(spec),
);
let destination = helpers::get_source_file_from_rescript_file(
&package.get_build_path().join(path),
&root_package.config.get_suffix(spec),
);

if source.exists() {
let _ =
std::fs::copy(&source, &destination).expect("copying source file failed");
}
if let SourceType::SourceFile(SourceFile {
implementation: Implementation { path, .. },
..
}) = &module.source_type
{
let source = helpers::get_source_file_from_rescript_file(
&Path::new(&package.path).join(path),
&root_package.config.get_suffix(spec),
);
let destination = helpers::get_source_file_from_rescript_file(
&package.get_build_path().join(path),
&root_package.config.get_suffix(spec),
);

if source.exists() {
let _ = std::fs::copy(&source, &destination).expect("copying source file failed");
}
_ => (),
}
}
});
Expand Down
4 changes: 2 additions & 2 deletions rewatch/src/build/compile/dependency_cycle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ fn find_shortest_cycle(modules: &Vec<(&String, &Module)>) -> Vec<String> {
}

// Skip nodes with no incoming edges
if in_degrees.get(&start_node).map_or(true, |&d| d == 0) {
if in_degrees.get(&start_node).is_none_or(|&d| d == 0) {
no_cycle_cache.insert(start_node.clone());
continue;
}

if let Some(cycle) = find_cycle_bfs(&start_node, &graph, current_shortest_length) {
if let Some(cycle) = find_cycle_bfs(start_node, &graph, current_shortest_length) {
if shortest_cycle.is_empty() || cycle.len() < shortest_cycle.len() {
shortest_cycle = cycle.clone();
current_shortest_length = cycle.len();
Expand Down
Loading