Skip to content

Commit

Permalink
Implement feature_missing manifest lint to prove manifest lints work!
Browse files Browse the repository at this point in the history
  • Loading branch information
obi1kenobi committed Nov 30, 2024
1 parent 3d2766d commit 1c76d69
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 9 deletions.
53 changes: 53 additions & 0 deletions src/lints/feature_missing.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
SemverQuery(
id: "feature_missing",
human_readable_name: "package feature removed or renamed",
description: "A feature has been removed from this package's Cargo.toml.",
required_update: Major,
lint_level: Deny,
reference_link: Some("https://doc.rust-lang.org/cargo/reference/semver.html#cargo-feature-remove"),
query: r#"
{
CrateDiff {
baseline {
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 {
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 has been removed from this package's Cargo.toml. This will break downstream crates that enable 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,
)
36 changes: 27 additions & 9 deletions src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ mod tests {
use std::borrow::Cow;
use std::collections::BTreeSet;
use std::path::PathBuf;
use std::sync::OnceLock;
use std::sync::{Arc, OnceLock};
use std::{collections::BTreeMap, path::Path};

use anyhow::Context;
Expand Down Expand Up @@ -631,14 +631,31 @@ mod tests {
// Reorder vector of results into a deterministic order that will compensate for
// nondeterminism in how the results are ordered.
let key_func = |elem: &BTreeMap<String, FieldValue>| {
let filename = elem.get("span_filename").and_then(|value| value.as_str());
let line = elem.get("span_begin_line");

match (filename, line) {
(Some(filename), Some(line)) => (filename.to_owned(), line.as_usize()),
(Some(_filename), None) => panic!("A valid query must output `span_filename`. See https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md for details."),
(None, Some(_line)) => panic!("A valid query must output `span_begin_line`. See https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md for details."),
(None, None) => panic!("A valid query must output both `span_filename` and `span_begin_line`. See https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md for details."),
// Queries should either:
// - define an explicit `ordering_key` string value sufficient to establish
// a total order of results for each crate, or
// - define `span_filename` and `span_begin_line` values where the lint is being raised,
// which will then define a total order of results for that query on that crate.
let ordering_key = elem
.get("ordering_key")
.and_then(|value| value.as_arc_str());
if let Some(key) = ordering_key {
(Arc::clone(key), 0)
} else {
let filename = elem.get("span_filename").map(|value| {
value
.as_arc_str()
.expect("`span_filename` was not a string")
});
let line = elem
.get("span_begin_line")
.map(|value: &FieldValue| value.as_usize().expect("begin line was not an int"));
match (filename, line) {
(Some(filename), Some(line)) => (Arc::clone(filename), line),
(Some(_filename), None) => panic!("A valid query must output `span_filename`. See https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md for details."),
(None, Some(_line)) => panic!("A valid query must output `span_begin_line`. See https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md for details."),
(None, None) => panic!("A valid query must output both `span_filename` and `span_begin_line`. See https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md for details."),
}
}
};
for value in query_execution_results.values_mut() {
Expand Down Expand Up @@ -1037,6 +1054,7 @@ add_lints!(
enum_variant_marked_non_exhaustive,
enum_variant_missing,
exported_function_changed_abi,
feature_missing,
function_abi_no_longer_unwind,
function_changed_abi,
function_const_removed,
Expand Down
13 changes: 13 additions & 0 deletions test_crates/feature_missing/new/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[package]
publish = false
name = "feature_missing"
version = "0.1.0"
edition = "2021"

[dependencies]

[features]
still_present = []
# Explicitly-added feature, to replace the implicit feature defined by
# the `optional = true` dependency in the previous crate version.
rand_core = []
Empty file.
31 changes: 31 additions & 0 deletions test_crates/feature_missing/old/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
[package]
publish = false
name = "feature_missing"
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 is a breaking change.
rand = { version = "*", optional = true }
# However, re-adding an explicit feature after removing the implicit one
# will avoid the breakage.
rand_core = { version = "*", optional = true}

[features]
still_present = []
going_missing = []

# We ignore unstable-looking feature names.
# All of the following will be removed, and none of them should be flagged.
unstable = []
nightly = []
bench = []
unstable-dash = []
unstable_underscore = []
nightly-dash = []
nightly_underscore = []
bench-dash = []
bench_underscore = []
_underscore_prefix = []
Empty file.
13 changes: 13 additions & 0 deletions test_crates/function_feature_changed/new/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@ name = "function_feature_changed"
version = "0.1.0"
edition = "2021"

# The purpose of this test crate is to ensure that cargo-semver-checks' feature selection
# is robust to changes in which features exist and are enabled.
#
# We want cargo-semver-checks runs to succeed and fail on the basis of
# what they find inside the crate, not just always fail because a feature has been deleted.
# Hence, we disable this lint.
#
# This *only* affects directly running cargo-semver-checks against this crate.
# It does not affect the lint query runs against all crates, since those run over
# pre-generated rustdoc JSON and `cargo metadata` output and ignore this configuration.
[package.metadata.cargo-semver-checks.lints]
feature_missing = "allow"

[dependencies]

[features]
Expand Down
22 changes: 22 additions & 0 deletions test_outputs/query_execution/feature_missing.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: src/query.rs
expression: "&query_execution_results"
---
{
"./test_crates/feature_missing/": [
{
"name": String("going_missing"),
"ordering_key": String("going_missing"),
},
{
"name": String("rand"),
"ordering_key": String("rand"),
},
],
"./test_crates/function_feature_changed/": [
{
"name": String("feature_to_be_removed"),
"ordering_key": String("feature_to_be_removed"),
},
],
}

0 comments on commit 1c76d69

Please sign in to comment.