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: struct_added_without_non_exhaustive #1091

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 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
57 changes: 57 additions & 0 deletions src/lints/struct_added_without_non_exhaustive.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
SemverQuery(
id: "struct_added_without_non_exhaustive",
human_readable_name: "pub struct added without #[non_exhaustive]",
description: "A new public struct was added without #[non_exhaustive], making it harder to evolve in the future.",
required_update: Minor,
Copy link
Owner

Choose a reason for hiding this comment

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

Since we don't have any prior "API evolution" advisory lints like this one (this is the first!), we haven't thought through what required_update should be here.

Right now, the only allowed options are Major and Minor. But neither seems quite right. Selecting Minor means that this lint won't run on major or minor releases, so new API additions like this will only be linted in patch releases. This doesn't seem quite right =/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm I see Minor is not right I believe, should we put it on-hold till we figure it out

lint_level: Warn,
reference_link: Some("https://doc.rust-lang.org/reference/attributes/type_system.html#the-non_exhaustive-attribute"),
query: r#"
{
CrateDiff {
current {
item {
... on Struct {
visibility_limit @filter(op: "=", value: ["$public"]) @output
name @output
struct_type @output

# Check that it's not marked #[non_exhaustive]
attrs @filter(op: "not_contains", value: ["$non_exhaustive"])

importable_path {
path @output @tag
public_api @filter(op: "=", value: ["$true"])
}

# Verify all fields are public
field @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
visibility_limit @filter(op: "!=", value: ["$public"])
}

span_: span @optional {
filename @output
begin_line @output
}
}
}
}
baseline @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
item {
... on Struct {
importable_path {
path @filter(op: "=", value: ["%path"])
}
}
}
}
}
}"#,
arguments: {
"public": "public",
"zero": 0,
"true": true,
"non_exhaustive": "non_exhaustive",
Frank-III marked this conversation as resolved.
Show resolved Hide resolved
},
error_message: "A new public struct was added without #[non_exhaustive], which may make it difficult to evolve in the future",
per_result_error_template: Some("struct {{join \"::\" path}} in file {{span_filename}}:{{span_begin_line}}"),
)
Frank-III marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1208,6 +1208,7 @@ add_lints!(
struct_repr_transparent_removed,
struct_with_no_pub_fields_changed_type,
struct_with_pub_fields_changed_type,
struct_added_without_non_exhaustive,
trait_added_supertrait,
trait_allows_fewer_const_generic_params,
trait_allows_fewer_generic_type_params,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "struct_added_without_non_exhaustive"
version = "0.1.0"
edition = "2021"

[dependencies]
50 changes: 50 additions & 0 deletions test_crates/struct_added_without_non_exhaustive/new/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Existing structs shouldn't trigger the lint
pub struct ExistingStruct {
pub field: i32,
}

// Should NOT trigger the lint - private struct
struct PrivateStruct {
pub field: i32,
}

// Should trigger the lint - new public struct with all pub fields
pub struct NewAllPubFields {
pub field1: i32,
pub field2: String,
}

// Should NOT trigger the lint - has private field
pub struct NewWithPrivateField {
pub field1: i32,
field2: String, // private field
}

// Should NOT trigger the lint - marked non_exhaustive
#[non_exhaustive]
pub struct NewWithNonExhaustive {
pub field1: i32,
pub field2: String,
}

// Should NOT trigger the lint - not public
struct NewPrivateStruct {
pub field1: i32,
pub field2: String,
}

// Should trigger the lint - unit struct
pub struct NewUnitStruct;

// Should NOT trigger the lint - private in public module
pub mod public_mod {
struct PrivateInPublicMod {
pub field: i32,
}
}

// Should trigger the lint - tuple struct with all pub fields
pub struct NewTupleStruct(pub i32, pub String);

// Should NOT trigger the lint - tuple struct with private field
pub struct NewTupleWithPrivate(pub i32, String);
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "struct_added_without_non_exhaustive"
version = "0.1.0"
edition = "2021"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Existing structs shouldn't trigger the lint
pub struct ExistingStruct {
pub field: i32,
}

struct PrivateStruct {
pub field: i32,
}
130 changes: 130 additions & 0 deletions test_outputs/query_execution/struct_added_without_non_exhaustive.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
---
source: src/query.rs
expression: "&query_execution_results"
---
{
"./test_crates/inherent_method_must_use_added/": [
{
"name": String("EnumToStructWithMustUseMethods"),
"path": List([
String("inherent_method_must_use_added"),
String("item_type_changed_inherent_method_must_use_added"),
String("EnumToStructWithMustUseMethods"),
]),
"span_begin_line": Uint64(8),
"span_filename": String("src/item_type_changed_inherent_method_must_use_added.rs"),
"struct_type": String("plain"),
"visibility_limit": String("public"),
},
{
"name": String("UnionToStructWithMustUseMethods"),
"path": List([
String("inherent_method_must_use_added"),
String("item_type_changed_inherent_method_must_use_added"),
String("UnionToStructWithMustUseMethods"),
]),
"span_begin_line": Uint64(161),
"span_filename": String("src/item_type_changed_inherent_method_must_use_added.rs"),
"struct_type": String("plain"),
"visibility_limit": String("public"),
},
{
"name": String("NewStructWithMustUseMethods"),
"path": List([
String("inherent_method_must_use_added"),
String("struct_inherent_method_must_use_added"),
String("NewStructWithMustUseMethods"),
]),
"span_begin_line": Uint64(65),
"span_filename": String("src/struct_inherent_method_must_use_added.rs"),
"struct_type": String("plain"),
"visibility_limit": String("public"),
},
],
"./test_crates/method_moved_to_trait_must_use_added/": [
{
"name": String("NewStructWithTraitMustUseMethods"),
"path": List([
String("method_moved_to_trait_must_use_added"),
String("struct_method_moved_to_trait_must_use_added"),
String("NewStructWithTraitMustUseMethods"),
]),
"span_begin_line": Uint64(147),
"span_filename": String("src/struct_method_moved_to_trait_must_use_added.rs"),
"struct_type": String("plain"),
"visibility_limit": String("public"),
},
],
"./test_crates/nonbreaking_item_rename/": [
{
"name": String("NewName"),
"path": List([
String("nonbreaking_item_rename"),
String("NewName"),
]),
"span_begin_line": Uint64(2),
"span_filename": String("src/lib.rs"),
"struct_type": String("unit"),
"visibility_limit": String("public"),
},
],
"./test_crates/struct_added_without_non_exhaustive/": [
{
"name": String("NewAllPubFields"),
"path": List([
String("struct_added_without_non_exhaustive"),
String("NewAllPubFields"),
]),
"span_begin_line": Uint64(12),
"span_filename": String("src/lib.rs"),
"struct_type": String("plain"),
"visibility_limit": String("public"),
},
{
"name": String("NewWithNonExhaustive"),
"path": List([
String("struct_added_without_non_exhaustive"),
String("NewWithNonExhaustive"),
]),
"span_begin_line": Uint64(25),
"span_filename": String("src/lib.rs"),
"struct_type": String("plain"),
"visibility_limit": String("public"),
},
{
"name": String("NewUnitStruct"),
"path": List([
String("struct_added_without_non_exhaustive"),
String("NewUnitStruct"),
]),
"span_begin_line": Uint64(37),
"span_filename": String("src/lib.rs"),
"struct_type": String("unit"),
"visibility_limit": String("public"),
},
{
"name": String("NewTupleStruct"),
"path": List([
String("struct_added_without_non_exhaustive"),
String("NewTupleStruct"),
]),
"span_begin_line": Uint64(47),
"span_filename": String("src/lib.rs"),
"struct_type": String("tuple"),
"visibility_limit": String("public"),
},
],
"./test_crates/switch_away_from_reexport_as_underscore/": [
{
"name": String("Struct"),
"path": List([
String("switch_away_from_reexport_as_underscore"),
String("Struct"),
]),
"span_begin_line": Uint64(4),
"span_filename": String("src/lib.rs"),
"struct_type": String("plain"),
"visibility_limit": String("public"),
},
],
}
Loading