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

new lint: pub_api_sealed_trait_became_unconditionally_sealed #1166

Merged
Show file tree
Hide file tree
Changes from 1 commit
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
55 changes: 55 additions & 0 deletions src/lints/pub_api_sealed_trait_became_unconditionally_sealed.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
SemverQuery(
id: "pub_api_sealed_trait_became_unconditionally_sealed",
human_readable_name: "Public API sealed trait became unconditionally sealed",
description: "A trait that was previously only sealed to public API consumers is now unconditionally sealed.
This change prevents all external implementations, including from related first-party crates
that might have relied on private API access",
required_update: Major,
lint_level: Deny,
Copy link
Owner

Choose a reason for hiding this comment

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

Is this the right lint level here?

reference_link: Some("https://rust-lang.github.io/api-guidelines/future-proofing.html#sealed-traits-protect-against-downstream-implementations-c-sealed"),
query: r#"
{
CrateDiff {
baseline {
item {
... on Trait {
visibility_limit @filter(op: "=", value: ["$public"]) @output
public_api_sealed @filter(op: "=", value: ["$true"])
unconditionally_sealed @filter(op: "!=", value: ["$true"])
importable_path {
path @output @tag
public_api @filter(op: "=", value: ["$true"])
}
}
}
}
current {
item {
... on Trait {
visibility_limit @filter(op: "=", value: ["$public"])
unconditionally_sealed @filter(op: "=", value: ["$true"])
name @output
importable_path {
path @filter(op: "=", value: ["%path"])
public_api @filter(op: "=", value: ["$true"])
}
span_: span @optional {
filename @output
begin_line @output
end_line @output
}
Copy link
Owner

Choose a reason for hiding this comment

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

nit: Please take a look at some of the other lints' queries and try to match the style and formatting of queries. The fact that this query is so dense and has no whitespace means it's quite difficult to read.

}
}
}
}
}"#,
arguments: {
"public": "public",
"true": true,
},
error_message: "A public API sealed trait is now unconditionally sealed, preventing all external implementations.
This change impacts related first-party crates (e.g., derive macros or other internal tooling)
that previously could implement the trait using non-public API",
Copy link
Owner

Choose a reason for hiding this comment

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

I don't believe this will get formatted correctly with our RON format. I think it'll end up stored and read as a multi-line string with the exact set of newline and consecutive space characters as exist in this file.

You can verify this by running cargo run -- semver-checks --manifest-path test_crates/pub_api_sealed_trait_became_unconditionally_sealed/new/Cargo.toml --baseline-root test_crates/pub_api_sealed_trait_became_unconditionally_sealed/old/Cargo.toml and seeing whether the error message is printed correctly. I suspect it'll come out hard-wrapped in an unpredictable and undesirable way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, the output is rather abnormal. I rewritten that to match other queries

per_result_error_template: Some("trait {{join \"::\" path}} in file {{span_filename}}:{{span_begin_line}}"),
witness: None,
)
1 change: 1 addition & 0 deletions src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1285,6 +1285,7 @@ add_lints!(
partial_ord_struct_fields_reordered,
proc_macro_marked_deprecated,
proc_macro_now_doc_hidden,
pub_api_sealed_trait_became_unconditionally_sealed,
pub_module_level_const_missing,
pub_module_level_const_now_doc_hidden,
pub_static_missing,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "pub_api_sealed_trait_became_unconditionally_sealed"
version = "0.1.0"
edition = "2021"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Traits transitioning from Public API Sealed → Unconditionally Sealed (Lint should detect these)
pub mod public_api_sealed_to_unconditionally_sealed {
mod hidden {
pub trait Sealed {}
pub struct Token;
}

pub trait TraitExtendsUnconditionallyHiddenTrait: hidden::Sealed {}

pub trait MethodReturningUnconditionallyHiddenToken {
fn method(&self) -> hidden::Token;
}

pub trait MethodTakingUnconditionallyHiddenToken {
fn method(&self, token: hidden::Token);
}

pub trait HiddenSealedWithWhereSelfBound
where
Self: hidden::Sealed,
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "pub_api_sealed_trait_became_unconditionally_sealed"
version = "0.1.0"
edition = "2021"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Traits transitioning from Public API Sealed → Unconditionally Sealed (Lint should detect these)
pub mod public_api_sealed_to_unconditionally_sealed {
#[doc(hidden)]
pub mod hidden {
pub trait Sealed {}
pub struct Token;
}

pub trait TraitExtendsUnconditionallyHiddenTrait: hidden::Sealed {}

pub trait MethodReturningUnconditionallyHiddenToken {
fn method(&self) -> hidden::Token;
}

pub trait MethodTakingUnconditionallyHiddenToken {
fn method(&self, token: hidden::Token);
}

pub trait HiddenSealedWithWhereSelfBound
where
Self: hidden::Sealed,
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
---
source: src/query.rs
expression: "&query_execution_results"
---
{
"./test_crates/pub_api_sealed_trait_became_unconditionally_sealed/": [
{
"name": String("TraitExtendsUnconditionallyHiddenTrait"),
"path": List([
String("pub_api_sealed_trait_became_unconditionally_sealed"),
String("public_api_sealed_to_unconditionally_sealed"),
String("TraitExtendsUnconditionallyHiddenTrait"),
]),
"span_begin_line": Uint64(8),
"span_end_line": Uint64(8),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
{
"name": String("MethodReturningUnconditionallyHiddenToken"),
"path": List([
String("pub_api_sealed_trait_became_unconditionally_sealed"),
String("public_api_sealed_to_unconditionally_sealed"),
String("MethodReturningUnconditionallyHiddenToken"),
]),
"span_begin_line": Uint64(10),
"span_end_line": Uint64(12),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
{
"name": String("MethodTakingUnconditionallyHiddenToken"),
"path": List([
String("pub_api_sealed_trait_became_unconditionally_sealed"),
String("public_api_sealed_to_unconditionally_sealed"),
String("MethodTakingUnconditionallyHiddenToken"),
]),
"span_begin_line": Uint64(14),
"span_end_line": Uint64(16),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
{
"name": String("HiddenSealedWithWhereSelfBound"),
"path": List([
String("pub_api_sealed_trait_became_unconditionally_sealed"),
String("public_api_sealed_to_unconditionally_sealed"),
String("HiddenSealedWithWhereSelfBound"),
]),
"span_begin_line": Uint64(18),
"span_end_line": Uint64(22),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
],
"./test_crates/trait_newly_sealed/": [
{
"name": String("PublicAPISealed"),
"path": List([
String("trait_newly_sealed"),
String("PublicAPISealed"),
]),
"span_begin_line": Uint64(29),
"span_end_line": Uint64(32),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
],
}
Loading