diff --git a/src/lints/method_requires_different_const_generic_params.ron b/src/lints/method_requires_different_const_generic_params.ron new file mode 100644 index 00000000..1b221fa3 --- /dev/null +++ b/src/lints/method_requires_different_const_generic_params.ron @@ -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 + # `::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, +) diff --git a/src/query.rs b/src/query.rs index 0e22e8af..1e18f5bd 100644 --- a/src/query.rs +++ b/src/query.rs @@ -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, diff --git a/test_crates/method_requires_different_const_generic_params/new/Cargo.toml b/test_crates/method_requires_different_const_generic_params/new/Cargo.toml new file mode 100644 index 00000000..54148952 --- /dev/null +++ b/test_crates/method_requires_different_const_generic_params/new/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "method_requires_different_const_generic_params" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/method_requires_different_const_generic_params/new/src/lib.rs b/test_crates/method_requires_different_const_generic_params/new/src/lib.rs new file mode 100644 index 00000000..43ac39a4 --- /dev/null +++ b/test_crates/method_requires_different_const_generic_params/new/src/lib.rs @@ -0,0 +1,15 @@ +pub struct Owner; + +impl Owner { + pub fn previously_not_generic() -> [i64; N] { + todo!() + } + + pub fn added_const_generic(data: [i64; N]) -> [i64; M] { + todo!() + } + + pub fn removed_const_generic(data: [i64; N]) -> [i64; 4] { + todo!() + } +} diff --git a/test_crates/method_requires_different_const_generic_params/old/Cargo.toml b/test_crates/method_requires_different_const_generic_params/old/Cargo.toml new file mode 100644 index 00000000..54148952 --- /dev/null +++ b/test_crates/method_requires_different_const_generic_params/old/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "method_requires_different_const_generic_params" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/method_requires_different_const_generic_params/old/src/lib.rs b/test_crates/method_requires_different_const_generic_params/old/src/lib.rs new file mode 100644 index 00000000..ce931c67 --- /dev/null +++ b/test_crates/method_requires_different_const_generic_params/old/src/lib.rs @@ -0,0 +1,11 @@ +pub struct Owner; + +impl Owner { + pub fn previously_not_generic() {} + + pub fn added_const_generic(data: [i64; N]) {} + + pub fn removed_const_generic(data: [i64; N]) -> [i64; M] { + todo!() + } +} diff --git a/test_outputs/query_execution/method_requires_different_const_generic_params.snap b/test_outputs/query_execution/method_requires_different_const_generic_params.snap new file mode 100644 index 00000000..ede9f12e --- /dev/null +++ b/test_outputs/query_execution/method_requires_different_const_generic_params.snap @@ -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"), + }, + ], +}