Skip to content

Commit

Permalink
fix: Remove implicit feature removal
Browse files Browse the repository at this point in the history
Due to problems we ran into with #14016, we're removing implicit
features from the 2024 edition to give ourselves more time to design it
as we should.

I could have added a new flag for this or made an EditionNext but I
decided to remove it in the hopes to avoid any path dependency in
solving this the next time.
  • Loading branch information
epage committed Oct 1, 2024
1 parent 2fd7321 commit 7be5a21
Show file tree
Hide file tree
Showing 11 changed files with 13 additions and 1,286 deletions.
6 changes: 1 addition & 5 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ use crate::sources::{PathSource, SourceConfigMap, CRATES_IO_INDEX, CRATES_IO_REG
use crate::util::edit_distance;
use crate::util::errors::{CargoResult, ManifestError};
use crate::util::interning::InternedString;
use crate::util::lints::{
analyze_cargo_lints_table, check_im_a_teapot, check_implicit_features, unused_dependencies,
};
use crate::util::lints::{analyze_cargo_lints_table, check_im_a_teapot};
use crate::util::toml::{read_manifest, InheritableFields};
use crate::util::{
context::CargoResolverConfig, context::ConfigRelativePath, context::IncompatibleRustVersions,
Expand Down Expand Up @@ -1240,8 +1238,6 @@ impl<'gctx> Workspace<'gctx> {
self.gctx,
)?;
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
check_implicit_features(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
unused_dependencies(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
if error_count > 0 {
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
"encountered {error_count} errors(s) while running lints"
Expand Down
77 changes: 1 addition & 76 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,12 @@ use crate::core::compiler::CompileKind;
use crate::core::compiler::RustcTargetData;
use crate::core::resolver::features::{DiffMap, FeatureOpts, FeatureResolver, FeaturesFor};
use crate::core::resolver::{HasDevUnits, Resolve, ResolveBehavior};
use crate::core::PackageIdSpecQuery as _;
use crate::core::{Edition, MaybePackage, Package, PackageId, Workspace};
use crate::core::{FeatureValue, PackageIdSpecQuery as _};
use crate::ops::resolve::WorkspaceResolve;
use crate::ops::{self, CompileOptions};
use crate::util::diagnostic_server::{Message, RustfixDiagnosticServer};
use crate::util::errors::CargoResult;
use crate::util::interning::InternedString;
use crate::util::GlobalContext;
use crate::util::{existing_vcs_repo, LockServer, LockServerClient};
use crate::{drop_eprint, drop_eprintln};
Expand Down Expand Up @@ -287,7 +286,6 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
fixes += rename_dep_fields_2024(workspace, "dependencies");
}

fixes += add_feature_for_unused_deps(pkg, root, ws.gctx());
fixes += rename_table(root, "project", "package");
if let Some(target) = root.get_mut("lib").and_then(|t| t.as_table_like_mut()) {
fixes += rename_target_fields_2024(target);
Expand Down Expand Up @@ -437,79 +435,6 @@ fn rename_table(parent: &mut dyn toml_edit::TableLike, old: &str, new: &str) ->
1
}

fn add_feature_for_unused_deps(
pkg: &Package,
parent: &mut dyn toml_edit::TableLike,
gctx: &GlobalContext,
) -> usize {
let manifest = pkg.manifest();

let activated_opt_deps = manifest
.normalized_toml()
.features()
.map(|map| {
map.values()
.flatten()
.filter_map(|f| match FeatureValue::new(InternedString::new(f)) {
FeatureValue::Dep { dep_name } => Some(dep_name.as_str()),
_ => None,
})
.collect::<HashSet<_>>()
})
.unwrap_or_default();

let mut fixes = 0;
for dep in manifest.dependencies() {
let dep_name_in_toml = dep.name_in_toml();
if dep.is_optional() && !activated_opt_deps.contains(dep_name_in_toml.as_str()) {
if let Some(features) = parent
.entry("features")
.or_insert(toml_edit::table())
.as_table_like_mut()
{
let activate_dep = format!("dep:{dep_name_in_toml}");
let strong_dep_feature_prefix = format!("{dep_name_in_toml}/");
features
.entry(dep_name_in_toml.as_str())
.or_insert_with(|| {
fixes += 1;
toml_edit::Item::Value(toml_edit::Value::Array(
toml_edit::Array::from_iter([&activate_dep]),
))
});
// Ensure `dep:dep_name` is present for `dep_name/feature_name` since `dep:` is the
// only way to guarantee an optional dependency is available for use.
//
// The way we avoid implicitly creating features in Edition2024 is we remove the
// dependency from `normalized_toml` if there is no `dep:` syntax as that is the only
// syntax that suppresses the creation of the implicit feature.
for (feature_name, activations) in features.iter_mut() {
let Some(activations) = activations.as_array_mut() else {
let _ = gctx.shell().warn(format_args!("skipping fix of feature `{feature_name}` in package `{}`: unsupported feature schema", pkg.name()));
continue;
};
if activations
.iter()
.any(|a| a.as_str().map(|a| a == activate_dep).unwrap_or(false))
{
continue;
}
let Some(activate_dep_pos) = activations.iter().position(|a| {
a.as_str()
.map(|a| a.starts_with(&strong_dep_feature_prefix))
.unwrap_or(false)
}) else {
continue;
};
fixes += 1;
activations.insert(activate_dep_pos, &activate_dep);
}
}
}
}
fixes
}

fn check_resolver_change<'gctx>(
ws: &Workspace<'gctx>,
target_data: &mut RustcTargetData<'gctx>,
Expand Down
Loading

0 comments on commit 7be5a21

Please sign in to comment.