From 9f7ff6fbfbf56c470581849c43631be1cc516d8b Mon Sep 17 00:00:00 2001 From: Predrag Gruevski Date: Tue, 17 Dec 2024 00:34:39 +0000 Subject: [PATCH] Add `feature_not_enabled_by_default` lint. It'll catch features that used to be enabled by default, and then stop being defaults. Adds the lint requested by https://github.com/obi1kenobi/cargo-semver-checks-action/issues/91 --- src/lints/feature_not_enabled_by_default.ron | 60 +++++++++++++++++++ src/query.rs | 1 + .../new/Cargo.toml | 45 ++++++++++++++ .../new/src/lib.rs | 0 .../old/Cargo.toml | 46 ++++++++++++++ .../old/src/lib.rs | 0 test_crates/features_simple/new/Cargo.toml | 5 ++ .../query_execution/feature_missing.snap | 7 +++ .../feature_not_enabled_by_default.snap | 27 +++++++++ 9 files changed, 191 insertions(+) create mode 100644 src/lints/feature_not_enabled_by_default.ron create mode 100644 test_crates/feature_not_enabled_by_default/new/Cargo.toml create mode 100644 test_crates/feature_not_enabled_by_default/new/src/lib.rs create mode 100644 test_crates/feature_not_enabled_by_default/old/Cargo.toml create mode 100644 test_crates/feature_not_enabled_by_default/old/src/lib.rs create mode 100644 test_outputs/query_execution/feature_not_enabled_by_default.snap diff --git a/src/lints/feature_not_enabled_by_default.ron b/src/lints/feature_not_enabled_by_default.ron new file mode 100644 index 00000000..020a5cc9 --- /dev/null +++ b/src/lints/feature_not_enabled_by_default.ron @@ -0,0 +1,60 @@ +SemverQuery( + id: "feature_not_enabled_by_default", + human_readable_name: "package feature is not enabled by default", + description: "A feature has been removed from this package's set of default features.", + required_update: Major, + lint_level: Deny, + reference_link: Some("https://doc.rust-lang.org/cargo/reference/semver.html#cargo-feature-remove-another"), + query: r#" + { + CrateDiff { + baseline { + default_feature { + # Until cargo ships with support for private and/or unstable feature names, + # we'll rely on feature names to detect whether to flag feature removals. + # + # This lint will ignore features that match any of the following: + # - start with an underscore (`_`) character + # - are named `unstable`, `nightly`, or `bench` + # - have a prefix of `unstable`, `nightly`, or `bench` followed by + # a dash (`-`) or underscore (`_`) character. + # + # Cargo tracking issues: + # - unstable/nightly features: https://github.com/rust-lang/cargo/issues/10881 + # - private/hidden features: https://github.com/rust-lang/cargo/issues/10882 + name @tag + @filter(op: "not_regex", value: ["$unstable_feature_pattern"]) + @filter(op: "not_has_prefix", value: ["$underscore"]) + @output + + # An explicit ordering key is needed since we don't have span information, + # which what we usually use to order results in tests. + name @output(name: "ordering_key") + } + } + current { + default_feature @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) { + name @filter(op: "=", value: ["%name"]) + } + + # Ensure the feature still exists, just isn't enabled by default. + # If the feature is missing entirely, we want to report it + # in the `feature_missing` lint, not here. + feature @fold @transform(op: "count") @filter(op: ">", value: ["$zero"]) { + name @filter(op: "=", value: ["%name"]) + } + } + } + }"#, + arguments: { + "zero": 0, + "unstable_feature_pattern": "^(?:unstable|nightly|bench)(?:[-_].*)?$", + "underscore": "_", + }, + error_message: "A feature is no longer enabled by default for this package. This will break downstream crates which rely on the package's default features and require the functionality of this feature.", + per_result_error_template: Some("feature {{name}} in the package's Cargo.toml"), + // TODO: It's currently not possible to write witnesses for manifest lints, + // since we'd need to generate a *Cargo.toml* witness instead of a Rust code witness. + // Issue: https://github.com/obi1kenobi/cargo-semver-checks/issues/1008 + witness: None, +) diff --git a/src/query.rs b/src/query.rs index 334be58d..89ffb04d 100644 --- a/src/query.rs +++ b/src/query.rs @@ -1150,6 +1150,7 @@ add_lints!( enum_variant_missing, exported_function_changed_abi, feature_missing, + feature_not_enabled_by_default, function_abi_no_longer_unwind, function_changed_abi, function_const_removed, diff --git a/test_crates/feature_not_enabled_by_default/new/Cargo.toml b/test_crates/feature_not_enabled_by_default/new/Cargo.toml new file mode 100644 index 00000000..82149a0c --- /dev/null +++ b/test_crates/feature_not_enabled_by_default/new/Cargo.toml @@ -0,0 +1,45 @@ +[package] +publish = false +name = "feature_not_enabled_by_default" +version = "0.1.0" +edition = "2021" + +[dependencies] +# Since `rand` isn't used in a feature with `dep:rand` syntax, +# it defines an implicit feature by that name. +# Removing that implicit feature from the defaults is a breaking change. +rand = { version = "*", optional = true } + +[features] +default = ["still_present", "transitive", "multi_root_transitive"] +still_present = [] +becoming_non_default = [] + +# The `transitive` feature used to depend on `indirect_feature`. +# `transitive` is enabled by default, but removing `indirect_feature` from `transitive` +# means that `indirect_feature` is *no longer enabled by default*. +# This is breaking and should be reported. +transitive = [] +indirect_feature = [] + +# The `root` feature is depended on in multiple ways in the default features. +# Breaking some of those paths, but not all of them, still means that `root` is enabled by default. +# There's no breaking change here, and no lints should trigger. +multi_root_transitive = ["path_a", "path_b"] +path_a = ["root"] +path_b = [] +root = [] + +# We ignore unstable-looking feature names. +# All of the following will be removed from the default features, +# and none of them should be flagged. +unstable = [] +nightly = [] +bench = [] +unstable-dash = [] +unstable_underscore = [] +nightly-dash = [] +nightly_underscore = [] +bench-dash = [] +bench_underscore = [] +_underscore_prefix = [] diff --git a/test_crates/feature_not_enabled_by_default/new/src/lib.rs b/test_crates/feature_not_enabled_by_default/new/src/lib.rs new file mode 100644 index 00000000..e69de29b diff --git a/test_crates/feature_not_enabled_by_default/old/Cargo.toml b/test_crates/feature_not_enabled_by_default/old/Cargo.toml new file mode 100644 index 00000000..cef72587 --- /dev/null +++ b/test_crates/feature_not_enabled_by_default/old/Cargo.toml @@ -0,0 +1,46 @@ +[package] +publish = false +name = "feature_not_enabled_by_default" +version = "0.1.0" +edition = "2021" + +[dependencies] +# Since `rand` isn't used in a feature with `dep:rand` syntax, +# it defines an implicit feature by that name. +# Removing that implicit feature from the defaults is a breaking change. +rand = { version = "*", optional = true } + +[features] +default = ["becoming_non_default", "still_present", "missing_entirely", "rand", "transitive", "multi_root_transitive", "unstable", "nightly", "bench", "unstable", "unstable_underscore", "nightly", "nightly_underscore", "bench", "bench_underscore", "_underscore_prefix"] +still_present = [] # No breakage here. +becoming_non_default = [] # Should be reported. +missing_entirely = [] # Flagged only by `feature_missing`, even though it was default too. + +# The `transitive` feature depends on `indirect_feature`. +# `transitive` is enabled by default, but removing `indirect_feature` from `transitive` +# means that `indirect_feature` is *no longer enabled by default*. +# This is breaking and should be reported. +transitive = ["indirect_feature"] +indirect_feature = [] + +# The `root` feature is depended on in multiple ways in the default features. +# Breaking some of those paths, but not all of them, still means that `root` is enabled by default. +# There's no breaking change here, and no lints should trigger. +multi_root_transitive = ["path_a", "path_b"] +path_a = ["root"] +path_b = ["root"] +root = [] + +# We ignore unstable-looking feature names. +# All of the following will be removed from the default features, +# and none of them should be flagged. +unstable = [] +nightly = [] +bench = [] +unstable-dash = [] +unstable_underscore = [] +nightly-dash = [] +nightly_underscore = [] +bench-dash = [] +bench_underscore = [] +_underscore_prefix = [] diff --git a/test_crates/feature_not_enabled_by_default/old/src/lib.rs b/test_crates/feature_not_enabled_by_default/old/src/lib.rs new file mode 100644 index 00000000..e69de29b diff --git a/test_crates/features_simple/new/Cargo.toml b/test_crates/features_simple/new/Cargo.toml index 9304adc5..fbb5046a 100644 --- a/test_crates/features_simple/new/Cargo.toml +++ b/test_crates/features_simple/new/Cargo.toml @@ -4,6 +4,11 @@ name = "features_simple" version = "0.1.0" edition = "2021" +[package.metadata.cargo-semver-checks.lints] +# This package is for testing that feature selection in the CLI works correctly, +# so we aren't looking for breaking changes in the features themselves. +feature_not_enabled_by_default = "allow" + [dependencies] [features] diff --git a/test_outputs/query_execution/feature_missing.snap b/test_outputs/query_execution/feature_missing.snap index 80dbcb6e..b5679fa7 100644 --- a/test_outputs/query_execution/feature_missing.snap +++ b/test_outputs/query_execution/feature_missing.snap @@ -1,6 +1,7 @@ --- source: src/query.rs expression: "&query_execution_results" +snapshot_kind: text --- { "./test_crates/feature_missing/": [ @@ -17,6 +18,12 @@ expression: "&query_execution_results" "ordering_key": String("rand_pcg"), }, ], + "./test_crates/feature_not_enabled_by_default/": [ + { + "name": String("missing_entirely"), + "ordering_key": String("missing_entirely"), + }, + ], "./test_crates/function_feature_changed/": [ { "name": String("feature_to_be_removed"), diff --git a/test_outputs/query_execution/feature_not_enabled_by_default.snap b/test_outputs/query_execution/feature_not_enabled_by_default.snap new file mode 100644 index 00000000..b295e753 --- /dev/null +++ b/test_outputs/query_execution/feature_not_enabled_by_default.snap @@ -0,0 +1,27 @@ +--- +source: src/query.rs +expression: "&query_execution_results" +snapshot_kind: text +--- +{ + "./test_crates/feature_not_enabled_by_default/": [ + { + "name": String("becoming_non_default"), + "ordering_key": String("becoming_non_default"), + }, + { + "name": String("indirect_feature"), + "ordering_key": String("indirect_feature"), + }, + { + "name": String("rand"), + "ordering_key": String("rand"), + }, + ], + "./test_crates/features_simple/": [ + { + "name": String("foo"), + "ordering_key": String("foo"), + }, + ], +}