Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add trait_method_parameter_count_changed lint. #1059

Merged
merged 2 commits into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1207,6 +1207,7 @@ add_lints!(
trait_method_added,
trait_method_missing,
trait_method_now_doc_hidden,
trait_method_parameter_count_changed,
trait_method_requires_different_const_generic_params,
trait_method_unsafe_added,
trait_method_unsafe_removed,
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"),
},
],
}
Loading