Skip to content

Commit

Permalink
Fix for moving method to trait (#24)
Browse files Browse the repository at this point in the history
* Do not report methods moved to a trait as breaking changes.

* Release 0.5.0.
  • Loading branch information
obi1kenobi authored Jul 27, 2022
1 parent 31dd0bf commit e879e68
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 7 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-semver-checks"
version = "0.4.4"
version = "0.5.0"
edition = "2021"
authors = ["Predrag Gruevski <obi1kenobi82@gmail.com>"]
license = "Apache-2.0"
Expand Down
20 changes: 20 additions & 0 deletions semver_tests/src/test_cases/item_missing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,24 @@ impl Foo {

#[cfg(not(feature = "inherent_method_missing"))]
pub fn will_be_removed_method(&self) {}

#[cfg(not(feature = "inherent_method_missing"))]
pub fn moved_trait_provided_method(&self) {}

#[cfg(not(feature = "inherent_method_missing"))]
pub fn moved_method(&self) {}
}

// Moving an inherent method to an implemented trait should not be a breaking change,
// both when the method is defined inside the trait and when it's implemented externally.
#[cfg(feature = "inherent_method_missing")]
pub trait Bar {
fn moved_trait_provided_method(&self) {}

fn moved_method(&self);
}

#[cfg(feature = "inherent_method_missing")]
impl Bar for Foo {
fn moved_method(&self) {}
}
34 changes: 31 additions & 3 deletions src/adapter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::sync::Arc;
use std::{collections::BTreeSet, sync::Arc};

use rustdoc_types::{Crate, Enum, Function, Impl, Item, Method, Span, Struct, Type, Variant};
use rustdoc_types::{
Crate, Enum, Function, Id, Impl, Item, ItemEnum, Method, Span, Struct, Type, Variant,
};
use trustfall_core::{
interpreter::{Adapter, DataContext, InterpretedQuery},
ir::{EdgeParameters, Eid, FieldValue, Vid},
Expand Down Expand Up @@ -714,7 +716,33 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
};

let impl_token = token.as_impl().expect("not an Impl token");
Box::new(impl_token.items.iter().filter_map(move |item_id| {
let provided_methods: Box<dyn Iterator<Item = &Id>> = if impl_token.provided_trait_methods.is_empty() {
Box::new(std::iter::empty())
} else {
let method_names: BTreeSet<&str> = impl_token.provided_trait_methods.iter().map(|x| x.as_str()).collect();

let trait_type = impl_token.trait_.as_ref().expect("no trait but provided_trait_methods was non-empty");
let trait_item = match trait_type {
Type::ResolvedPath { name: _, id, args: _, param_names: _ } => {
&item_index[id]
}
_ => unimplemented!("found provided_trait_methods when the trait was not a ResolvedPath: {trait_type:?}"),
};

if let ItemEnum::Trait(trait_item) = &trait_item.inner {
Box::new(trait_item.items.iter().filter(move |item_id| {
let next_item = &item_index[*item_id];
if let Some(name) = &next_item.name {
method_names.contains(name.as_str())
} else {
false
}
}))
} else {
unreachable!("found a non-trait type {trait_item:?}");
}
};
Box::new(provided_methods.chain(impl_token.items.iter()).filter_map(move |item_id| {
let next_item = &item_index[item_id];
match &next_item.inner {
rustdoc_types::ItemEnum::Method(..) => {
Expand Down
8 changes: 6 additions & 2 deletions src/queries/inherent_method_missing.ron
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ SemverQuery(
path @filter(op: "=", value: ["%path"])
}
inherent_impl @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
# We use "impl" instead of "inherent_impl" here because moving
# an inherently-implemented method to a trait is not necessarily
# a breaking change, so we don't want to report it.
impl @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
method {
visibility_limit @filter(op: "=", value: ["$public"])
visibility_limit @filter(op: "one_of", value: ["$public_or_default"])
name @filter(op: "=", value: ["%method_name"])
}
}
Expand All @@ -54,6 +57,7 @@ SemverQuery(
}"#,
arguments: {
"public": "public",
"public_or_default": ["public", "default"],
"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.",
Expand Down

0 comments on commit e879e68

Please # to comment.