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

Check for removal of methods and associated functions. #23

Merged
merged 1 commit into from
Jul 27, 2022
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
1 change: 1 addition & 0 deletions scripts/regenerate_test_rustdocs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ features=(
'enum_variant_added'
'enum_variant_missing'
'function_missing'
'inherent_method_missing'
'struct_marked_non_exhaustive'
'struct_missing'
'struct_pub_field_missing'
Expand Down
1 change: 1 addition & 0 deletions semver_tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ enum_missing = []
enum_variant_added = []
enum_variant_missing = []
function_missing = []
inherent_method_missing = []
struct_marked_non_exhaustive = []
struct_missing = []
struct_pub_field_missing = []
Expand Down
14 changes: 14 additions & 0 deletions semver_tests/src/test_cases/item_missing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,17 @@ pub enum WillBeRemovedEnum {}

#[cfg(not(feature = "function_missing"))]
pub fn will_be_removed_fn() {}

// Test that associated function removal:
// - is caught and reported by `inherent_method_missing`
// - is not caught by `function_missing`.
pub struct Foo {}

impl Foo {
#[cfg(not(feature = "function_missing"))]
#[cfg(not(feature = "inherent_method_missing"))]
pub fn will_be_removed_associated_fn() {}

#[cfg(not(feature = "inherent_method_missing"))]
pub fn will_be_removed_method(&self) {}
}
128 changes: 121 additions & 7 deletions src/adapter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::sync::Arc;

use rustdoc_types::{Crate, Enum, Function, Item, Method, Span, Struct, Type, Variant};
use rustdoc_types::{Crate, Enum, Function, Impl, Item, Method, Span, Struct, Type, Variant};
use trustfall_core::{
interpreter::{Adapter, DataContext, InterpretedQuery},
ir::{EdgeParameters, Eid, FieldValue, Vid},
Expand Down Expand Up @@ -94,6 +94,7 @@ impl<'a> Token<'a> {
rustdoc_types::ItemEnum::Variant(Variant::Tuple(..)) => "TupleVariant",
rustdoc_types::ItemEnum::Variant(Variant::Struct(..)) => "StructVariant",
rustdoc_types::ItemEnum::StructField(..) => "StructField",
rustdoc_types::ItemEnum::Impl(..) => "Impl",
_ => unreachable!("unexpected item.inner for item: {item:?}"),
},
TokenKind::Span(..) => "Span",
Expand Down Expand Up @@ -179,6 +180,13 @@ impl<'a> Token<'a> {
_ => None,
})
}

fn as_impl(&self) -> Option<&'a Impl> {
self.as_item().and_then(|item| match &item.inner {
rustdoc_types::ItemEnum::Impl(x) => Some(x),
_ => None,
})
}
}

impl<'a> From<&'a Item> for TokenKind<'a> {
Expand Down Expand Up @@ -297,6 +305,16 @@ fn get_function_like_property(token: &Token, field_name: &str) -> FieldValue {
}
}

fn get_impl_property(token: &Token, field_name: &str) -> FieldValue {
let impl_token = token.as_impl().expect("token was not an Impl");
match field_name {
"unsafe" => impl_token.is_unsafe.into(),
"negative" => impl_token.negative.into(),
"synthetic" => impl_token.synthetic.into(),
_ => unreachable!("Impl property {field_name}"),
}
}

fn property_mapper<'a>(
ctx: DataContext<Token<'a>>,
field_name: &str,
Expand Down Expand Up @@ -363,8 +381,8 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
property_mapper(ctx, field_name.as_ref(), get_item_property)
}))
}
"Struct" | "StructField" | "Enum" | "Variant" | "PlainVariant" | "TupleVariant"
| "StructVariant" | "Function" | "Method"
"ImplOwner" | "Struct" | "StructField" | "Enum" | "Variant" | "PlainVariant"
| "TupleVariant" | "StructVariant" | "Function" | "Method" | "Impl"
if matches!(
field_name.as_ref(),
"id" | "crate_id" | "name" | "docs" | "attrs" | "visibility_limit"
Expand Down Expand Up @@ -400,6 +418,11 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
property_mapper(ctx, field_name.as_ref(), get_function_like_property)
}))
}
"Impl" => {
Box::new(data_contexts.map(move |ctx| {
property_mapper(ctx, field_name.as_ref(), get_impl_property)
}))
}
_ => unreachable!("project_property {current_type_name} {field_name}"),
}
}
Expand Down Expand Up @@ -481,6 +504,7 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
| rustdoc_types::ItemEnum::Variant(..)
| rustdoc_types::ItemEnum::Function(..)
| rustdoc_types::ItemEnum::Method(..)
| rustdoc_types::ItemEnum::Impl(..)
)
})
.map(move |value| origin.make_item_token(value));
Expand All @@ -495,7 +519,9 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
),
}
}
"Importable" | "Struct" | "Enum" | "Function" if edge_name.as_ref() == "path" => {
"Importable" | "ImplOwner" | "Struct" | "Enum" | "Function"
if edge_name.as_ref() == "path" =>
{
let current_crate = self.current_crate;
let previous_crate = self.previous_crate;

Expand Down Expand Up @@ -528,8 +554,9 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
(ctx, neighbors)
}))
}
"Item" | "Struct" | "StructField" | "Enum" | "Variant" | "PlainVariant"
| "TupleVariant" | "StructVariant" | "Function" | "Method"
"Item" | "ImplOwner" | "Struct" | "StructField" | "Enum" | "Variant"
| "PlainVariant" | "TupleVariant" | "StructVariant" | "Function" | "Method"
| "Impl"
if edge_name.as_ref() == "span" =>
{
Box::new(data_contexts.map(move |ctx| {
Expand All @@ -550,6 +577,53 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
(ctx, neighbors)
}))
}
"ImplOwner" | "Struct" | "Enum"
if matches!(edge_name.as_ref(), "impl" | "inherent_impl") =>
{
let current_crate = self.current_crate;
let previous_crate = self.previous_crate;
let inherent_impls_only = edge_name.as_ref() == "inherent_impl";
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_index = match origin {
Origin::CurrentCrate => &current_crate.index,
Origin::PreviousCrate => {
&previous_crate.expect("no previous crate provided").index
}
};

// Get the IDs of all the impl blocks.
// Relies on the fact that only structs and enums can have impls,
// so we know that the token must represent either a struct or an enum.
let impl_ids = token
.as_struct_item()
.map(|(_, s)| &s.impls)
.or_else(|| token.as_enum().map(|e| &e.impls))
.expect("token was neither a struct nor an enum");

Box::new(impl_ids.iter().filter_map(move |item_id| {
let next_item = item_index.get(item_id);
next_item.and_then(|next_item| match &next_item.inner {
rustdoc_types::ItemEnum::Impl(imp) => {
if !inherent_impls_only || imp.trait_.is_none() {
Some(origin.make_item_token(next_item))
} else {
None
}
}
_ => None,
})
}))
}
};

(ctx, neighbors)
}))
}
"Struct" => match edge_name.as_ref() {
"field" => {
let current_crate = self.current_crate;
Expand Down Expand Up @@ -621,6 +695,44 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
unreachable!("project_neighbors {current_type_name} {edge_name} {parameters:?}")
}
},
"Impl" => match edge_name.as_ref() {
"method" => {
let current_crate = self.current_crate;
let previous_crate = self.previous_crate;
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_index = match origin {
Origin::CurrentCrate => &current_crate.index,
Origin::PreviousCrate => {
&previous_crate.expect("no previous crate provided").index
}
};

let impl_token = token.as_impl().expect("not an Impl token");
Box::new(impl_token.items.iter().filter_map(move |item_id| {
let next_item = &item_index[item_id];
match &next_item.inner {
rustdoc_types::ItemEnum::Method(..) => {
Some(origin.make_item_token(next_item))
}
_ => None,
}
}))
}
};

(ctx, neighbors)
}))
}
_ => {
unreachable!("project_neighbors {current_type_name} {edge_name} {parameters:?}")
}
},
_ => unreachable!("project_neighbors {current_type_name} {edge_name} {parameters:?}"),
}
}
Expand All @@ -634,7 +746,7 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
_vertex_hint: Vid,
) -> Box<dyn Iterator<Item = (DataContext<Self::DataToken>, bool)> + 'a> {
match current_type_name.as_ref() {
"Item" | "Variant" | "FunctionLike" => {
"Item" | "Variant" | "FunctionLike" | "Importable" | "ImplOwner" => {
Box::new(data_contexts.map(move |ctx| {
let can_coerce = match &ctx.current_token {
None => false,
Expand All @@ -646,6 +758,7 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
actual_type_name,
"PlainVariant" | "TupleVariant" | "StructVariant"
),
"ImplOwner" => matches!(actual_type_name, "Struct" | "Enum"),
_ => {
// The remaining types are final (don't have any subtypes)
// so we can just compare the actual type name to
Expand Down Expand Up @@ -744,6 +857,7 @@ mod tests {
enum_variant_added,
enum_variant_missing,
function_missing,
inherent_method_missing,
struct_marked_non_exhaustive,
struct_missing,
struct_pub_field_missing,
Expand Down
61 changes: 61 additions & 0 deletions src/queries/inherent_method_missing.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
SemverQuery(
id: "inherent_method_missing",
human_readable_name: "pub method removed or renamed",
description: "A publicly-visible method or associated fn is no longer available under its prior name, which is a major breaking change for code that depends on it.",
required_update: Major,
reference_link: Some("https://doc.rust-lang.org/cargo/reference/semver.html#item-remove"),
query: r#"
{
CrateDiff {
baseline {
item {
... on ImplOwner {
visibility_limit @filter(op: "=", value: ["$public"]) @output
name @output @tag

path {
path @output @tag
}

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

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

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

inherent_impl @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
method {
visibility_limit @filter(op: "=", value: ["$public"])
name @filter(op: "=", value: ["%method_name"])
}
}
}
}
}
}
}"#,
arguments: {
"public": "public",
"zero": 0,
},
error_message: "A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.",
per_result_error_template: Some("{{name}}::{{method_name}}, previously 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 @@ -73,6 +73,7 @@ impl SemverQuery {
include_str!("./queries/unit_struct_changed_kind.ron"),
include_str!("./queries/variant_marked_non_exhaustive.ron"),
include_str!("./queries/function_missing.ron"),
include_str!("./queries/inherent_method_missing.ron"),
];
for query_text in query_text_contents {
let query: SemverQuery = ron::from_str(query_text).expect("query failed to parse");
Expand Down
Loading