Skip to content

Commit

Permalink
Add trait_method_parameter_count_changed lint.
Browse files Browse the repository at this point in the history
It catches cases where trait methods change arity, i.e. start requiring a different number of arguments than they used to. This breaks calling and/or implementing those methods.
  • Loading branch information
obi1kenobi committed Dec 21, 2024
1 parent 127ba43 commit 7eeec61
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 0 deletions.
77 changes: 77 additions & 0 deletions src/lints/trait_method_parameter_count_changed.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
SemverQuery(
id: "trait_method_parameter_count_changed",
human_readable_name: "pub trait method parameter count changed",
description: "A trait method requires a different number of parameters than it used to.",
required_update: Major,
lint_level: Deny,
reference_link: Some("https://doc.rust-lang.org/cargo/reference/semver.html#major-any-change-to-trait-item-signatures"),
query: r#"
{
CrateDiff {
baseline {
item {
... on Trait {
visibility_limit @filter(op: "=", value: ["$public"]) @output
name @output
importable_path {
path @output @tag
public_api @filter(op: "=", value: ["$true"])
}
method {
method_name: name @output @tag
# TODO: Once we decide to split trait method lints based on whether the
# trait is sealed (e.g. see `trait_method_missing`), split this lint:
# - Sealed traits should ignore ineligible (~approx. `#[doc(hidden)]`)
# methods since they are indeed never public API.
# - Non-sealed traits should only ignore ineligible methods
# if they have a default implementation; otherwise, implementors
# of the trait have to impl the method regardless of `#[doc(hidden)]`.
public_api_eligible @filter(op: "=", value: ["$true"])
old_parameter_: parameter @fold @transform(op: "count") @output @tag(name: "parameters")
}
}
}
}
current {
item {
... on Trait {
visibility_limit @filter(op: "=", value: ["$public"])
importable_path {
path @filter(op: "=", value: ["%path"])
public_api @filter(op: "=", value: ["$true"])
}
method {
name @filter(op: "=", value: ["%method_name"])
new_parameter_: parameter @fold @transform(op: "count") @filter(op: "!=", value: ["%parameters"]) @output
span_: span @optional {
filename @output
begin_line @output
}
}
}
}
}
}
}"#,
arguments: {
"public": "public",
"true": true,
},
// TODO: This has a false-positive edge case, because it assumes the trait method
// was *previously* callable. This might not be the case for partially-sealed traits
// which can make some methods uncallable. In that case, the method signature change
// is not a breaking change, since it could never have been called in the first place.
// If the trait wasn't sealed and the method didn't have a default impl, then
// the change is still breaking even if the method wasn't callable -- it's just
// on the side of implementing the trait, not calling the method.
error_message: "A trait method now takes a different number of parameters.",
per_result_error_template: Some("{{name}}::{{method_name}} now takes {{new_parameter_count}} instead of {{old_parameter_count}} parameters, in file {{span_filename}}:{{span_begin_line}}"),
)
1 change: 1 addition & 0 deletions src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,7 @@ add_lints!(
trait_method_added,
trait_method_missing,
trait_method_now_doc_hidden,
trait_method_parameter_count_changed,
trait_method_unsafe_added,
trait_method_unsafe_removed,
trait_mismatched_generic_lifetimes,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "trait_method_parameter_count_changed"
version = "0.1.0"
edition = "2021"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub trait Example {
fn gain_a_parameter(x: i64);

fn lose_a_parameter();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "trait_method_parameter_count_changed"
version = "0.1.0"
edition = "2021"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub trait Example {
fn gain_a_parameter();

fn lose_a_parameter(a: i64);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
---
source: src/query.rs
expression: "&query_execution_results"
snapshot_kind: text
---
{
"./test_crates/trait_method_default_impl_removed/": [
{
"method_name": String("method_default_impl_removed_and_becomes_sealed"),
"name": String("TraitE"),
"new_parameter_count": Uint64(2),
"old_parameter_count": Uint64(1),
"path": List([
String("trait_method_default_impl_removed"),
String("TraitE"),
]),
"span_begin_line": Uint64(27),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
],
"./test_crates/trait_method_parameter_count_changed/": [
{
"method_name": String("gain_a_parameter"),
"name": String("Example"),
"new_parameter_count": Uint64(1),
"old_parameter_count": Uint64(0),
"path": List([
String("trait_method_parameter_count_changed"),
String("Example"),
]),
"span_begin_line": Uint64(2),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
{
"method_name": String("lose_a_parameter"),
"name": String("Example"),
"new_parameter_count": Uint64(0),
"old_parameter_count": Uint64(1),
"path": List([
String("trait_method_parameter_count_changed"),
String("Example"),
]),
"span_begin_line": Uint64(4),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
],
"./test_crates/trait_newly_sealed/": [
{
"method_name": String("method"),
"name": String("TraitMethodBecomesSealed"),
"new_parameter_count": Uint64(2),
"old_parameter_count": Uint64(1),
"path": List([
String("trait_newly_sealed"),
String("TraitMethodBecomesSealed"),
]),
"span_begin_line": Uint64(14),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
],
}

0 comments on commit 7eeec61

Please sign in to comment.