Skip to content

Commit

Permalink
Simplify checking feature syntax
Browse files Browse the repository at this point in the history
  • Loading branch information
@Eh2406
Eh2406 committed Jun 19, 2024
1 parent ac8c6ab commit f0ab501
Showing 1 changed file with 52 additions and 89 deletions.
141 changes: 52 additions & 89 deletions src/cargo/core/summary.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,15 @@ fn build_feature_map(
dependencies: &[Dependency],
) -> CargoResult<FeatureMap> {
use self::FeatureValue::*;
let mut dep_map = HashMap::new();
// A map of dependency names to whether there are any that are optional.
let mut dep_map: HashMap<InternedString, bool> = HashMap::new();
for dep in dependencies.iter() {
dep_map
.entry(dep.name_in_toml())
.or_insert_with(Vec::new)
.push(dep);
let has_optional = dep_map.entry(dep.name_in_toml()).or_insert(false);
if dep.is_optional() {
*has_optional = true;
}
}
let dep_map = dep_map; // We are done mutating this variable

let mut map: FeatureMap = features
.iter()
Expand All @@ -180,117 +182,78 @@ fn build_feature_map(
let explicitly_listed: HashSet<_> = map
.values()
.flatten()
.filter_map(|fv| match fv {
Dep { dep_name } => Some(*dep_name),
_ => None,
})
.filter_map(|fv| fv.explicitly_name())
.collect();
for dep in dependencies {
if !dep.is_optional() {
continue;
}
let dep_name_in_toml = dep.name_in_toml();
if features.contains_key(&dep_name_in_toml) || explicitly_listed.contains(&dep_name_in_toml)
{
let dep_name = dep.name_in_toml();
if features.contains_key(&dep_name) || explicitly_listed.contains(&dep_name) {
continue;
}
let fv = Dep {
dep_name: dep_name_in_toml,
};
map.insert(dep_name_in_toml, vec![fv]);
map.insert(dep_name, vec![Dep { dep_name }]);
}
let map = map; // We are done mutating this variable

// Validate features are listed properly.
for (feature, fvs) in &map {
FeatureName::new(feature)?;
for fv in fvs {
// Find data for the referenced dependency...
let dep_data = {
match fv {
Feature(dep_name) | Dep { dep_name, .. } | DepFeature { dep_name, .. } => {
dep_map.get(dep_name)
}
}
};
let is_optional_dep = dep_data
.iter()
.flat_map(|d| d.iter())
.any(|d| d.is_optional());
let dep_data = dep_map.get(&fv.dep_name());
let is_any_dep = dep_data.is_some();
let is_optional_dep = dep_data.is_some_and(|&o| o);
match fv {
Feature(f) => {
if !features.contains_key(f) {
if !is_any_dep {
bail!(
"feature `{}` includes `{}` which is neither a dependency \
nor another feature",
feature,
fv
);
"feature `{feature}` includes `{fv}` which is neither a dependency \
nor another feature"
);
}
if is_optional_dep {
if !map.contains_key(f) {
bail!(
"feature `{}` includes `{}`, but `{}` is an \
"feature `{feature}` includes `{fv}`, but `{f}` is an \
optional dependency without an implicit feature\n\
Use `dep:{}` to enable the dependency.",
feature,
fv,
f,
f
Use `dep:{f}` to enable the dependency."
);
}
} else {
bail!("feature `{}` includes `{}`, but `{}` is not an optional dependency\n\
bail!("feature `{feature}` includes `{fv}`, but `{f}` is not an optional dependency\n\
A non-optional dependency of the same name is defined; \
consider adding `optional = true` to its definition.",
feature, fv, f);
consider adding `optional = true` to its definition.");
}
}
}
Dep { dep_name } => {
if !is_any_dep {
bail!(
"feature `{}` includes `{}`, but `{}` is not listed as a dependency",
feature,
fv,
dep_name
);
bail!("feature `{feature}` includes `{fv}`, but `{dep_name}` is not listed as a dependency");
}
if !is_optional_dep {
bail!(
"feature `{}` includes `{}`, but `{}` is not an optional dependency\n\
"feature `{feature}` includes `{fv}`, but `{dep_name}` is not an optional dependency\n\
A non-optional dependency of the same name is defined; \
consider adding `optional = true` to its definition.",
feature,
fv,
dep_name
consider adding `optional = true` to its definition."
);
}
}
DepFeature {
dep_name,
dep_feature,
weak,
..
} => {
// Early check for some unlikely syntax.
if dep_feature.contains('/') {
bail!(
"multiple slashes in feature `{}` (included by feature `{}`) are not allowed",
fv,
feature
);
bail!("multiple slashes in feature `{fv}` (included by feature `{feature}`) are not allowed");
}

// dep: cannot be combined with /
if let Some(stripped_dep) = dep_name.strip_prefix("dep:") {
let has_other_dep = explicitly_listed.contains(stripped_dep);
let is_optional = dep_map
.get(stripped_dep)
.iter()
.flat_map(|d| d.iter())
.any(|d| d.is_optional());
let is_optional = dep_map.get(stripped_dep).is_some_and(|&o| o);
let extra_help = if *weak || has_other_dep || !is_optional {
// In this case, the user should just remove dep:.
// Note that "hiding" an optional dependency
Expand All @@ -314,18 +277,14 @@ fn build_feature_map(

// Validation of the feature name will be performed in the resolver.
if !is_any_dep {
bail!(
"feature `{}` includes `{}`, but `{}` is not a dependency",
feature,
fv,
dep_name
);
bail!("feature `{feature}` includes `{fv}`, but `{dep_name}` is not a dependency");
}
if *weak && !is_optional_dep {
bail!("feature `{}` includes `{}` with a `?`, but `{}` is not an optional dependency\n\
bail!(
"feature `{feature}` includes `{fv}` with a `?`, but `{dep_name}` is not an optional dependency\n\
A non-optional dependency of the same name is defined; \
consider removing the `?` or changing the dependency to be optional",
feature, fv, dep_name);
consider removing the `?` or changing the dependency to be optional"
);
}
}
}
Expand Down Expand Up @@ -376,19 +335,13 @@ pub enum FeatureValue {

impl FeatureValue {
pub fn new(feature: InternedString) -> FeatureValue {
match feature.find('/') {
Some(pos) => {
let (dep, dep_feat) = feature.split_at(pos);
let dep_feat = &dep_feat[1..];
let (dep, weak) = if let Some(dep) = dep.strip_suffix('?') {
(dep, true)
} else {
(dep, false)
};
match feature.split_once('/') {
Some((dep, dep_feat)) => {
let dep_name = dep.strip_suffix('?');
FeatureValue::DepFeature {
dep_name: InternedString::new(dep),
dep_name: InternedString::new(dep_name.unwrap_or(dep)),
dep_feature: InternedString::new(dep_feat),
weak,
weak: dep_name.is_some(),
}
}
None => {
Expand All @@ -403,25 +356,35 @@ impl FeatureValue {
}
}

/// Returns `true` if this feature explicitly used `dep:` syntax.
pub fn has_dep_prefix(&self) -> bool {
matches!(self, FeatureValue::Dep { .. })
fn explicitly_name(&self) -> Option<InternedString> {
match self {
FeatureValue::Dep { dep_name, .. } => Some(*dep_name),
_ => None,
}
}

fn dep_name(&self) -> InternedString {
match self {
FeatureValue::Feature(dep_name)
| FeatureValue::Dep { dep_name, .. }
| FeatureValue::DepFeature { dep_name, .. } => *dep_name,
}
}
}

impl fmt::Display for FeatureValue {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use self::FeatureValue::*;
match self {
Feature(feat) => write!(f, "{}", feat),
Dep { dep_name } => write!(f, "dep:{}", dep_name),
Feature(feat) => write!(f, "{feat}"),
Dep { dep_name } => write!(f, "dep:{dep_name}"),
DepFeature {
dep_name,
dep_feature,
weak,
} => {
let weak = if *weak { "?" } else { "" };
write!(f, "{}{}/{}", dep_name, weak, dep_feature)
write!(f, "{dep_name}{weak}/{dep_feature}")
}
}
}
Expand Down

0 comments on commit f0ab501

Please sign in to comment.