Skip to content

Resolve Rust 1.88 clippy string lints and format strings (svm + misc) #6910

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
steviez merged 6 commits into from
Jul 15, 2025
Merged

Resolve Rust 1.88 clippy string lints and format strings (svm + misc) #6910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7545635
clippy: uninlined_format_args (runtime, svm and related)
steviez Jul 10, 2025
5ef802a
some manual cleanup of trailing commas
steviez Jul 10, 2025
2d321be
Run fmt with format_strings to ensure <= 100 char
steviez Jul 10, 2025
b8c8705
Run cargo fmt on missed files
steviez Jul 15, 2025
42faf10
cleanup new file
steviez Jul 15, 2025
d459db0
rm trailing commas
steviez Jul 15, 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
69 changes: 40 additions & 29 deletions platform-tools-sdk/cargo-build-sbf/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ pub fn is_version_string(arg: &str) -> Result<(), String> {
if semver_re.is_match(arg) {
return Ok(());
}
Err("a version string may start with 'v' and contains major and minor version numbers separated by a dot, e.g. v1.32 or 1.32".to_string())
Err(
"a version string may start with 'v' and contains major and minor version numbers \
separated by a dot, e.g. v1.32 or 1.32"
.to_string(),
)
}

fn home_dir() -> PathBuf {
Expand Down Expand Up @@ -145,7 +149,7 @@ fn prepare_environment(
};

env::set_current_dir(root_dir).unwrap_or_else(|err| {
error!("Unable to set current directory to {}: {}", root_dir, err);
error!("Unable to set current directory to {root_dir}: {err}");
exit(1);
});

Expand All @@ -166,7 +170,7 @@ fn invoke_cargo(config: &Config) {
if corrupted_toolchain(config) {
error!(
"The Solana toolchain is corrupted. Please, run cargo-build-sbf with the \
--force-tools-install argument to fix it."
--force-tools-install argument to fix it."
);
exit(1);
}
Expand All @@ -188,10 +192,7 @@ fn invoke_cargo(config: &Config) {
);
let rustflags = env::var("RUSTFLAGS").ok().unwrap_or_default();
if env::var("RUSTFLAGS").is_ok() {
warn!(
"Removed RUSTFLAGS from cargo environment, because it overrides {}.",
cargo_target,
);
warn!("Removed RUSTFLAGS from cargo environment, because it overrides {cargo_target}.");
env::remove_var("RUSTFLAGS")
}
let target_rustflags = env::var(&cargo_target).ok();
Expand Down Expand Up @@ -256,7 +257,7 @@ fn invoke_cargo(config: &Config) {
);

if config.verbose {
debug!("{}", output);
debug!("{output}");
}
}

Expand All @@ -275,10 +276,13 @@ fn generate_program_name(package: &cargo_metadata::Package) -> Option<String> {
};

if let Some(other_crate) = other_crate_type {
warn!("Package '{}' has two crate types defined: cdylib and {}. \
This setting precludes link-time optimizations (LTO). Use cdylib for programs \
to be deployed and rlib for packages to be imported by other programs as libraries.",
package.name, other_crate);
warn!(
"Package '{}' has two crate types defined: cdylib and {}. This setting \
precludes link-time optimizations (LTO). Use cdylib for programs to be \
deployed and rlib for packages to be imported by other programs as \
libraries.",
package.name, other_crate
);
}

Some(&target.name)
Expand Down Expand Up @@ -311,7 +315,7 @@ fn build_solana(config: Config, manifest_path: Option<PathBuf>) {
}

let metadata = metadata_command.exec().unwrap_or_else(|err| {
error!("Failed to obtain package metadata: {}", err);
error!("Failed to obtain package metadata: {err}");
exit(1);
});

Expand Down Expand Up @@ -443,14 +447,21 @@ fn main() {
.long("skip-tools-install")
.takes_value(false)
.conflicts_with("force_tools_install")
.help("Skip downloading and installing platform-tools, assuming they are properly mounted"),
)
.arg(
Arg::new("no_rustup_override")
.help(
"Skip downloading and installing platform-tools, assuming they are properly \
mounted",
),
)
.arg(
Arg::new("no_rustup_override")
.long("no-rustup-override")
.takes_value(false)
.conflicts_with("force_tools_install")
.help("Do not use rustup to manage the toolchain. By default, cargo-build-sbf invokes rustup to find the Solana rustc using a `+solana` toolchain override. This flag disables that behavior."),
.help(
"Do not use rustup to manage the toolchain. By default, cargo-build-sbf \
invokes rustup to find the Solana rustc using a `+solana` toolchain \
override. This flag disables that behavior.",
),
)
.arg(
Arg::new("generate_child_script_on_failure")
Expand Down Expand Up @@ -521,16 +532,16 @@ fn main() {
Arg::new("optimize_size")
.long("optimize-size")
.takes_value(false)
.help("Optimize program for size. This option may reduce program size, potentially increasing CU consumption.")
)
.arg(
Arg::new("lto")
.long("lto")
.takes_value(false)
.help("Enable Link-Time Optimization (LTO) for all crates being built. \
This option may decrease program size and CU consumption. The default option is LTO \
disabled, as one may get mixed results with it.")
.help(
"Optimize program for size. This option may reduce program size, potentially \
increasing CU consumption.",
),
)
.arg(Arg::new("lto").long("lto").takes_value(false).help(
"Enable Link-Time Optimization (LTO) for all crates being built. This option may \
decrease program size and CU consumption. The default option is LTO disabled, as one \
may get mixed results with it.",
))
.get_matches_from(args);

let sbf_sdk: PathBuf = matches.value_of_t_or_exit("sbf_sdk");
Expand Down Expand Up @@ -605,8 +616,8 @@ fn main() {
};
let manifest_path: Option<PathBuf> = matches.value_of_t("manifest_path").ok();
if config.verbose {
debug!("{:?}", config);
debug!("manifest_path: {:?}", manifest_path);
debug!("{config:?}");
debug!("manifest_path: {manifest_path:?}");
}
build_solana(config, manifest_path);
}
22 changes: 13 additions & 9 deletions platform-tools-sdk/cargo-build-sbf/src/post_processing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,20 +85,25 @@ pub(crate) fn post_process(config: &Config, target_directory: &Path, program_nam
config.generate_child_script_on_failure,
);
if config.verbose {
debug!("{}", output);
debug!("{output}");
}
}

if config.dump && file_older_or_missing(&program_unstripped_so, &program_dump) {
let dump_script = config.sbf_sdk.join("scripts").join("dump.sh");
#[cfg(windows)]
{
error!("Using Bash scripts from within a program is not supported on Windows, skipping `--dump`.");
error!(
"Please run \"{} {} {}\" from a Bash-supporting shell, then re-run this command to see the processed program dump.",
"Using Bash scripts from within a program is not supported on Windows, \
skipping `--dump`."
);
error!(
"Please run \"{} {} {}\" from a Bash-supporting shell, then re-run this \
command to see the processed program dump.",
&dump_script.display(),
&program_unstripped_so.display(),
&program_dump.display());
&program_dump.display()
);
}
#[cfg(not(windows))]
{
Expand All @@ -108,7 +113,7 @@ pub(crate) fn post_process(config: &Config, target_directory: &Path, program_nam
config.generate_child_script_on_failure,
);
if config.verbose {
debug!("{}", output);
debug!("{output}");
}
}
postprocess_dump(&program_dump);
Expand All @@ -130,7 +135,7 @@ pub(crate) fn post_process(config: &Config, target_directory: &Path, program_nam
config.generate_child_script_on_failure,
);
if config.verbose {
debug!("{}", output);
debug!("{output}");
}
}

Expand Down Expand Up @@ -179,7 +184,7 @@ fn check_undefined_symbols(config: &Config, program: &Path) {
config.generate_child_script_on_failure,
);
if config.verbose {
debug!("{}", output);
debug!("{output}");
}
let mut unresolved_symbols: Vec<String> = Vec::new();
for line in output.lines() {
Expand All @@ -194,8 +199,7 @@ fn check_undefined_symbols(config: &Config, program: &Path) {
}
if !unresolved_symbols.is_empty() {
warn!(
"The following functions are undefined and not known syscalls {:?}.",
unresolved_symbols
"The following functions are undefined and not known syscalls {unresolved_symbols:?}."
);
warn!(" Calling them will trigger a run-time error.");
}
Expand Down
55 changes: 37 additions & 18 deletions platform-tools-sdk/cargo-build-sbf/src/toolchain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ fn validate_platform_tools_version(requested_version: &str, builtin_version: &st
}
let latest_version = get_latest_platform_tools_version().unwrap_or_else(|err| {
debug!(
"Can't get the latest version of platform-tools: {}. Using built-in version {}.",
err, builtin_version,
"Can't get the latest version of platform-tools: {err}. Using built-in version \
{builtin_version}."
);
builtin_version.to_string()
});
Expand All @@ -84,8 +84,8 @@ fn validate_platform_tools_version(requested_version: &str, builtin_version: &st
downloadable_version(requested_version)
} else {
warn!(
"Version {} is not valid, latest version is {}. Using the built-in version {}",
requested_version, latest_version, builtin_version,
"Version {requested_version} is not valid, latest version is {latest_version}. Using \
the built-in version {builtin_version}"
);
builtin_version.to_string()
}
Expand Down Expand Up @@ -128,14 +128,14 @@ fn install_if_missing(
) -> Result<(), String> {
if config.force_tools_install {
if target_path.is_dir() {
debug!("Remove directory {:?}", target_path);
debug!("Remove directory {target_path:?}");
fs::remove_dir_all(target_path).map_err(|err| err.to_string())?;
}
let source_base = config.sbf_sdk.join("dependencies");
if source_base.exists() {
let source_path = source_base.join(package);
if source_path.exists() {
debug!("Remove file {:?}", source_path);
debug!("Remove file {source_path:?}");
fs::remove_file(source_path).map_err(|err| err.to_string())?;
}
}
Expand All @@ -151,7 +151,7 @@ fn install_if_missing(
.next()
.is_none()
{
debug!("Remove directory {:?}", target_path);
debug!("Remove directory {target_path:?}");
fs::remove_dir(target_path).map_err(|err| err.to_string())?;
}

Expand All @@ -164,7 +164,7 @@ fn install_if_missing(
.unwrap_or(false)
{
if target_path.exists() {
debug!("Remove file {:?}", target_path);
debug!("Remove file {target_path:?}");
fs::remove_file(target_path).map_err(|err| err.to_string())?;
}
fs::create_dir_all(target_path).map_err(|err| err.to_string())?;
Expand Down Expand Up @@ -247,7 +247,7 @@ fn link_solana_toolchain(config: &Config) {
config.generate_child_script_on_failure,
);
if config.verbose {
debug!("{}", rustup_output);
debug!("{rustup_output}");
}
let mut do_link = true;
for line in rustup_output.lines() {
Expand All @@ -263,7 +263,7 @@ fn link_solana_toolchain(config: &Config) {
config.generate_child_script_on_failure,
);
if config.verbose {
debug!("{}", output);
debug!("{output}");
}
} else {
do_link = false;
Expand All @@ -284,7 +284,7 @@ fn link_solana_toolchain(config: &Config) {
config.generate_child_script_on_failure,
);
if config.verbose {
debug!("{}", output);
debug!("{output}");
}
}
}
Expand All @@ -295,15 +295,30 @@ pub(crate) fn install_tools(
metadata: &cargo_metadata::Metadata,
) {
let platform_tools_version = config.platform_tools_version.unwrap_or_else(|| {
let workspace_tools_version = metadata.workspace_metadata.get("solana").and_then(|v| v.get("tools-version")).and_then(|v| v.as_str());
let package_tools_version = package.map(|p| p.metadata.get("solana").and_then(|v| v.get("tools-version")).and_then(|v| v.as_str())).unwrap_or(None);
let workspace_tools_version = metadata
.workspace_metadata
.get("solana")
.and_then(|v| v.get("tools-version"))
.and_then(|v| v.as_str());
let package_tools_version = package
.map(|p| {
p.metadata
.get("solana")
.and_then(|v| v.get("tools-version"))
.and_then(|v| v.as_str())
})
.unwrap_or(None);
match (workspace_tools_version, package_tools_version) {
(Some(workspace_version), Some(package_version)) => {
if workspace_version != package_version {
warn!("Workspace and package specify conflicting tools versions, {workspace_version} and {package_version}, using package version {package_version}");
warn!(
"Workspace and package specify conflicting tools versions, \
{workspace_version} and {package_version}, using package version \
{package_version}"
);
}
package_version
},
}
(Some(workspace_version), None) => workspace_version,
(None, Some(package_version)) => package_version,
(None, None) => DEFAULT_PLATFORM_TOOLS_VERSION,
Expand Down Expand Up @@ -351,7 +366,7 @@ pub(crate) fn install_tools(
exit(1);
});
}
error!("Failed to install platform-tools: {}", err);
error!("Failed to install platform-tools: {err}");
exit(1);
});
}
Expand All @@ -367,7 +382,8 @@ pub(crate) fn install_tools(
// this by removing RUSTC from the child process environment.
if env::var("RUSTC").is_ok() {
warn!(
"Removed RUSTC from cargo environment, because it overrides +solana cargo command line option."
"Removed RUSTC from cargo environment, because it overrides +solana cargo command \
line option."
);
env::remove_var("RUSTC")
}
Expand All @@ -380,7 +396,10 @@ fn check_solana_target_installed(target: &str) {
let rustc = PathBuf::from(rustc);
let output = spawn(&rustc, ["--print", "target-list"], false);
if !output.contains(target) {
error!("Provided {:?} does not have {} target. The Solana rustc must be available in $PATH or the $RUSTC environment variable for the build to succeed.", rustc, target);
error!(
"Provided {rustc:?} does not have {target} target. The Solana rustc must be available \
in $PATH or the $RUSTC environment variable for the build to succeed."
);
exit(1);
}
}
9 changes: 3 additions & 6 deletions platform-tools-sdk/cargo-build-sbf/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ where
.iter()
.map(|arg| arg.as_ref().to_str().unwrap_or("?"))
.join(" ");
info!("spawn: {:?} {}", program, msg);
info!("spawn: {program:?} {msg}");

let child = Command::new(program)
.args(args)
Expand All @@ -49,12 +49,9 @@ where
writeln!(out, "{key}=\"{value}\" \\").unwrap();
}
write!(out, "{}", program.display()).unwrap();
writeln!(out, "{}", msg).unwrap();
writeln!(out, "{msg}").unwrap();
out.flush().unwrap();
error!(
"To rerun the failed command for debugging use {}",
script_name,
);
error!("To rerun the failed command for debugging use {script_name}");
exit(1);
}
output
Expand Down
Loading
Loading