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

private struct fields #620

Merged
merged 1 commit into from
Jan 6, 2022
Merged
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
5 changes: 1 addition & 4 deletions crates/analyzer/src/db/queries/structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,13 @@ pub fn struct_field_type(
let mut scope = ItemScope::new(db, field_data.parent.module(db));

let ast::Field {
is_pub,
is_pub: _,
is_const,
name: _,
typ,
value,
} = &field_data.ast.kind;

if *is_pub {
scope.not_yet_implemented("struct `pub` fields", field_data.ast.span);
}
if *is_const {
scope.not_yet_implemented("struct `const` fields", field_data.ast.span);
}
Expand Down
29 changes: 29 additions & 0 deletions crates/analyzer/src/namespace/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ impl Item {
}
}

pub fn is_struct(&self, val: &StructId) -> bool {
matches!(self, Item::Type(TypeDef::Struct(current)) if current == val)
}

pub fn item_kind_display_name(&self) -> &'static str {
match self {
Item::Type(_) => "type",
Expand Down Expand Up @@ -1312,6 +1316,10 @@ impl StructId {
db.struct_type(*self)
}

pub fn has_private_field(&self, db: &dyn AnalyzerDb) -> bool {
self.private_fields(db).iter().count() > 0
}

pub fn field(&self, db: &dyn AnalyzerDb, name: &str) -> Option<StructFieldId> {
self.fields(db).get(name).copied()
}
Expand All @@ -1338,6 +1346,20 @@ impl StructId {
pub fn parent(&self, db: &dyn AnalyzerDb) -> Item {
Item::Module(self.data(db).module)
}
pub fn private_fields(&self, db: &dyn AnalyzerDb) -> Rc<IndexMap<SmolStr, StructFieldId>> {
Rc::new(
self.fields(db)
.iter()
.filter_map(|(name, field)| {
if field.is_public(db) {
None
} else {
Some((name.clone(), *field))
}
})
.collect(),
)
}
pub fn dependency_graph(&self, db: &dyn AnalyzerDb) -> Rc<DepGraph> {
db.struct_dependency_graph(*self).0
}
Expand Down Expand Up @@ -1366,12 +1388,19 @@ impl StructFieldId {
pub fn name(&self, db: &dyn AnalyzerDb) -> SmolStr {
self.data(db).ast.name().into()
}
pub fn span(&self, db: &dyn AnalyzerDb) -> Span {
self.data(db).ast.span
}
pub fn data(&self, db: &dyn AnalyzerDb) -> Rc<StructField> {
db.lookup_intern_struct_field(*self)
}
pub fn typ(&self, db: &dyn AnalyzerDb) -> Result<types::FixedSize, TypeError> {
db.struct_field_type(*self).value
}
pub fn is_public(&self, db: &dyn AnalyzerDb) -> bool {
self.data(db).ast.kind.is_pub
}

pub fn sink_diagnostics(&self, db: &dyn AnalyzerDb, sink: &mut impl DiagnosticSink) {
db.struct_field_type(*self).sink_diagnostics(sink)
}
Expand Down
6 changes: 6 additions & 0 deletions crates/analyzer/src/namespace/scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use crate::context::{AnalyzerContext, CallType, ExpressionAttributes, FunctionBody, NamedThing};
use crate::errors::{AlreadyDefined, TypeError};
use crate::namespace::items::Item;
use crate::namespace::items::{Class, EventId, FunctionId, ModuleId};
use crate::namespace::types::FixedSize;
use crate::AnalyzerDb;
Expand Down Expand Up @@ -339,6 +340,11 @@ impl<'a, 'b> BlockScope<'a, 'b> {
pub fn inherits_type(&self, typ: BlockScopeType) -> bool {
self.typ == typ || self.parent.map_or(false, |scope| scope.inherits_type(typ))
}

/// Return the root item of the block scope
pub fn root_item(&self) -> Item {
self.root.function.parent(self.db())
}
}

/// temporary helper until `BTreeMap::try_insert` is stabilized
Expand Down
37 changes: 35 additions & 2 deletions crates/analyzer/src/traversal/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,9 +619,20 @@ fn expr_attribute(
// If the value is a struct, we return the type of the struct field. The location stays the
// same and can be memory or storage.
Type::Struct(struct_) => {
if let Some(field) = struct_.id.field(scope.db(), &field.kind) {
if let Some(struct_field) = struct_.id.field(scope.db(), &field.kind) {
if !scope.root_item().is_struct(&struct_.id) && !struct_field.is_public(scope.db())
{
scope.fancy_error(
&format!(
"Can not access private field `{}` on struct `{}`",
&field.kind, struct_.name
),
vec![Label::primary(field.span, "private field")],
vec![],
);
}
Ok(ExpressionAttributes::new(
field.typ(scope.db())?.into(),
struct_field.typ(scope.db())?.into(),
attrs.location,
))
} else {
Expand Down Expand Up @@ -1335,6 +1346,28 @@ fn expr_call_struct_constructor(
args: &Node<Vec<Node<fe::CallArg>>>,
) -> Result<(ExpressionAttributes, CallType), FatalError> {
let db = scope.root.db;

if struct_.id.has_private_field(db) && !scope.root_item().is_struct(&struct_.id) {
scope.fancy_error(
&format!(
"Can not call private constructor of struct `{}` ",
struct_.name
),
struct_
.id
.private_fields(db)
.iter()
.map(|(name, field)| {
Label::primary(field.span(db), format!("Field `{}` is private", name))
})
.collect(),
vec![format!(
"Suggestion: implement a method `new(...)` on struct `{}` to call the constructor and return the struct",
struct_.name
)],
);
}

let fields = struct_
.id
.fields(db)
Expand Down
2 changes: 2 additions & 0 deletions crates/analyzer/tests/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ test_file! { needs_mem_copy }
test_file! { not_callable }
test_file! { not_in_scope }
test_file! { not_in_scope_2 }
test_file! { private_struct_field }
test_file! { return_addition_with_mixed_types }
test_file! { return_call_to_fn_with_param_type_mismatch }
test_file! { return_call_to_fn_without_return }
Expand All @@ -290,6 +291,7 @@ test_file! { return_type_not_fixedsize }
test_file! { undefined_type_param }

test_file! { strict_boolean_if_else }
test_file! { struct_private_constructor }
test_file! { struct_call_bad_args }
test_file! { struct_call_without_kw_args }
test_file! { non_pub_init }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ expression: "build_snapshot(&files, module_id, &db)"
note:
┌─ stress/abi_encoding_stress.fe:2:5
2 │ my_num: u256
│ ^^^^^^^^^^^^ u256
3 │ my_num2: u8
│ ^^^^^^^^^^^ u8
4 │ my_bool: bool
│ ^^^^^^^^^^^^^ bool
5 │ my_addr: address
│ ^^^^^^^^^^^^^^^^ address
2 │ pub my_num: u256
│ ^^^^^^^^^^^^^^^^ u256
3 │ pub my_num2: u8
│ ^^^^^^^^^^^^^^^ u8
4 │ pub my_bool: bool
│ ^^^^^^^^^^^^^^^^^ bool
5 │ pub my_addr: address
│ ^^^^^^^^^^^^^^^^^^^^ address

note:
┌─ stress/abi_encoding_stress.fe:8:5
Expand Down
6 changes: 3 additions & 3 deletions crates/analyzer/tests/snapshots/analysis__associated_fns.snap
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: crates/analyzer/tests/analysis.rs
expression: "build_snapshot(\"features/associated_fns.fe\", &src, module, &db)"
expression: "build_snapshot(&files, module_id, &db)"

---
note:
Expand Down Expand Up @@ -50,8 +50,8 @@ note:
note:
┌─ features/associated_fns.fe:7:3
7 │ x: u256
│ ^^^^^^^ u256
7 │ pub x: u256
│ ^^^^^^^^^^^ u256

note:
┌─ features/associated_fns.fe:8:3
Expand Down
Loading