Skip to content

Commit 3406358

Browse files
authored
Turn struct field privacy warnings into errors (#5569)
## Description Closes #5520. As agreed with @FuelLabs/tooling, one LSP test fixture is changed to temporarily fetch its `std` dependency from the local repository. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
1 parent adfc551 commit 3406358

File tree

14 files changed

+108
-541
lines changed

14 files changed

+108
-541
lines changed

sway-core/src/language/ty/declaration/storage.rs

+7-20
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use std::hash::{Hash, Hasher};
33
use sway_error::{
44
error::{CompileError, StructFieldUsageContext},
55
handler::{ErrorEmitted, Handler},
6-
warning::{CompileWarning, Warning},
76
};
87
use sway_types::{state::StateIndex, Ident, Named, Span, Spanned};
98

@@ -135,25 +134,13 @@ impl TyStorageDecl {
135134
match struct_decl.find_field(field) {
136135
Some(struct_field) => {
137136
if is_public_struct_access && struct_field.is_private() {
138-
// TODO: Uncomment this code and delete the one with warnings once struct field privacy becomes a hard error.
139-
// https://github.com/FuelLabs/sway/issues/5520
140-
// return Err(handler.emit_err(CompileError::StructFieldIsPrivate {
141-
// field_name: field.into(),
142-
// struct_name: struct_decl.call_path.suffix.clone(),
143-
// field_decl_span: struct_field.name.span(),
144-
// struct_can_be_changed,
145-
// usage_context: StructFieldUsageContext::StorageAccess,
146-
// }));
147-
handler.emit_warn(CompileWarning {
148-
span: field.span(),
149-
warning_content: Warning::StructFieldIsPrivate {
150-
field_name: field.into(),
151-
struct_name: struct_decl.call_path.suffix.clone(),
152-
field_decl_span: struct_field.name.span(),
153-
struct_can_be_changed,
154-
usage_context: StructFieldUsageContext::StorageAccess,
155-
},
156-
});
137+
return Err(handler.emit_err(CompileError::StructFieldIsPrivate {
138+
field_name: field.into(),
139+
struct_name: struct_decl.call_path.suffix.clone(),
140+
field_decl_span: struct_field.name.span(),
141+
struct_can_be_changed,
142+
usage_context: StructFieldUsageContext::StorageAccess,
143+
}));
157144
}
158145

159146
// Everything is fine. Push the storage access descriptor and move to the next field.

sway-core/src/semantic_analysis/ast_node/expression/match_expression/typed/typed_scrutinee.rs

+42-90
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use itertools::Itertools;
22
use sway_error::{
33
error::{CompileError, StructFieldUsageContext},
44
handler::{ErrorEmitted, Handler},
5-
warning::{CompileWarning, Warning},
65
};
76
use sway_types::{BaseIdent, Ident, Span, Spanned};
87

@@ -308,25 +307,13 @@ fn type_check_struct(
308307
.expect("The struct field with the given field name must exist.");
309308

310309
if struct_field.is_private() {
311-
// TODO: Uncomment this code and delete the one with warnings once struct field privacy becomes a hard error.
312-
// https://github.com/FuelLabs/sway/issues/5520
313-
// handler.emit_err(CompileError::StructFieldIsPrivate {
314-
// field_name: field_name.into(),
315-
// struct_name: struct_decl.call_path.suffix.clone(),
316-
// field_decl_span: struct_field.name.span(),
317-
// struct_can_be_changed,
318-
// usage_context: StructFieldUsageContext::PatternMatching { has_rest_pattern },
319-
// });
320-
handler.emit_warn(CompileWarning {
321-
span: field_name.span(),
322-
warning_content: Warning::StructFieldIsPrivate {
323-
field_name: field_name.into(),
324-
struct_name: struct_decl.call_path.suffix.clone(),
325-
field_decl_span: struct_field.name.span(),
326-
struct_can_be_changed,
327-
usage_context: StructFieldUsageContext::PatternMatching {
328-
has_rest_pattern,
329-
},
310+
handler.emit_err(CompileError::StructFieldIsPrivate {
311+
field_name: field_name.into(),
312+
struct_name: struct_decl.call_path.suffix.clone(),
313+
field_decl_span: struct_field.name.span(),
314+
struct_can_be_changed,
315+
usage_context: StructFieldUsageContext::PatternMatching {
316+
has_rest_pattern,
330317
},
331318
});
332319
}
@@ -377,83 +364,48 @@ fn type_check_struct(
377364
.collect_vec()
378365
};
379366

380-
// TODO: Uncomment this code and delete the one with warnings once struct field privacy becomes a hard error.
381-
// https://github.com/FuelLabs/sway/issues/5520
382-
// handler.emit_err(
383-
// match (is_public_struct_access, all_public_fields_are_matched, only_public_fields_are_matched) {
384-
// // Public access. Only all public fields are matched. All missing fields are private.
385-
// // -> Emit error for the mandatory ignore `..`.
386-
// (true, true, true) => CompileError::MatchStructPatternMustIgnorePrivateFields {
387-
// private_fields: missing_fields(false),
388-
// struct_name: struct_decl.call_path.suffix.clone(),
389-
// struct_decl_span: struct_decl.span(),
390-
// all_fields_are_private: struct_decl.has_only_private_fields(),
391-
// span: span.clone(),
392-
// },
393-
394-
// // Public access. All public fields are matched. Some private fields are matched.
395-
// // -> Do not emit error here because it is already covered when reporting private field.
396-
// (true, true, false) => unreachable!("The above if condition eliminates this case."),
397-
398-
// // Public access. Some or non of the public fields are matched. Some or none of the private fields are matched.
399-
// // -> Emit error listing only missing public fields. Recommendation for mandatory use of `..` is already given
400-
// // when reporting the inaccessible private field.
401-
// // or
402-
// // In struct decl module access. We do not distinguish between private and public fields here.
403-
// // -> Emit error listing all missing fields.
404-
// (true, false, _) | (false, _, _) => CompileError::MatchStructPatternMissingFields {
405-
// missing_fields: missing_fields(is_public_struct_access),
406-
// missing_fields_are_public: is_public_struct_access,
407-
// struct_name: struct_decl.call_path.suffix.clone(),
408-
// struct_decl_span: struct_decl.span(),
409-
// total_number_of_fields: struct_decl.fields.len(),
410-
// span: span.clone(),
411-
// },
412-
// });
413-
414-
match (
415-
is_public_struct_access,
416-
all_public_fields_are_matched,
417-
only_public_fields_are_matched,
418-
) {
419-
// Public access. Only all public fields are matched. All missing fields are private.
420-
// -> Emit error for the mandatory ignore `..`.
421-
(true, true, true) => {
422-
handler.emit_warn(CompileWarning {
423-
span: span.clone(),
424-
warning_content: Warning::MatchStructPatternMustIgnorePrivateFields {
367+
handler.emit_err(
368+
match (
369+
is_public_struct_access,
370+
all_public_fields_are_matched,
371+
only_public_fields_are_matched,
372+
) {
373+
// Public access. Only all public fields are matched. All missing fields are private.
374+
// -> Emit error for the mandatory ignore `..`.
375+
(true, true, true) => {
376+
CompileError::MatchStructPatternMustIgnorePrivateFields {
425377
private_fields: missing_fields(false),
426378
struct_name: struct_decl.call_path.suffix.clone(),
427379
struct_decl_span: struct_decl.span(),
428380
all_fields_are_private: struct_decl.has_only_private_fields(),
429381
span: span.clone(),
430-
},
431-
});
432-
}
382+
}
383+
}
433384

434-
// Public access. All public fields are matched. Some private fields are matched.
435-
// -> Do not emit error here because it is already covered when reporting private field.
436-
(true, true, false) => {
437-
unreachable!("The above if condition eliminates this case.")
438-
}
385+
// Public access. All public fields are matched. Some private fields are matched.
386+
// -> Do not emit error here because it is already covered when reporting private field.
387+
(true, true, false) => {
388+
unreachable!("The above if condition eliminates this case.")
389+
}
439390

440-
// Public access. Some or non of the public fields are matched. Some or none of the private fields are matched.
441-
// -> Emit error listing only missing public fields. Recommendation for mandatory use of `..` is already given
442-
// when reporting the inaccessible private field.
443-
// or
444-
// In struct decl module access. We do not distinguish between private and public fields here.
445-
// -> Emit error listing all missing fields.
446-
(true, false, _) | (false, _, _) => {
447-
handler.emit_err(CompileError::MatchStructPatternMissingFields {
448-
missing_fields: missing_fields(is_public_struct_access),
449-
missing_fields_are_public: is_public_struct_access,
450-
struct_name: struct_decl.call_path.suffix.clone(),
451-
struct_decl_span: struct_decl.span(),
452-
total_number_of_fields: struct_decl.fields.len(),
453-
span: span.clone(),
454-
});
455-
}
456-
};
391+
// Public access. Some or non of the public fields are matched. Some or none of the private fields are matched.
392+
// -> Emit error listing only missing public fields. Recommendation for mandatory use of `..` is already given
393+
// when reporting the inaccessible private field.
394+
// or
395+
// In struct decl module access. We do not distinguish between private and public fields here.
396+
// -> Emit error listing all missing fields.
397+
(true, false, _) | (false, _, _) => {
398+
CompileError::MatchStructPatternMissingFields {
399+
missing_fields: missing_fields(is_public_struct_access),
400+
missing_fields_are_public: is_public_struct_access,
401+
struct_name: struct_decl.call_path.suffix.clone(),
402+
struct_decl_span: struct_decl.span(),
403+
total_number_of_fields: struct_decl.fields.len(),
404+
span: span.clone(),
405+
}
406+
}
407+
},
408+
);
457409
}
458410
}
459411

sway-core/src/semantic_analysis/ast_node/expression/typed_expression/struct_field_access.rs

+7-20
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use sway_error::{
22
error::{CompileError, StructFieldUsageContext},
33
handler::{ErrorEmitted, Handler},
4-
warning::{CompileWarning, Warning},
54
};
65
use sway_types::{Ident, Span, Spanned};
76

@@ -69,25 +68,13 @@ pub(crate) fn instantiate_struct_field_access(
6968
let field = match decl.find_field(&field_to_access) {
7069
Some(field) => {
7170
if is_public_struct_access && field.is_private() {
72-
// TODO: Uncomment this code and delete the one with warnings once struct field privacy becomes a hard error.
73-
// https://github.com/FuelLabs/sway/issues/5520
74-
// return Err(handler.emit_err(CompileError::StructFieldIsPrivate {
75-
// field_name: (&field_to_access).into(),
76-
// struct_name: decl.call_path.suffix.clone(),
77-
// field_decl_span: field.name.span(),
78-
// struct_can_be_changed,
79-
// usage_context: StructFieldUsageContext::StructFieldAccess,
80-
// }));
81-
handler.emit_warn(CompileWarning {
82-
span: field_to_access.span(),
83-
warning_content: Warning::StructFieldIsPrivate {
84-
field_name: (&field_to_access).into(),
85-
struct_name: decl.call_path.suffix.clone(),
86-
field_decl_span: field.name.span(),
87-
struct_can_be_changed,
88-
usage_context: StructFieldUsageContext::StructFieldAccess,
89-
},
90-
});
71+
return Err(handler.emit_err(CompileError::StructFieldIsPrivate {
72+
field_name: (&field_to_access).into(),
73+
struct_name: decl.call_path.suffix.clone(),
74+
field_decl_span: field.name.span(),
75+
struct_can_be_changed,
76+
usage_context: StructFieldUsageContext::StructFieldAccess,
77+
}));
9178
}
9279

9380
field.clone()

sway-core/src/semantic_analysis/ast_node/expression/typed_expression/struct_instantiation.rs

+25-57
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use itertools::Itertools;
22
use sway_error::{
33
error::{CompileError, StructFieldUsageContext},
44
handler::{ErrorEmitted, Handler},
5-
warning::{CompileWarning, Warning},
65
};
76
use sway_types::{Ident, Span, Spanned};
87

@@ -135,34 +134,19 @@ pub(crate) fn struct_instantiation(
135134
ctx.storage_declaration(),
136135
);
137136

138-
// TODO: Uncomment this code and delete the one with warnings once struct field privacy becomes a hard error.
139-
// https://github.com/FuelLabs/sway/issues/5520
140-
// handler.emit_err(CompileError::StructCannotBeInstantiated {
141-
// struct_name: struct_name.clone(),
142-
// span: inner_span.clone(),
143-
// struct_decl_span: struct_decl.span.clone(),
144-
// private_fields: struct_fields.iter().filter(|field| field.is_private()).map(|field| field.name.clone()).collect(),
145-
// constructors,
146-
// all_fields_are_private,
147-
// is_in_storage_declaration: ctx.storage_declaration(),
148-
// struct_can_be_changed,
149-
// });
150-
handler.emit_warn(CompileWarning {
137+
handler.emit_err(CompileError::StructCannotBeInstantiated {
138+
struct_name: struct_name.clone(),
151139
span: inner_span.clone(),
152-
warning_content: Warning::StructCannotBeInstantiated {
153-
struct_name: struct_name.clone(),
154-
span: inner_span.clone(),
155-
struct_decl_span: struct_decl.span.clone(),
156-
private_fields: struct_fields
157-
.iter()
158-
.filter(|field| field.is_private())
159-
.map(|field| field.name.clone())
160-
.collect(),
161-
constructors,
162-
all_fields_are_private,
163-
is_in_storage_declaration: ctx.storage_declaration(),
164-
struct_can_be_changed,
165-
},
140+
struct_decl_span: struct_decl.span.clone(),
141+
private_fields: struct_fields
142+
.iter()
143+
.filter(|field| field.is_private())
144+
.map(|field| field.name.clone())
145+
.collect(),
146+
constructors,
147+
all_fields_are_private,
148+
is_in_storage_declaration: ctx.storage_declaration(),
149+
struct_can_be_changed,
166150
});
167151
}
168152

@@ -212,35 +196,19 @@ pub(crate) fn struct_instantiation(
212196
for field in fields {
213197
if let Some(ty_field) = struct_fields.iter().find(|x| x.name == field.name) {
214198
if ty_field.is_private() {
215-
// TODO: Uncomment this code and delete the one with warnings once struct field privacy becomes a hard error.
216-
// https://github.com/FuelLabs/sway/issues/5520
217-
// handler.emit_err(CompileError::StructFieldIsPrivate {
218-
// field_name: (&field.name).into(),
219-
// struct_name: struct_name.clone(),
220-
// field_decl_span: ty_field.name.span(),
221-
// struct_can_be_changed,
222-
// usage_context: if ctx.storage_declaration() {
223-
// StructFieldUsageContext::StorageDeclaration { struct_can_be_instantiated }
224-
// } else {
225-
// StructFieldUsageContext::StructInstantiation { struct_can_be_instantiated }
226-
// }
227-
// });
228-
handler.emit_warn(CompileWarning {
229-
span: field.name.span(),
230-
warning_content: Warning::StructFieldIsPrivate {
231-
field_name: (&field.name).into(),
232-
struct_name: struct_name.clone(),
233-
field_decl_span: ty_field.name.span(),
234-
struct_can_be_changed,
235-
usage_context: if ctx.storage_declaration() {
236-
StructFieldUsageContext::StorageDeclaration {
237-
struct_can_be_instantiated,
238-
}
239-
} else {
240-
StructFieldUsageContext::StructInstantiation {
241-
struct_can_be_instantiated,
242-
}
243-
},
199+
handler.emit_err(CompileError::StructFieldIsPrivate {
200+
field_name: (&field.name).into(),
201+
struct_name: struct_name.clone(),
202+
field_decl_span: ty_field.name.span(),
203+
struct_can_be_changed,
204+
usage_context: if ctx.storage_declaration() {
205+
StructFieldUsageContext::StorageDeclaration {
206+
struct_can_be_instantiated,
207+
}
208+
} else {
209+
StructFieldUsageContext::StructInstantiation {
210+
struct_can_be_instantiated,
211+
}
244212
},
245213
});
246214
}

0 commit comments

Comments
 (0)