Add lints for type generics in functions, methods, and trait methods. (…

…#1081)

Adds the lints:
- `function_requires_different_generic_type_params`
- `method_requires_different_generic_type_params`
- `trait_method_requires_different_generic_type_params`

src/lints/function_requires_different_generic_type_params.ron

@@ -0,0 +1,82 @@
+SemverQuery(
+    id: "function_requires_different_generic_type_params",
+    human_readable_name: "function now requires a different number of generic type parameters",
+    // Currently, generic types 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 function now requires a different number of generic type parameters than before.",
+    required_update: Major,
+    lint_level: Deny,
+    reference_link: Some("https://doc.rust-lang.org/reference/items/generics.html"),
+    query: r#"
+    {
+        CrateDiff {
+            baseline {
+                item {
+                    ... on Function {
+                        visibility_limit @filter(op: "=", value: ["$public"])
+                        name @output
+
+                        importable_path {
+                            path @tag @output
+                            public_api @filter(op: "=", value: ["$true"])
+                        }
+
+                        generic_parameter @fold
+                                          @transform(op: "count")
+                                          @tag(name: "old_required_type_count")
+                                          @output(name: "old_required_type_count") {
+                            ... on GenericTypeParameter {
+                                # Ignore generic type parameters introduced by `impl Trait`.
+                                synthetic @filter(op: "!=", value: ["$true"])
+
+                                old_required_types: name @output
+                            }
+                        }
+                    }
+                }
+            }
+            current {
+                item {
+                    ... on Function {
+                        visibility_limit @filter(op: "=", value: ["$public"]) @output
+
+                        importable_path {
+                            path @filter(op: "=", value: ["%path"])
+                            public_api @filter(op: "=", value: ["$true"])
+                        }
+
+                        generic_parameter @fold
+                                          @transform(op: "count")
+                                          @filter(op: "!=", value: ["%old_required_type_count"])
+                                          @output(name: "new_required_type_count") {
+                            ... on GenericTypeParameter {
+                                # Ignore generic type parameters introduced by `impl Trait`.
+                                synthetic @filter(op: "!=", value: ["$true"])
+
+                                new_required_types: name @output
+                            }
+                        }
+
+                        span_: span @optional {
+                            filename @output
+                            begin_line @output
+                        }
+                    }
+                }
+            }
+        }
+    }"#,
+    arguments: {
+        "public": "public",
+        "true": true,
+    },
+    error_message: "A function now requires a different number of generic type parameters than it used to. Uses of this function that supplied the previous number of generic types will be broken.",
+    per_result_error_template: Some("function {{name}} ({{old_required_type_count}} -> {{new_required_type_count}} generic types) in {{span_filename}}:{{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 function invocation with the old number
+    // of generic types, which is insufficient for the new definition.
+    witness: None,
+)

src/lints/method_requires_different_generic_type_params.ron

@@ -0,0 +1,150 @@
+SemverQuery(
+    id: "method_requires_different_generic_type_params",
+    human_readable_name: "method now requires a different number of generic type parameters",
+    // Currently, generic types 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 generic type parameters than before.",
+    required_update: Major,
+    lint_level: Deny,
+    reference_link: Some("https://doc.rust-lang.org/reference/items/generics.html"),
+    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_type_count")
+                                                  @output(name: "old_required_type_count") {
+                                    ... on GenericTypeParameter {
+                                        # Ignore generic type parameters introduced by `impl Trait`.
+                                        synthetic @filter(op: "!=", value: ["$true"])
+
+                                        old_required_types: 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 generic types 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 generic types"
+                        # 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_type_count"]) {
+                                    ... on GenericTypeParameter {
+                                        # Ignore generic type parameters introduced by `impl Trait`.
+                                        synthetic @filter(op: "!=", value: ["$true"])
+
+                                        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_type_count") {
+                                    ... on GenericTypeParameter {
+                                        # Ignore generic type parameters introduced by `impl Trait`.
+                                        synthetic @filter(op: "!=", value: ["$true"])
+
+                                        new_required_types: 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 generic type parameters than it used to. Uses of this method that supplied the previous number of generic types will be broken.",
+    per_result_error_template: Some("{{join \"::\" path}}::{{method_name}} takes {{unpack_if_singleton new_required_type_count}} generic types instead of {{old_required_type_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 generic types, which is insufficient for the new definition.
+    witness: None,
+)

src/lints/trait_allows_fewer_generic_type_params.ron

@@ -43,7 +43,7 @@ SemverQuery(
                         generic_parameter @fold
                                           @transform(op: "count")
                                           @filter(op: "<", value: ["%old_allowed_type_count"])
-                                          @output(name: "new_allowed_typeold_allowed_type_count") {
+                                          @output(name: "new_required_type_count") {
                             ... on GenericTypeParameter {
                                 new_allowed_types: name @output
                             }
@@ -63,7 +63,7 @@ SemverQuery(
         "true": true,
     },
     error_message: "A trait now allows fewer generic type parameters than it used to. Uses of this trait that supplied all previously-supported generic types will be broken.",
-    per_result_error_template: Some("trait {{name}} allows {{old_allowed_type_count}} -> {{new_allowed_typeold_allowed_type_count}} generic types in {{span_filename}}:{{span_begin_line}}"),
+    per_result_error_template: Some("trait {{name}} allows {{old_allowed_type_count}} -> {{new_required_type_count}} generic types in {{span_filename}}:{{span_begin_line}}"),
     // TODO: see https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md#adding-a-witness
     // for information about this field.
     //

src/lints/trait_method_requires_different_generic_type_params.ron

@@ -0,0 +1,98 @@
+SemverQuery(
+    id: "trait_method_requires_different_generic_type_params",
+    human_readable_name: "trait method now requires a different number of generic type parameters",
+    // Currently, generic types 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 trait method now requires a different number of generic type parameters than before.",
+    required_update: Major,
+    lint_level: Deny,
+    reference_link: Some("https://doc.rust-lang.org/reference/items/generics.html"),
+    query: r#"
+    {
+        CrateDiff {
+            baseline {
+                item {
+                    ... on Trait {
+                        visibility_limit @filter(op: "=", value: ["$public"])
+                        name @output
+
+                        importable_path {
+                            path @tag @output
+                            public_api @filter(op: "=", value: ["$true"])
+                        }
+
+                        method {
+                            public_api_eligible @filter(op: "=", value: ["$true"])
+                            method: name @output @tag
+
+                            generic_parameter @fold
+                                              @transform(op: "count")
+                                              @tag(name: "old_required_type_count")
+                                              @output(name: "old_required_type_count") {
+                                ... on GenericTypeParameter {
+                                    # Ignore generic type parameters introduced by `impl Trait`.
+                                    synthetic @filter(op: "!=", value: ["$true"])
+
+                                    old_required_types: name @output
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+            current {
+                item {
+                    ... on Trait {
+                        visibility_limit @filter(op: "=", value: ["$public"]) @output
+
+                        importable_path {
+                            path @filter(op: "=", value: ["%path"])
+                            public_api @filter(op: "=", value: ["$true"])
+                        }
+
+                        method {
+                            public_api_eligible @filter(op: "=", value: ["$true"])
+                            name @filter(op: "=", value: ["%method"])
+
+                            generic_parameter @fold
+                                              @transform(op: "count")
+                                              @filter(op: "!=", value: ["%old_required_type_count"])
+                                              @output(name: "new_required_type_count") {
+                                ... on GenericTypeParameter {
+                                    # Ignore generic type parameters introduced by `impl Trait`.
+                                    synthetic @filter(op: "!=", value: ["$true"])
+
+                                    new_required_types: name @output
+                                }
+                            }
+
+                            span_: span @optional {
+                                filename @output
+                                begin_line @output
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }"#,
+    arguments: {
+        "public": "public",
+        "true": true,
+    },
+    error_message: "A trait method now requires a different number of generic type parameters than it used to. Calls or implementations of this trait method using the previous number of generic types will be broken.",
+    per_result_error_template: Some("{{name}}::{{method}} ({{old_required_type_count}} -> {{new_required_type_count}} generic types) in {{span_filename}}:{{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 here depends on whether the trait method is implementable or sealed,
+    // and whether the method is callable.
+    // - If the method is callable, the witness would be a trait method invocation
+    //   with the old number of generic types, which is insufficient for the new definition.
+    // - If the trait is not sealed, and the method is implementable, then the witness would be
+    //   an implementation of that trait that supplies an implementation of the method with the old
+    //   number of generic types. Refinement by making a method implementation more generic isn't
+    //   supported, so this is always breaking in current Rust.
+    witness: None,
+)

src/query.rs

@@ -1162,6 +1162,7 @@ add_lints!(
     function_now_doc_hidden,
     function_parameter_count_changed,
     function_requires_different_const_generic_params,
+    function_requires_different_generic_type_params,
     function_unsafe_added,
     inherent_associated_const_now_doc_hidden,
     inherent_associated_pub_const_missing,
@@ -1174,6 +1175,7 @@ add_lints!(
     macro_now_doc_hidden,
     method_parameter_count_changed,
     method_requires_different_const_generic_params,
+    method_requires_different_generic_type_params,
     module_missing,
     non_exhaustive_struct_changed_type,
     proc_macro_now_doc_hidden,
@@ -1211,6 +1213,7 @@ add_lints!(
     trait_method_now_doc_hidden,
     trait_method_parameter_count_changed,
     trait_method_requires_different_const_generic_params,
+    trait_method_requires_different_generic_type_params,
     trait_method_unsafe_added,
     trait_method_unsafe_removed,
     trait_mismatched_generic_lifetimes,

test_crates/function_requires_different_generic_type_params/new/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+publish = false
+name = "function_requires_different_generic_type_params"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]

test_crates/function_requires_different_generic_type_params/new/src/lib.rs

@@ -0,0 +1,13 @@
+pub fn previously_not_generic<T>() -> T {
+    todo!()
+}
+
+pub fn add_generic_type<T, U>(data: T) -> U {
+    todo!()
+}
+
+pub fn remove_generic_type(data: i64) {}
+
+// Not a major breaking change!
+// https://predr.ag/blog/some-rust-breaking-changes-do-not-require-major-version/
+pub fn becomes_impl_trait(data: impl Into<String>) {}