Skip to content

Commit 939a114

Browse files
committed
Simplify checking feature syntax
1 parent ac8c6ab commit 939a114

File tree

1 file changed

+53
-95
lines changed

1 file changed

+53
-95
lines changed

src/cargo/core/summary.rs

Lines changed: 53 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,12 @@ fn build_feature_map(
156156
dependencies: &[Dependency],
157157
) -> CargoResult<FeatureMap> {
158158
use self::FeatureValue::*;
159-
let mut dep_map = HashMap::new();
159+
// A map of dependency names to whether there are any that are optional.
160+
let mut dep_map: HashMap<InternedString, bool> = HashMap::new();
160161
for dep in dependencies.iter() {
161-
dep_map
162-
.entry(dep.name_in_toml())
163-
.or_insert_with(Vec::new)
164-
.push(dep);
162+
*dep_map.entry(dep.name_in_toml()).or_insert(false) |= dep.is_optional();
165163
}
164+
let dep_map = dep_map; // We are done mutating this variable
166165

167166
let mut map: FeatureMap = features
168167
.iter()
@@ -180,117 +179,78 @@ fn build_feature_map(
180179
let explicitly_listed: HashSet<_> = map
181180
.values()
182181
.flatten()
183-
.filter_map(|fv| match fv {
184-
Dep { dep_name } => Some(*dep_name),
185-
_ => None,
186-
})
182+
.filter_map(|fv| fv.explicitly_name())
187183
.collect();
188184
for dep in dependencies {
189185
if !dep.is_optional() {
190186
continue;
191187
}
192-
let dep_name_in_toml = dep.name_in_toml();
193-
if features.contains_key(&dep_name_in_toml) || explicitly_listed.contains(&dep_name_in_toml)
194-
{
188+
let dep_name = dep.name_in_toml();
189+
if features.contains_key(&dep_name) || explicitly_listed.contains(&dep_name) {
195190
continue;
196191
}
197-
let fv = Dep {
198-
dep_name: dep_name_in_toml,
199-
};
200-
map.insert(dep_name_in_toml, vec![fv]);
192+
map.insert(dep_name, vec![Dep { dep_name }]);
201193
}
194+
let map = map; // We are done mutating this variable
202195

203196
// Validate features are listed properly.
204197
for (feature, fvs) in &map {
205198
FeatureName::new(feature)?;
206199
for fv in fvs {
207200
// Find data for the referenced dependency...
208-
let dep_data = {
209-
match fv {
210-
Feature(dep_name) | Dep { dep_name, .. } | DepFeature { dep_name, .. } => {
211-
dep_map.get(dep_name)
212-
}
213-
}
214-
};
215-
let is_optional_dep = dep_data
216-
.iter()
217-
.flat_map(|d| d.iter())
218-
.any(|d| d.is_optional());
201+
let dep_data = dep_map.get(&fv.dep_name());
219202
let is_any_dep = dep_data.is_some();
203+
let is_optional_dep = dep_data.is_some_and(|&o| o);
220204
match fv {
221205
Feature(f) => {
222206
if !features.contains_key(f) {
223207
if !is_any_dep {
224208
bail!(
225-
"feature `{}` includes `{}` which is neither a dependency \
226-
nor another feature",
227-
feature,
228-
fv
229-
);
209+
"feature `{feature}` includes `{fv}` which is neither a dependency \
210+
nor another feature"
211+
);
230212
}
231213
if is_optional_dep {
232214
if !map.contains_key(f) {
233215
bail!(
234-
"feature `{}` includes `{}`, but `{}` is an \
216+
"feature `{feature}` includes `{fv}`, but `{f}` is an \
235217
optional dependency without an implicit feature\n\
236-
Use `dep:{}` to enable the dependency.",
237-
feature,
238-
fv,
239-
f,
240-
f
218+
Use `dep:{f}` to enable the dependency."
241219
);
242220
}
243221
} else {
244-
bail!("feature `{}` includes `{}`, but `{}` is not an optional dependency\n\
222+
bail!("feature `{feature}` includes `{fv}`, but `{f}` is not an optional dependency\n\
245223
A non-optional dependency of the same name is defined; \
246-
consider adding `optional = true` to its definition.",
247-
feature, fv, f);
224+
consider adding `optional = true` to its definition.");
248225
}
249226
}
250227
}
251228
Dep { dep_name } => {
252229
if !is_any_dep {
253-
bail!(
254-
"feature `{}` includes `{}`, but `{}` is not listed as a dependency",
255-
feature,
256-
fv,
257-
dep_name
258-
);
230+
bail!("feature `{feature}` includes `{fv}`, but `{dep_name}` is not listed as a dependency");
259231
}
260232
if !is_optional_dep {
261233
bail!(
262-
"feature `{}` includes `{}`, but `{}` is not an optional dependency\n\
234+
"feature `{feature}` includes `{fv}`, but `{dep_name}` is not an optional dependency\n\
263235
A non-optional dependency of the same name is defined; \
264-
consider adding `optional = true` to its definition.",
265-
feature,
266-
fv,
267-
dep_name
236+
consider adding `optional = true` to its definition."
268237
);
269238
}
270239
}
271240
DepFeature {
272241
dep_name,
273242
dep_feature,
274243
weak,
275-
..
276244
} => {
277245
// Early check for some unlikely syntax.
278246
if dep_feature.contains('/') {
279-
bail!(
280-
"multiple slashes in feature `{}` (included by feature `{}`) are not allowed",
281-
fv,
282-
feature
283-
);
247+
bail!("multiple slashes in feature `{fv}` (included by feature `{feature}`) are not allowed");
284248
}
285249

286250
// dep: cannot be combined with /
287251
if let Some(stripped_dep) = dep_name.strip_prefix("dep:") {
288252
let has_other_dep = explicitly_listed.contains(stripped_dep);
289-
let is_optional = dep_map
290-
.get(stripped_dep)
291-
.iter()
292-
.flat_map(|d| d.iter())
293-
.any(|d| d.is_optional());
253+
let is_optional = dep_map.get(stripped_dep).is_some_and(|&o| o);
294254
let extra_help = if *weak || has_other_dep || !is_optional {
295255
// In this case, the user should just remove dep:.
296256
// Note that "hiding" an optional dependency
@@ -314,18 +274,14 @@ fn build_feature_map(
314274

315275
// Validation of the feature name will be performed in the resolver.
316276
if !is_any_dep {
317-
bail!(
318-
"feature `{}` includes `{}`, but `{}` is not a dependency",
319-
feature,
320-
fv,
321-
dep_name
322-
);
277+
bail!("feature `{feature}` includes `{fv}`, but `{dep_name}` is not a dependency");
323278
}
324279
if *weak && !is_optional_dep {
325-
bail!("feature `{}` includes `{}` with a `?`, but `{}` is not an optional dependency\n\
280+
bail!(
281+
"feature `{feature}` includes `{fv}` with a `?`, but `{dep_name}` is not an optional dependency\n\
326282
A non-optional dependency of the same name is defined; \
327-
consider removing the `?` or changing the dependency to be optional",
328-
feature, fv, dep_name);
283+
consider removing the `?` or changing the dependency to be optional"
284+
);
329285
}
330286
}
331287
}
@@ -341,15 +297,13 @@ fn build_feature_map(
341297
_ => None,
342298
})
343299
.collect();
344-
if let Some(dep) = dependencies
300+
if let Some((dep, _)) = dep_map
345301
.iter()
346-
.find(|dep| dep.is_optional() && !used.contains(&dep.name_in_toml()))
302+
.find(|&(dep, &is_optional)| is_optional && !used.contains(dep))
347303
{
348304
bail!(
349-
"optional dependency `{}` is not included in any feature\n\
350-
Make sure that `dep:{}` is included in one of features in the [features] table.",
351-
dep.name_in_toml(),
352-
dep.name_in_toml(),
305+
"optional dependency `{dep}` is not included in any feature\n\
306+
Make sure that `dep:{dep}` is included in one of features in the [features] table."
353307
);
354308
}
355309

@@ -376,19 +330,13 @@ pub enum FeatureValue {
376330

377331
impl FeatureValue {
378332
pub fn new(feature: InternedString) -> FeatureValue {
379-
match feature.find('/') {
380-
Some(pos) => {
381-
let (dep, dep_feat) = feature.split_at(pos);
382-
let dep_feat = &dep_feat[1..];
383-
let (dep, weak) = if let Some(dep) = dep.strip_suffix('?') {
384-
(dep, true)
385-
} else {
386-
(dep, false)
387-
};
333+
match feature.split_once('/') {
334+
Some((dep, dep_feat)) => {
335+
let dep_name = dep.strip_suffix('?');
388336
FeatureValue::DepFeature {
389-
dep_name: InternedString::new(dep),
337+
dep_name: InternedString::new(dep_name.unwrap_or(dep)),
390338
dep_feature: InternedString::new(dep_feat),
391-
weak,
339+
weak: dep_name.is_some(),
392340
}
393341
}
394342
None => {
@@ -403,25 +351,35 @@ impl FeatureValue {
403351
}
404352
}
405353

406-
/// Returns `true` if this feature explicitly used `dep:` syntax.
407-
pub fn has_dep_prefix(&self) -> bool {
408-
matches!(self, FeatureValue::Dep { .. })
354+
fn explicitly_name(&self) -> Option<InternedString> {
355+
match self {
356+
FeatureValue::Dep { dep_name, .. } => Some(*dep_name),
357+
_ => None,
358+
}
359+
}
360+
361+
fn dep_name(&self) -> InternedString {
362+
match self {
363+
FeatureValue::Feature(dep_name)
364+
| FeatureValue::Dep { dep_name, .. }
365+
| FeatureValue::DepFeature { dep_name, .. } => *dep_name,
366+
}
409367
}
410368
}
411369

412370
impl fmt::Display for FeatureValue {
413371
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
414372
use self::FeatureValue::*;
415373
match self {
416-
Feature(feat) => write!(f, "{}", feat),
417-
Dep { dep_name } => write!(f, "dep:{}", dep_name),
374+
Feature(feat) => write!(f, "{feat}"),
375+
Dep { dep_name } => write!(f, "dep:{dep_name}"),
418376
DepFeature {
419377
dep_name,
420378
dep_feature,
421379
weak,
422380
} => {
423381
let weak = if *weak { "?" } else { "" };
424-
write!(f, "{}{}/{}", dep_name, weak, dep_feature)
382+
write!(f, "{dep_name}{weak}/{dep_feature}")
425383
}
426384
}
427385
}

0 commit comments

Comments
 (0)