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.

Sign up for GitHub

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

feat: enhance schema attr resolve for the double star expr #1743

Merged
merged 1 commit into from
Nov 6, 2024
Merged

feat: enhance schema attr resolve for the double star expr #1743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
160 changes: 139 additions & 21 deletions kclvm/sema/src/resolver/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ impl<'ctx> Resolver<'_> {
};
self.switch_config_expr_context_by_names(&names)
}
// Double star expression
None => SwitchConfigContextState::KeepConfigUnchanged as usize,
}
}
Expand Down Expand Up @@ -310,12 +311,60 @@ impl<'ctx> Resolver<'_> {
) {
if !name.is_empty() {
if let Some(Some(obj)) = self.ctx.config_expr_context.last() {
let obj = obj.clone();
self.must_check_config_attr(name, &key.get_span_pos(), &obj.ty);
let ty = obj.ty.clone();
self.must_check_config_attr(name, &ty, &key.get_span_pos(), None);
}
}
}

fn check_config_value_recursively(&mut self, value_ty: &TypeRef, value_span: &Range) {
match &value_ty.kind {
TypeKind::Dict(DictType {
key_ty: _,
val_ty: _,
attrs,
}) => {
for (key, attr) in attrs {
self.check_attr_recursively(&key, &attr.ty, &attr.range, value_span);
}
}
TypeKind::Schema(schema_ty) => {
for (key, attr) in &schema_ty.attrs {
self.check_attr_recursively(&key, &attr.ty, &attr.range, value_span);
}
}
_ => {}
}
}

fn check_attr_recursively(
&mut self,
key: &str,
attr_ty: &TypeRef,
attr_span: &Range,
value_span: &Range,
) {
if !key.is_empty() {
if let Some(Some(obj)) = self.ctx.config_expr_context.last() {
let ty = obj.ty.clone();
self.must_check_config_attr(key, &ty, value_span, Some(attr_span));
}
let stack_depth = self.switch_config_expr_context_by_name(key);
if let Some(Some(obj)) = self.ctx.config_expr_context.last() {
let ty = obj.ty.clone();
self.attr_must_assignable_to(
attr_ty.clone(),
ty,
value_span.clone(),
Some(obj.get_span_pos()),
Some(attr_span.clone()),
);
}
self.check_config_value_recursively(attr_ty, value_span);
self.clear_config_expr_context(stack_depth, false);
}
}

/// Check the key-value in 'ConfigExpr', such as check_defined and check_type
///
/// Notes:
Expand Down Expand Up @@ -383,6 +432,11 @@ impl<'ctx> Resolver<'_> {
self.clear_config_expr_context(stack_depth, false);
return return_ty;
}
} else {
// For double star expression, we can recursively check nested configuration properties at compile time.
let value_ty = self.expr(value);
self.check_config_value_recursively(&value_ty, &value.get_span_pos());
return Some(value_ty);
}
None
}
Expand All @@ -402,9 +456,15 @@ impl<'ctx> Resolver<'_> {
}

/// Check config attr has been defined.
pub(crate) fn must_check_config_attr(&mut self, attr: &str, range: &Range, ty: &TypeRef) {
pub(crate) fn must_check_config_attr(
&mut self,
attr: &str,
ty: &TypeRef,
range: &Range,
attr_range: Option<&Range>,
) {
if let TypeKind::Schema(schema_ty) = &ty.kind {
self.check_config_attr(attr, range, schema_ty)
self.check_config_attr(attr, schema_ty, range, attr_range);
} else if let TypeKind::Union(types) = &ty.kind {
let mut schema_names = vec![];
let mut total_suggs = vec![];
Expand All @@ -429,8 +489,10 @@ impl<'ctx> Resolver<'_> {
}
}
if !schema_names.is_empty() {
self.handler.add_compile_error_with_suggestions(
&format!(
let mut msgs = vec![Message {
range: range.clone(),
style: Style::LineAndColumn,
message: format!(
"Cannot add member '{}' to '{}'{}",
attr,
if schema_names.len() > 1 {
Expand All @@ -444,15 +506,31 @@ impl<'ctx> Resolver<'_> {
format!(", did you mean '{:?}'?", total_suggs)
},
),
range.clone(),
Some(total_suggs),
);
note: None,
suggested_replacement: Some(total_suggs),
}];
if let Some(attr_range) = attr_range {
msgs.push(Message {
range: attr_range.clone(),
style: Style::LineAndColumn,
message: "config attribute is defined here".to_string(),
note: None,
suggested_replacement: None,
});
}
self.handler.add_error(ErrorKind::CompileError, &msgs);
}
}
}

/// Check config attr has been defined.
pub(crate) fn check_config_attr(&mut self, attr: &str, range: &Range, schema_ty: &SchemaType) {
pub(crate) fn check_config_attr(
&mut self,
attr: &str,
schema_ty: &SchemaType,
range: &Range,
attr_range: Option<&Range>,
) {
let runtime_type = kclvm_runtime::schema_runtime_type(&schema_ty.name, &schema_ty.pkgpath);
match self.ctx.schema_mapping.get(&runtime_type) {
Some(schema_mapping_ty) => {
Expand All @@ -462,14 +540,26 @@ impl<'ctx> Resolver<'_> {
&& schema_ty_ref.index_signature.is_none()
{
let (suggs, msg) = self.get_config_attr_err_suggestion(attr, schema_ty);
self.handler.add_compile_error_with_suggestions(
&format!(
let mut msgs = vec![Message {
range: range.clone(),
style: Style::LineAndColumn,
message: format!(
"Cannot add member '{}' to schema '{}'{}",
attr, schema_ty_ref.name, msg,
),
range.clone(),
Some(suggs),
);
note: None,
suggested_replacement: Some(suggs),
}];
if let Some(attr_range) = attr_range {
msgs.push(Message {
range: attr_range.clone(),
style: Style::LineAndColumn,
message: "config attribute is defined here".to_string(),
note: None,
suggested_replacement: None,
});
}
self.handler.add_error(ErrorKind::CompileError, &msgs);
}
}
None => {
Expand All @@ -478,14 +568,26 @@ impl<'ctx> Resolver<'_> {
&& schema_ty.index_signature.is_none()
{
let (suggs, msg) = self.get_config_attr_err_suggestion(attr, schema_ty);
self.handler.add_compile_error_with_suggestions(
&format!(
let mut msgs = vec![Message {
range: range.clone(),
style: Style::LineAndColumn,
message: format!(
"Cannot add member '{}' to schema '{}'{}",
attr, schema_ty.name, msg,
),
range.clone(),
Some(suggs),
);
note: None,
suggested_replacement: Some(suggs),
}];
if let Some(attr_range) = attr_range {
msgs.push(Message {
range: attr_range.clone(),
style: Style::LineAndColumn,
message: "config attribute is defined here".to_string(),
note: None,
suggested_replacement: None,
});
}
self.handler.add_error(ErrorKind::CompileError, &msgs);
}
}
};
Expand Down Expand Up @@ -652,13 +754,29 @@ impl<'ctx> Resolver<'_> {
TypeKind::None | TypeKind::Any => {
val_types.push(val_ty.clone());
}
TypeKind::Dict(DictType { key_ty, val_ty, .. }) => {
TypeKind::Dict(DictType {
key_ty,
val_ty,
attrs: merged_attrs,
}) => {
key_types.push(key_ty.clone());
val_types.push(val_ty.clone());
for (key, value) in merged_attrs {
attrs.insert(key.to_string(), value.clone());
}
}
TypeKind::Schema(schema_ty) => {
key_types.push(schema_ty.key_ty());
val_types.push(schema_ty.val_ty());
for (key, attr) in &schema_ty.attrs {
attrs.insert(
key.to_string(),
Attr {
ty: attr.ty.clone(),
range: attr.range.clone(),
},
);
}
}
TypeKind::Union(types)
if self
Expand Down
21 changes: 21 additions & 0 deletions kclvm/sema/src/resolver/ty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,18 @@ impl<'ctx> Resolver<'_> {
expected_ty: TypeRef,
range: Range,
def_range: Option<Range>,
) {
self.attr_must_assignable_to(ty, expected_ty, range, def_range, None);
}

/// Attribute must assignable to the expected type.
pub fn attr_must_assignable_to(
&mut self,
ty: TypeRef,
expected_ty: TypeRef,
range: Range,
def_range: Option<Range>,
attr_range: Option<Range>,
) {
if !self.check_type(ty.clone(), expected_ty.clone(), &range) {
let mut msgs = vec![Message {
Expand Down Expand Up @@ -132,6 +144,15 @@ impl<'ctx> Resolver<'_> {
});
}
}
if let Some(attr_range) = attr_range {
msgs.push(Message {
range: attr_range.clone(),
style: Style::LineAndColumn,
message: "config attribute is defined here".to_string(),
note: None,
suggested_replacement: None,
});
}
self.handler.add_error(ErrorKind::TypeError, &msgs);
}
}
Expand Down
6 changes: 3 additions & 3 deletions kclvm/sema/src/resolver/var.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl<'ctx> Resolver<'_> {
for name in &names[1..] {
// Store and config attr check
if self.ctx.l_value {
self.must_check_config_attr(name, &range, &ty);
self.must_check_config_attr(name, &ty, &range, None);
}
ty = self.load_attr(ty, name, range.clone());
tys.push(ty.clone());
Expand Down Expand Up @@ -164,13 +164,13 @@ impl<'ctx> Resolver<'_> {
match path {
ast::MemberOrIndex::Member(member) => {
let attr = &member.node;
self.must_check_config_attr(attr, &range, &ty);
self.must_check_config_attr(attr, &ty, &range, None);
ty = self.load_attr(ty, attr, range.clone());
tys.push(ty.clone());
}
ast::MemberOrIndex::Index(index) => {
if let ast::Expr::StringLit(string_lit) = &index.node {
self.must_check_config_attr(&string_lit.value, &range, &ty);
self.must_check_config_attr(&string_lit.value, &ty, &range, None);
}
ty = self.subscript_index(ty, index, range.clone());
tys.push(ty.clone());
Expand Down
21 changes: 21 additions & 0 deletions test/grammar/schema/invalid/add_attribute_double_star_expr/main.k
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
innerConfig = {
nam = ""
config = {
nam = ""
}
}

config = {
**innerConfig
}

schema Config:
name: str
config: ConfigInner

schema ConfigInner:
name: str

c = Config {
**config
}
22 changes: 22 additions & 0 deletions test/grammar/schema/invalid/add_attribute_double_star_expr/stderr.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E2L23]: CompileError
--> ${CWD}/main.k:20:7
|
20 | **config
| ^ Cannot add member 'nam' to schema 'Config', did you mean '["name"]'?
|
--> ${CWD}/main.k:2:5
|
2 | nam = ""
| ^ config attribute is defined here
|
error[E2L23]: CompileError
--> ${CWD}/main.k:20:7
|
20 | **config
| ^ Cannot add member 'nam' to schema 'ConfigInner', did you mean '["name"]'?
|
--> ${CWD}/main.k:4:9
|
4 | nam = ""
| ^ config attribute is defined here
|
21 changes: 21 additions & 0 deletions test/grammar/types/var_type_annotation/type_fail_13/stderr.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,32 @@ error[E2G22]: TypeError
3 | A = "2"
| ^ expected int, got str(2)
|

--> ${CWD}/main.k:1:1
|
1 | config: {"A"|"B": int} = {
| ^ variable is defined here, its type is int, but got str(2)
|

error[E2G22]: TypeError
--> ${CWD}/main.k:2:5
|
2 | if True:
| ^ expected int, got str
|

--> ${CWD}/main.k:1:1
|
1 | config: {"A"|"B": int} = {
| ^ variable is defined here, its type is int, but got str
|

--> ${CWD}/main.k:3:9
|
3 | A = "2"
| ^ config attribute is defined here
|

error[E2G22]: TypeError
--> ${CWD}/main.k:1:1
|
Expand Down
Loading
Loading