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 method_requires_different_const_generic_params lint. #1058

Merged
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
142 changes: 142 additions & 0 deletions src/lints/method_requires_different_const_generic_params.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
SemverQuery(
id: "method_requires_different_const_generic_params",
human_readable_name: "method now requires a different number of const generic parameters",
// Currently, const generics in functions and methods cannot have defaults set.
// This is why we have only one lint ("requires different number") instead of
// two separate lints ("requires" / "allows") like for structs/traits etc.
description: "A method now requires a different number of const generic parameters than before.",
required_update: Major,
lint_level: Deny,
reference_link: Some("https://doc.rust-lang.org/reference/items/generics.html#const-generics"),
query: r#"
{
CrateDiff {
baseline {
item {
... on ImplOwner {
visibility_limit @filter(op: "=", value: ["$public"])

importable_path {
path @tag @output
public_api @filter(op: "=", value: ["$true"])
}

inherent_impl {
method {
method_visibility: visibility_limit @filter(op: "=", value: ["$public"]) @output
method_name: name @output @tag
public_api_eligible @filter(op: "=", value: ["$true"])

generic_parameter @fold
@transform(op: "count")
@tag(name: "old_required_const_count")
@output(name: "old_required_const_count") {
... on GenericConstParameter {

old_required_consts: name @output
}
}

span_: span @optional {
filename @output
begin_line @output
}
}
}
}
}
}
current {
item {
... on ImplOwner {
visibility_limit @filter(op: "=", value: ["$public"]) @output
name @output

importable_path {
path @filter(op: "=", value: ["%path"])
public_api @filter(op: "=", value: ["$true"])
}

# We use "impl" instead of "inherent_impl" here because moving
# an inherently-implemented method to a trait is not necessarily
# a breaking change, but changing the number of const generics is.
#
# Method names are not unique on an ImplOwner! It's perfectly valid to have
# both an inherent method `foo()` as well as a trait-provided method
# `<Self as Bar>::foo()` at the same time. Whenever possible, rustc
# attempts to "do the right thing" and dispatch to the correct one.
#
# Because of the above, this check has to be written as
# "there is no method with the correct name and number of const generics"
# rather than the (incorrect!) alternative
# "the named method does not have the correct number of arguments."
#
# The above by itself is still not enough: say if the method was removed,
# that still makes the "there is no method ..." statement true.
# So we add an additional clause demanding that a method by that name
# with appropriate visibility actually exists.
impl @fold @transform(op: "count") @filter(op: ">", value: ["$zero"]) {
method {
visibility_limit @filter(op: "one_of", value: ["$public_or_default"])
name @filter(op: "=", value: ["%method_name"])
public_api_eligible @filter(op: "=", value: ["$true"])
}
}
impl @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
method {
visibility_limit @filter(op: "one_of", value: ["$public_or_default"])
name @filter(op: "=", value: ["%method_name"])
public_api_eligible @filter(op: "=", value: ["$true"])

generic_parameter @fold
@transform(op: "count")
@filter(op: "=", value: ["%old_required_const_count"]) {
... on GenericConstParameter {
name
}
}
}
}

# Get the non-matching methods by that name so we can report them
# in the lint error message.
impl @fold {
method {
visibility_limit @filter(op: "one_of", value: ["$public_or_default"])
name @filter(op: "=", value: ["%method_name"])
public_api_eligible @filter(op: "=", value: ["$true"])

generic_parameter @fold
@transform(op: "count")
@output(name: "new_required_const_count") {
... on GenericConstParameter {
new_required_consts: name @output
}
}

non_matching_span_: span @optional {
filename @output
begin_line @output
}
}
}
}
}
}
}
}"#,
arguments: {
"public": "public",
"public_or_default": ["public", "default"],
"true": true,
"zero": 0,
},
error_message: "A method now requires a different number of const generic parameters than it used to. Uses of this method that supplied the previous number of const generics will be broken.",
per_result_error_template: Some("{{join \"::\" path}}::{{method_name}} takes {{unpack_if_singleton new_required_const_count}} const generics instead of {{old_required_const_count}}, in {{multiple_spans non_matching_span_filename non_matching_span_begin_line}}"),
// TODO: see https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md#adding-a-witness
// for information about this field.
//
// The witness would be a invocation with the old number
// of const generics, which is insufficient for the new definition.
witness: None,
)
1 change: 1 addition & 0 deletions src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1172,6 +1172,7 @@ add_lints!(
macro_no_longer_exported,
macro_now_doc_hidden,
method_parameter_count_changed,
method_requires_different_const_generic_params,
module_missing,
non_exhaustive_struct_changed_type,
proc_macro_now_doc_hidden,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "method_requires_different_const_generic_params"
version = "0.1.0"
edition = "2021"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
pub struct Owner;

impl Owner {
pub fn previously_not_generic<const N: usize>() -> [i64; N] {
todo!()
}

pub fn added_const_generic<const N: usize, const M: usize>(data: [i64; N]) -> [i64; M] {
todo!()
}

pub fn removed_const_generic<const N: usize>(data: [i64; N]) -> [i64; 4] {
todo!()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "method_requires_different_const_generic_params"
version = "0.1.0"
edition = "2021"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pub struct Owner;

impl Owner {
pub fn previously_not_generic() {}

pub fn added_const_generic<const N: usize>(data: [i64; N]) {}

pub fn removed_const_generic<const N: usize, const M: usize>(data: [i64; N]) -> [i64; M] {
todo!()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
---
source: src/query.rs
expression: "&query_execution_results"
snapshot_kind: text
---
{
"./test_crates/method_requires_different_const_generic_params/": [
{
"method_name": String("previously_not_generic"),
"method_visibility": String("public"),
"name": String("Owner"),
"new_required_const_count": List([
Uint64(1),
]),
"new_required_consts": List([
List([
String("N"),
]),
]),
"non_matching_span_begin_line": List([
Uint64(4),
]),
"non_matching_span_filename": List([
String("src/lib.rs"),
]),
"old_required_const_count": Uint64(0),
"old_required_consts": List([]),
"path": List([
String("method_requires_different_const_generic_params"),
String("Owner"),
]),
"span_begin_line": Uint64(4),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
{
"method_name": String("added_const_generic"),
"method_visibility": String("public"),
"name": String("Owner"),
"new_required_const_count": List([
Uint64(2),
]),
"new_required_consts": List([
List([
String("N"),
String("M"),
]),
]),
"non_matching_span_begin_line": List([
Uint64(8),
]),
"non_matching_span_filename": List([
String("src/lib.rs"),
]),
"old_required_const_count": Uint64(1),
"old_required_consts": List([
String("N"),
]),
"path": List([
String("method_requires_different_const_generic_params"),
String("Owner"),
]),
"span_begin_line": Uint64(6),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
{
"method_name": String("removed_const_generic"),
"method_visibility": String("public"),
"name": String("Owner"),
"new_required_const_count": List([
Uint64(1),
]),
"new_required_consts": List([
List([
String("N"),
]),
]),
"non_matching_span_begin_line": List([
Uint64(12),
]),
"non_matching_span_filename": List([
String("src/lib.rs"),
]),
"old_required_const_count": Uint64(2),
"old_required_consts": List([
String("N"),
String("M"),
]),
"path": List([
String("method_requires_different_const_generic_params"),
String("Owner"),
]),
"span_begin_line": Uint64(8),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
],
}
Loading