Skip to content

Commit 99c1cdd

Browse files
committed
feat: Emit error when both from_name and from_type are empty
1 parent 1487096 commit 99c1cdd

File tree

3 files changed

+25
-12
lines changed

3 files changed

+25
-12
lines changed

crates/stackable-versioned-macros/src/attrs/common/item.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -143,20 +143,20 @@ impl ItemAttributes {
143143

144144
let mut errors = Error::accumulator();
145145

146-
// Semantic validation
146+
// Common validation
147147
errors.handle(self.validate_action_combinations(item_ident, item_type));
148148
errors.handle(self.validate_action_order(item_ident, item_type));
149149
errors.handle(self.validate_item_name(item_ident, item_type));
150-
errors.handle(self.validate_changed_item_name(item_type));
151150
errors.handle(self.validate_item_attributes(item_attrs));
152151

152+
// Action specific validation
153+
errors.handle(self.validate_changed_action(item_ident, item_type));
154+
153155
// TODO (@Techassi): Add hint if a field or variant is added in the
154156
// first version that it might be clever to remove the 'added'
155157
// attribute.
156158

157-
errors.finish()?;
158-
159-
Ok(())
159+
errors.finish()
160160
}
161161

162162
/// This associated function is called by the top-level validation function
@@ -293,22 +293,35 @@ impl ItemAttributes {
293293
}
294294
}
295295
}
296+
296297
Ok(())
297298
}
298299

299300
/// This associated function is called by the top-level validation function
300301
/// and validates that parameters provided to the `changed` actions are
301302
/// valid.
302-
fn validate_changed_item_name(&self, item_type: &ItemType) -> Result<(), Error> {
303+
fn validate_changed_action(
304+
&self,
305+
item_ident: &Ident,
306+
item_type: &ItemType,
307+
) -> Result<(), Error> {
303308
let prefix = match item_type {
304309
ItemType::Field => DEPRECATED_FIELD_PREFIX,
305310
ItemType::Variant => DEPRECATED_VARIANT_PREFIX,
306311
};
307312

308313
let mut errors = Error::accumulator();
309314

310-
// This ensures that `from_name` doesn't include the deprecation prefix.
311315
for change in &self.changes {
316+
// Ensure that from_name and from_type are not empty at the same
317+
// time.
318+
if change.from_name.is_none() && change.from_type.is_none() {
319+
errors.push(Error::custom(format!(
320+
"{item_type} was marked as `changed`, but both `from_name` and `from_type` are unset"
321+
)).with_span(item_ident));
322+
}
323+
324+
// Ensure that `from_name` doesn't include the deprecation prefix.
312325
if let Some(from_name) = change.from_name.as_ref() {
313326
if from_name.starts_with(prefix) {
314327
errors.push(

crates/stackable-versioned-macros/tests/default/fail/changed.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ fn main() {
99
struct Foo {
1010
#[versioned(
1111
changed(since = "v1beta1", from_name = "deprecated_bar"),
12-
changed(since = "v1", from_name = "deprecated_baz")
12+
changed(since = "v1")
1313
)]
1414
bar: usize,
1515
}

crates/stackable-versioned-macros/tests/default/fail/changed.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ error: the previous field name must not start with the deprecation prefix
44
11 | changed(since = "v1beta1", from_name = "deprecated_bar"),
55
| ^^^^^^^^^^^^^^^^
66

7-
error: the previous field name must not start with the deprecation prefix
8-
--> tests/default/fail/changed.rs:12:47
7+
error: field was marked as `changed`, but both `from_name` and `from_type` are unset
8+
--> tests/default/fail/changed.rs:14:9
99
|
10-
12 | changed(since = "v1", from_name = "deprecated_baz")
11-
| ^^^^^^^^^^^^^^^^
10+
14 | bar: usize,
11+
| ^^^

0 commit comments

Comments
 (0)