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

Test case: replace type with a more generic one without major change. #337

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "type_alias_over_more_generic_underlying"
version = "0.1.0"
edition = "2021"

[dependencies]
78 changes: 78 additions & 0 deletions test_crates/type_alias_over_more_generic_underlying/new/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
//! We're going to replace the original type (named like `OriginalType`) with two types:
//! - `pub struct NewType<A, B = ()>`
//! - `pub type OriginalType<A> = NewType<A>`
//!
//! This is not semver-major, since the type alias has:
//! - the same name
//! - the same fields/variants
//! - the same constructibility and pattern-matching behavior.

pub struct NewStructOnlyPrivateFields<A> {
_marker: std::marker::PhantomData<A>,
}

pub type OriginalStructOnlyPrivateFields = NewStructOnlyPrivateFields<()>;

// ---

pub struct NewStructGeneric<A, B = ()> {
_marker: std::marker::PhantomData<(A, B)>,
}

pub type OriginalStructGeneric<A> = NewStructGeneric<A>;

// ---

pub struct NewStructMixOfFields<A> {
_marker: std::marker::PhantomData<A>,
pub x: i64,
}

pub type OriginalStructMixOfFields = NewStructMixOfFields<()>;

// ---

pub struct NewStructConstructible<T> {
pub x: T,
}

pub type OriginalStructConstructible = NewStructConstructible<i64>;
Comment on lines +35 to +39
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the case you need to look into of whether it is semver compatible. I have a vague recollection of someone warning about replacing a type with type and I think it was with {} construction syntax.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be another great use of witness crates (#223), except to demonstrate that some syntax does not break, instead of demonstrating that it breaks.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I either found the issue you had in mind or something related: #338

TL;DR: pub type replacement of a constructible unit or tuple struct is breaking, since the unit/tuple constructor is not usable on the type alias.

Copy link

@QuineDot QuineDot Feb 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trait bounds on type aliases aren't checked. This is linted (sometimes an error, sometimes a warning), but the lint has false positives.

Additionally -- and as far as I know this is not documented anywhere -- while lifetime constraints aren't enforced per se, they do have semantic impact. Therefore, not having the same lifetime bounds (or lack of bounds) on the type as are on the original struct is a breaking change. But if it doesn't cause local breakage, people who get the lint warning are likely to apply the suggestion and remove the semantically relevant bounds.

I suspect most people don't know the dyn Trait lifetime default mechanism in the first place, and RFC 2093 made the potential for breakage a lot more nuanced to boot, so that may be an area to look into independently of type aliases as well.


// ---

pub struct NewStructEmpty<A> {
_marker: std::marker::PhantomData<A>,
}

pub type OriginalStructEmpty = NewStructEmpty<()>;

// ---

#[non_exhaustive]
pub struct NewStructConstructibleNonexhaustive<A> {
pub x: A,
}

pub type OriginalStructConstructibleNonexhaustive = NewStructConstructibleNonexhaustive<i64>;

// ---

#[non_exhaustive]
pub struct NewStructEmptyNonexhaustive<A> {
pub x: A,
}

pub type OriginalStructEmptyNonexhaustive = NewStructEmptyNonexhaustive<i64>;

// ---
// The enum case is interesting: to add a generic parameter, we need to use it somewhere.
// We'll need to add a variant, so the original enum must have been non-exhaustive
// to avoid an immediate major breaking change.

#[non_exhaustive]
pub enum NewEnumNonexhaustive<A> {
First,
Second(A),
}

pub type OriginalEnumNonexhaustive = NewEnumNonexhaustive<()>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "type_alias_over_more_generic_underlying"
version = "0.1.0"
edition = "2021"

[dependencies]
46 changes: 46 additions & 0 deletions test_crates/type_alias_over_more_generic_underlying/old/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
//! We're going to replace the original type (named like `OriginalType`) with two types:
//! - `pub struct NewType<A, B = ()>`
//! - `pub type OriginalType<A> = NewType<A>`
//!
//! This is not semver-major, since the type alias has:
//! - the same name
//! - the same fields
//! - the same constructibility and pattern-matching behavior.

pub struct OriginalStructOnlyPrivateFields {
_marker: std::marker::PhantomData<()>,
}

pub struct OriginalStructGeneric<A> {
_marker: std::marker::PhantomData<A>,
}

pub struct OriginalStructMixOfFields {
_marker: std::marker::PhantomData<()>,
pub x: i64,
}

pub struct OriginalStructConstructible {
pub x: i64,
}

pub struct OriginalStructEmpty {}

#[non_exhaustive]
pub struct OriginalStructConstructibleNonexhaustive {
pub x: i64,
}

#[non_exhaustive]
pub struct OriginalStructEmptyNonexhaustive {
pub x: i64,
}

// The enum case is interesting: to add a generic parameter, we need to use it somewhere.
// We'll need to add a variant, so the original enum must have been non-exhaustive
// to avoid an immediate major breaking change.

#[non_exhaustive]
pub enum OriginalEnumNonexhaustive {
First,
}