Check for #[repr(i*)]/#[repr(u*)] semver violations. (#29)

* Add type info to schema and use it in the query.

* Update query and add more test data.

* Support PhantomData markers in #[repr(transparent)] check.

* Check for removal of repr(u*) and repr(i*) attributes.

* Check for #[repr(i/u*)] changing to another integer repr.

scripts/regenerate_test_rustdocs.sh

@@ -22,6 +22,8 @@ mv "$RUSTDOC_OUTPUT" "$TARGET_DIR/baseline.json"
 features=(
     'enum_missing'
     'enum_repr_c_removed'
+    'enum_repr_int_changed'
+    'enum_repr_int_removed'
     'enum_variant_added'
     'enum_variant_missing'
     'function_missing'

semver_tests/Cargo.toml

@@ -9,6 +9,8 @@ edition = "2021"
 
 [features]
 enum_missing = []
+enum_repr_int_changed = []
+enum_repr_int_removed = []
 enum_repr_c_removed = []
 enum_variant_added = []
 enum_variant_missing = []

semver_tests/src/test_cases/enum_repr_int_changed.rs

@@ -0,0 +1,71 @@
+#[cfg(not(feature = "enum_repr_int_changed"))]
+#[repr(u8)]
+pub enum U8ToU16Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(feature = "enum_repr_int_changed")]
+#[repr(u16)]
+pub enum U8ToU16Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(not(feature = "enum_repr_int_changed"))]
+#[repr(i32)]
+pub enum I32ToI8Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(feature = "enum_repr_int_changed")]
+#[repr(i8)]
+pub enum I32ToI8Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(not(feature = "enum_repr_int_changed"))]
+#[repr(i32)]
+pub enum I32ToU32Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(feature = "enum_repr_int_changed")]
+#[repr(u32)]
+pub enum I32ToU32Enum {
+    Bar,
+    Baz,
+}
+
+// The following enums have *removals* of repr(i*) and repr(u*),
+// not changes to another repr(i*) or repr(u*).
+// They should not be reported by this rule, because they have their own rule.
+
+#[cfg(not(feature = "enum_repr_int_changed"))]
+#[repr(u8)]
+pub enum U8Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(feature = "enum_repr_int_changed")]
+pub enum U8Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(not(feature = "enum_repr_int_changed"))]
+#[repr(i32)]
+pub enum I32Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(feature = "enum_repr_int_changed")]
+pub enum I32Enum {
+    Bar,
+    Baz,
+}

semver_tests/src/test_cases/enum_repr_int_removed.rs

@@ -0,0 +1,25 @@
+#[cfg(not(feature = "enum_repr_int_removed"))]
+#[repr(u8)]
+pub enum U8Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(feature = "enum_repr_int_removed")]
+pub enum U8Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(not(feature = "enum_repr_int_removed"))]
+#[repr(i32)]
+pub enum I32Enum {
+    Bar,
+    Baz,
+}
+
+#[cfg(feature = "enum_repr_int_removed")]
+pub enum I32Enum {
+    Bar,
+    Baz,
+}

semver_tests/src/test_cases/mod.rs

@@ -1,4 +1,6 @@
 pub mod enum_repr_c_removed;
+pub mod enum_repr_int_changed;
+pub mod enum_repr_int_removed;
 pub mod enum_variant_added;
 pub mod enum_variant_missing;
 pub mod item_missing;

src/adapter.rs

@@ -61,6 +61,13 @@ impl Origin {
             kind: TokenKind::RawType(raw_type),
         }
     }
+
+    fn make_attribute_token<'a>(&self, attr: &'a str) -> Token<'a> {
+        Token {
+            origin: *self,
+            kind: TokenKind::Attribute(attr),
+        }
+    }
 }
 
 #[derive(Debug, Clone)]
@@ -69,15 +76,6 @@ pub struct Token<'a> {
     kind: TokenKind<'a>,
 }
 
-impl<'a> Token<'a> {
-    fn new_crate(origin: Origin, crate_: &'a Crate) -> Self {
-        Self {
-            origin,
-            kind: TokenKind::Crate(crate_),
-        }
-    }
-}
-
 #[derive(Debug, Clone)]
 pub enum TokenKind<'a> {
     CrateDiff((&'a Crate, &'a Crate)),
@@ -86,10 +84,18 @@ pub enum TokenKind<'a> {
     Span(&'a Span),
     Path(&'a [String]),
     RawType(&'a Type),
+    Attribute(&'a str),
 }
 
 #[allow(dead_code)]
 impl<'a> Token<'a> {
+    fn new_crate(origin: Origin, crate_: &'a Crate) -> Self {
+        Self {
+            origin,
+            kind: TokenKind::Crate(crate_),
+        }
+    }
+
     /// The name of the actual runtime type of this token,
     /// intended to fulfill resolution requests for the __typename property.
     #[inline]
@@ -111,6 +117,7 @@ impl<'a> Token<'a> {
             TokenKind::Path(..) => "Path",
             TokenKind::Crate(..) => "Crate",
             TokenKind::CrateDiff(..) => "CrateDiff",
+            TokenKind::Attribute(..) => "Attribute",
             TokenKind::RawType(ty) => match ty {
                 rustdoc_types::Type::ResolvedPath { .. } => "ResolvedPathType",
                 rustdoc_types::Type::Primitive(..) => "PrimitiveType",
@@ -203,6 +210,13 @@ impl<'a> Token<'a> {
         })
     }
 
+    fn as_attribute(&self) -> Option<&'a str> {
+        match &self.kind {
+            TokenKind::Attribute(attr) => Some(*attr),
+            _ => None,
+        }
+    }
+
     fn as_raw_type(&self) -> Option<&'a rustdoc_types::Type> {
         match &self.kind {
             TokenKind::RawType(ty) => Some(*ty),
@@ -337,6 +351,14 @@ fn get_impl_property(token: &Token, field_name: &str) -> FieldValue {
     }
 }
 
+fn get_attribute_property(token: &Token, field_name: &str) -> FieldValue {
+    let attribute_token = token.as_attribute().expect("token was not an Attribute");
+    match field_name {
+        "value" => attribute_token.into(),
+        _ => unreachable!("Attribute property {field_name}"),
+    }
+}
+
 fn get_resolved_path_type_property(token: &Token, field_name: &str) -> FieldValue {
     let type_token = token.as_raw_type().expect("token was not a RawType");
     match type_token {
@@ -467,6 +489,9 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
                         property_mapper(ctx, field_name.as_ref(), get_impl_property)
                     }))
                 }
+                "Attribute" => Box::new(data_contexts.map(move |ctx| {
+                    property_mapper(ctx, field_name.as_ref(), get_attribute_property)
+                })),
                 "ResolvedPathType" => Box::new(data_contexts.map(move |ctx| {
                     property_mapper(ctx, field_name.as_ref(), get_resolved_path_type_property)
                 })),
@@ -607,25 +632,47 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
             "Item" | "ImplOwner" | "Struct" | "StructField" | "Enum" | "Variant"
             | "PlainVariant" | "TupleVariant" | "StructVariant" | "Function" | "Method"
             | "Impl"
-                if edge_name.as_ref() == "span" =>
+                if matches!(edge_name.as_ref(), "span" | "attribute") =>
             {
-                Box::new(data_contexts.map(move |ctx| {
-                    let neighbors: Box<dyn Iterator<Item = Self::DataToken> + 'a> =
-                        match &ctx.current_token {
-                            None => Box::new(std::iter::empty()),
-                            Some(token) => {
-                                let origin = token.origin;
-                                let item = token.as_item().expect("token was not an Item");
-                                if let Some(span) = &item.span {
-                                    Box::new(std::iter::once(origin.make_span_token(span)))
-                                } else {
-                                    Box::new(std::iter::empty())
+                match edge_name.as_ref() {
+                    "span" => Box::new(data_contexts.map(move |ctx| {
+                        let neighbors: Box<dyn Iterator<Item = Self::DataToken> + 'a> =
+                            match &ctx.current_token {
+                                None => Box::new(std::iter::empty()),
+                                Some(token) => {
+                                    let origin = token.origin;
+                                    let item = token.as_item().expect("token was not an Item");
+                                    if let Some(span) = &item.span {
+                                        Box::new(std::iter::once(origin.make_span_token(span)))
+                                    } else {
+                                        Box::new(std::iter::empty())
+                                    }
                                 }
-                            }
-                        };
+                            };
 
-                    (ctx, neighbors)
-                }))
+                        (ctx, neighbors)
+                    })),
+                    "attribute" => Box::new(data_contexts.map(move |ctx| {
+                        let neighbors: Box<dyn Iterator<Item = Self::DataToken> + 'a> =
+                            match &ctx.current_token {
+                                None => Box::new(std::iter::empty()),
+                                Some(token) => {
+                                    let origin = token.origin;
+                                    let item = token.as_item().expect("token was not an Item");
+                                    Box::new(
+                                        item.attrs
+                                            .iter()
+                                            .map(move |attr| origin.make_attribute_token(attr)),
+                                    )
+                                }
+                            };
+
+                        (ctx, neighbors)
+                    })),
+                    _ => unreachable!(
+                        "project_neighbors {current_type_name} {edge_name} {parameters:?}"
+                    ),
+                }
             }
             "ImplOwner" | "Struct" | "Enum"
                 if matches!(edge_name.as_ref(), "impl" | "inherent_impl") =>
@@ -959,6 +1006,8 @@ mod tests {
     query_execution_tests!(
         enum_missing,
         enum_repr_c_removed,
+        enum_repr_int_changed,
+        enum_repr_int_removed,
         enum_variant_added,
         enum_variant_missing,
         function_missing,

src/queries/enum_repr_int_changed.ron

@@ -0,0 +1,66 @@
+SemverQuery(
+    id: "enum_repr_int_changed",
+    human_readable_name: "enum repr(u*)/repr(i*) changed",
+    description: "The repr(u*) or repr(i*) attribute on an enum was changed to another integer type. This can cause its memory representation to change, breaking FFI use cases.",
+    required_update: Major,
+
+    // TODO: Change the reference link to point to the cargo semver reference
+    //       once it has a section on repr(u*)/repr(i*).
+    reference_link: Some("https://doc.rust-lang.org/nomicon/other-reprs.html#repru-repri"),
+    query: r#"
+    {
+        CrateDiff {
+            baseline {
+                item {
+                    ... on Enum {
+                        visibility_limit @filter(op: "=", value: ["$public"]) @output
+                        name @tag @output
+
+                        attribute {
+                            attr_value: value @filter(op: "regex", value: ["$repr_regex"])
+                                                @tag @output(name: "old_attr")
+                        }
+
+                        path {
+                            path @tag @output
+                        }
+                    }
+                }
+            }
+            current {
+                item {
+                    ... on Enum {
+                        visibility_limit @filter(op: "=", value: ["$public"])
+                        name @filter(op: "=", value: ["%name"])
+
+                        path {
+                            path @filter(op: "=", value: ["%path"])
+                        }
+
+                        # Check that there exists an attribute that:
+                        # - looks like repr(i*) or repr(u*)
+                        # - but is not the same repr(i*) or repr(u*) that we had before.
+                        # This is the breaking change.
+                        attribute @fold @transform(op: "count") @filter(op: ">", value: ["$zero"]) {
+                            value @filter(op: "regex", value: ["$repr_regex"])
+                                    @filter(op: "!=", value: ["%attr_value"])
+                                    @output(name: "new_attr")
+                        }
+
+                        span_: span @optional {
+                            filename @output
+                            begin_line @output
+                        }
+                    }
+                }
+            }
+        }
+    }"#,
+    arguments: {
+        "public": "public",
+        "repr_regex": "#\\[repr\\([ui]\\d+\\)\\]",
+        "zero": 0,
+    },
+    error_message: "The repr(u*) or repr(i*) attribute on an enum was changed to another integer type. This can cause its memory representation to change, breaking FFI use cases.",
+    per_result_error_template: Some("enum {{name}} {{old_attr}} -> {{new_attr.0}} in {{span_filename}}:{{span_begin_line}}"),
+)