Skip to content
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

fix(toml): Validate crates_types/proc-macro for bin like others #13841

Merged
merged 8 commits into from
May 2, 2024
4 changes: 2 additions & 2 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ fn resolve_toml(
edition,
original_package.autobins,
warnings,
errors,
resolved_toml.lib.is_some(),
)?);
resolved_toml.example = Some(targets::resolve_examples(
Expand Down Expand Up @@ -1070,7 +1071,7 @@ fn to_real_manifest(
manifest_file: &Path,
gctx: &GlobalContext,
warnings: &mut Vec<String>,
errors: &mut Vec<String>,
_errors: &mut Vec<String>,
) -> CargoResult<Manifest> {
let embedded = is_embedded(manifest_file);
let package_root = manifest_file.parent().unwrap();
Expand Down Expand Up @@ -1212,7 +1213,6 @@ fn to_real_manifest(
edition,
&resolved_package.metabuild,
warnings,
errors,
)?;

if targets.iter().all(|t| t.is_custom_build()) {
Expand Down
178 changes: 101 additions & 77 deletions src/cargo/util/toml/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ pub(super) fn to_targets(
edition: Edition,
metabuild: &Option<StringOrVec>,
warnings: &mut Vec<String>,
errors: &mut Vec<String>,
) -> CargoResult<Vec<Target>> {
let mut targets = Vec::new();

Expand All @@ -64,7 +63,6 @@ pub(super) fn to_targets(
resolved_toml.bin.as_deref().unwrap_or_default(),
package_root,
edition,
errors,
)?);

targets.extend(to_example_targets(
Expand Down Expand Up @@ -143,10 +141,10 @@ pub fn resolve_lib(
let Some(mut lib) = lib else { return Ok(None) };
lib.name
.get_or_insert_with(|| package_name.replace("-", "_"));

// Check early to improve error messages
validate_lib_name(&lib, warnings)?;

// Checking the original lib
validate_proc_macro(&lib, "library", edition, warnings)?;
validate_crate_types(&lib, "library", edition, warnings)?;

Expand Down Expand Up @@ -260,6 +258,7 @@ pub fn resolve_bins(
edition: Edition,
autodiscover: Option<bool>,
warnings: &mut Vec<String>,
errors: &mut Vec<String>,
has_lib: bool,
) -> CargoResult<Vec<TomlBinTarget>> {
let inferred = inferred_bins(package_root, package_name);
Expand All @@ -277,8 +276,12 @@ pub fn resolve_bins(
);

for bin in &mut bins {
// Check early to improve error messages
validate_bin_name(bin, warnings)?;

validate_bin_crate_types(bin, edition, warnings, errors)?;
validate_bin_proc_macro(bin, edition, warnings, errors)?;

let path = target_path(bin, &inferred, "bin", package_root, edition, &mut |_| {
if let Some(legacy_path) = legacy_bin_path(package_root, name_or_panic(bin), has_lib) {
warnings.push(format!(
Expand Down Expand Up @@ -308,7 +311,6 @@ fn to_bin_targets(
bins: &[TomlBinTarget],
package_root: &Path,
edition: Edition,
errors: &mut Vec<String>,
) -> CargoResult<Vec<Target>> {
// This loop performs basic checks on each of the TomlTarget in `bins`.
for bin in bins {
Expand All @@ -317,27 +319,6 @@ fn to_bin_targets(
if bin.filename.is_some() {
features.require(Feature::different_binary_name())?;
}

if let Some(crate_types) = bin.crate_types() {
if !crate_types.is_empty() {
let name = name_or_panic(bin);
errors.push(format!(
"the target `{}` is a binary and can't have any \
crate-types set (currently \"{}\")",
name,
crate_types.join(", ")
));
}
}

if bin.proc_macro() == Some(true) {
let name = name_or_panic(bin);
errors.push(format!(
"the target `{}` is a binary and can't have `proc-macro` \
set `true`",
name
));
}
}

validate_unique_names(&bins, "binary")?;
Expand Down Expand Up @@ -608,7 +589,9 @@ fn resolve_targets_with_legacy_path(
);

for target in &toml_targets {
// Check early to improve error messages
validate_target_name(target, target_kind_human, target_kind, warnings)?;

validate_proc_macro(target, target_kind_human, edition, warnings)?;
validate_crate_types(target, target_kind_human, edition, warnings)?;
}
Expand Down Expand Up @@ -812,58 +795,6 @@ fn inferred_to_toml_targets(inferred: &[(String, PathBuf)]) -> Vec<TomlTarget> {
.collect()
}

fn validate_lib_name(target: &TomlTarget, warnings: &mut Vec<String>) -> CargoResult<()> {
validate_target_name(target, "library", "lib", warnings)?;
let name = name_or_panic(target);
if name.contains('-') {
anyhow::bail!("library target names cannot contain hyphens: {}", name)
}

Ok(())
}

fn validate_bin_name(bin: &TomlTarget, warnings: &mut Vec<String>) -> CargoResult<()> {
validate_target_name(bin, "binary", "bin", warnings)?;
let name = name_or_panic(bin).to_owned();
if restricted_names::is_conflicting_artifact_name(&name) {
anyhow::bail!(
"the binary target name `{name}` is forbidden, \
it conflicts with cargo's build directory names",
)
}

Ok(())
}

fn validate_target_name(
target: &TomlTarget,
target_kind_human: &str,
target_kind: &str,
warnings: &mut Vec<String>,
) -> CargoResult<()> {
match target.name {
Some(ref name) => {
if name.trim().is_empty() {
anyhow::bail!("{} target names cannot be empty", target_kind_human)
}
if cfg!(windows) && restricted_names::is_windows_reserved(name) {
warnings.push(format!(
"{} target `{}` is a reserved Windows filename, \
this target will not work on Windows platforms",
target_kind_human, name
));
}
}
None => anyhow::bail!(
"{} target {}.name is required",
target_kind_human,
target_kind
),
}

Ok(())
}

/// Will check a list of toml targets, and make sure the target names are unique within a vector.
fn validate_unique_names(targets: &[TomlTarget], target_kind: &str) -> CargoResult<()> {
let mut seen = HashSet::new();
Expand Down Expand Up @@ -1076,6 +1007,77 @@ fn name_or_panic(target: &TomlTarget) -> &str {
.unwrap_or_else(|| panic!("target name is required"))
}

fn validate_lib_name(target: &TomlTarget, warnings: &mut Vec<String>) -> CargoResult<()> {
validate_target_name(target, "library", "lib", warnings)?;
let name = name_or_panic(target);
if name.contains('-') {
anyhow::bail!("library target names cannot contain hyphens: {}", name)
}

Ok(())
}

fn validate_bin_name(bin: &TomlTarget, warnings: &mut Vec<String>) -> CargoResult<()> {
validate_target_name(bin, "binary", "bin", warnings)?;
let name = name_or_panic(bin).to_owned();
if restricted_names::is_conflicting_artifact_name(&name) {
anyhow::bail!(
"the binary target name `{name}` is forbidden, \
it conflicts with cargo's build directory names",
)
}

Ok(())
}

fn validate_target_name(
target: &TomlTarget,
target_kind_human: &str,
target_kind: &str,
warnings: &mut Vec<String>,
) -> CargoResult<()> {
match target.name {
Some(ref name) => {
if name.trim().is_empty() {
anyhow::bail!("{} target names cannot be empty", target_kind_human)
}
if cfg!(windows) && restricted_names::is_windows_reserved(name) {
warnings.push(format!(
"{} target `{}` is a reserved Windows filename, \
this target will not work on Windows platforms",
target_kind_human, name
));
}
}
None => anyhow::bail!(
"{} target {}.name is required",
target_kind_human,
target_kind
),
}

Ok(())
}

fn validate_bin_proc_macro(
target: &TomlTarget,
edition: Edition,
warnings: &mut Vec<String>,
errors: &mut Vec<String>,
) -> CargoResult<()> {
if target.proc_macro() == Some(true) {
let name = name_or_panic(target);
errors.push(format!(
"the target `{}` is a binary and can't have `proc-macro` \
set `true`",
name
));
} else {
validate_proc_macro(target, "binary", edition, warnings)?;
}
Ok(())
}

fn validate_proc_macro(
target: &TomlTarget,
kind: &str,
Expand All @@ -1093,6 +1095,28 @@ fn validate_proc_macro(
)
}

fn validate_bin_crate_types(
target: &TomlTarget,
edition: Edition,
warnings: &mut Vec<String>,
errors: &mut Vec<String>,
) -> CargoResult<()> {
if let Some(crate_types) = target.crate_types() {
if !crate_types.is_empty() {
let name = name_or_panic(target);
errors.push(format!(
"the target `{}` is a binary and can't have any \
crate-types set (currently \"{}\")",
name,
crate_types.join(", ")
));
} else {
validate_crate_types(target, "binary", edition, warnings)?;
}
}
Ok(())
}

fn validate_crate_types(
target: &TomlTarget,
kind: &str,
Expand Down
Loading