Skip to content

Commit

Permalink
[parser][printer] Consistent comment attachment and printing for types (
Browse files Browse the repository at this point in the history 
#1165)
  • Loading branch information
@SamChou19815
SamChou19815 authored Dec 30, 2023
1 parent 41c8391 commit a25dbee
Show file tree
Hide file tree
Showing 15 changed files with 669 additions and 338 deletions.
70 changes: 51 additions & 19 deletions crates/samlang-ast/src/source.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,19 +146,34 @@ pub mod annotation {
}
}

#[derive(Clone, PartialEq, Eq)]
pub struct TypeArguments {
pub location: Location,
pub start_associated_comments: CommentReference,
pub ending_associated_comments: CommentReference,
pub arguments: Vec<T>,
}

#[derive(Clone, PartialEq, Eq)]
pub struct Id {
pub location: Location,
pub module_reference: ModuleReference,
pub id: super::Id,
pub type_arguments: Vec<T>,
pub type_arguments: Option<TypeArguments>,
}

#[derive(Clone, PartialEq, Eq)]
pub struct FunctionParameters {
pub location: Location,
pub ending_associated_comments: CommentReference,
pub parameters: Vec<T>,
}

#[derive(Clone, PartialEq, Eq)]
pub struct Function {
pub location: Location,
pub associated_comments: CommentReference,
pub argument_types: Vec<T>,
pub parameters: FunctionParameters,
pub return_type: Box<T>,
}

Expand Down Expand Up @@ -189,6 +204,21 @@ pub mod annotation {
}
}
}

#[derive(Clone, PartialEq, Eq)]
pub struct TypeParameter {
pub loc: Location,
pub name: super::Id,
pub bound: Option<Id>,
}

#[derive(Clone, PartialEq, Eq)]
pub struct TypeParameters {
pub location: Location,
pub start_associated_comments: CommentReference,
pub ending_associated_comments: CommentReference,
pub parameters: Vec<TypeParameter>,
}
}

#[derive(Clone, Copy, PartialEq, Eq)]
Expand Down Expand Up @@ -595,21 +625,14 @@ pub mod expr {
}
}

#[derive(Clone, PartialEq, Eq)]
pub struct TypeParameter {
pub loc: Location,
pub name: Id,
pub bound: Option<annotation::Id>,
}

#[derive(Clone, PartialEq, Eq)]
pub struct ClassMemberDeclaration {
pub loc: Location,
pub associated_comments: CommentReference,
pub is_public: bool,
pub is_method: bool,
pub name: Id,
pub type_parameters: Rc<Vec<TypeParameter>>,
pub type_parameters: Option<annotation::TypeParameters>,
pub type_: annotation::Function,
pub parameters: Rc<Vec<AnnotatedId<()>>>,
}
Expand All @@ -626,7 +649,7 @@ pub struct InterfaceDeclarationCommon<D, M> {
pub associated_comments: CommentReference,
pub private: bool,
pub name: Id,
pub type_parameters: Vec<TypeParameter>,
pub type_parameters: Option<annotation::TypeParameters>,
/** The node after colon, interpreted as extends in interfaces and implements in classes. */
pub extends_or_implements_nodes: Vec<annotation::Id>,
pub type_definition: D,
Expand Down Expand Up @@ -714,10 +737,10 @@ impl<T: Clone> Toplevel<T> {
}
}

pub fn type_parameters(&self) -> &Vec<TypeParameter> {
pub fn type_parameters(&self) -> Option<&annotation::TypeParameters> {
match self {
Toplevel::Interface(i) => &i.type_parameters,
Toplevel::Class(c) => &c.type_parameters,
Toplevel::Interface(i) => i.type_parameters.as_ref(),
Toplevel::Class(c) => c.type_parameters.as_ref(),
}
}

Expand Down Expand Up @@ -760,8 +783,8 @@ pub struct Module<T: Clone> {
}

pub mod test_builder {
use super::super::loc::Location;
use super::*;
use super::{super::loc::Location, annotation::TypeArguments};
use samlang_heap::{ModuleReference, PStr};

pub struct CustomizedAstBuilder {}
Expand Down Expand Up @@ -804,7 +827,7 @@ pub mod test_builder {
location: Location::dummy(),
module_reference: ModuleReference::ROOT,
id: Id::from(PStr::STR_TYPE),
type_arguments: vec![],
type_arguments: None,
})
}

Expand All @@ -817,7 +840,12 @@ pub mod test_builder {
location: Location::dummy(),
module_reference: ModuleReference::DUMMY,
id: Id::from(id),
type_arguments,
type_arguments: Some(TypeArguments {
location: Location::dummy(),
start_associated_comments: NO_COMMENT_REFERENCE,
ending_associated_comments: NO_COMMENT_REFERENCE,
arguments: type_arguments,
}),
}
}

Expand All @@ -835,13 +863,17 @@ pub mod test_builder {

pub fn fn_annot_unwrapped(
&self,
argument_types: Vec<annotation::T>,
parameters: Vec<annotation::T>,
return_type: annotation::T,
) -> annotation::Function {
annotation::Function {
location: Location::dummy(),
associated_comments: NO_COMMENT_REFERENCE,
argument_types,
parameters: annotation::FunctionParameters {
location: Location::dummy(),
ending_associated_comments: NO_COMMENT_REFERENCE,
parameters,
},
return_type: Box::new(return_type),
}
}
Expand Down
84 changes: 69 additions & 15 deletions crates/samlang-ast/src/source_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,12 +389,33 @@ mod tests {
annotation: Some(annotation::T::Fn(annotation::Function {
location: Location::dummy(),
associated_comments: NO_COMMENT_REFERENCE,
argument_types: vec![annotation::T::Id(annotation::Id {
parameters: annotation::FunctionParameters {
location: Location::dummy(),
module_reference: ModuleReference::DUMMY,
id: Id::from(heap.alloc_str_for_test("name")),
type_arguments: vec![],
})],
ending_associated_comments: NO_COMMENT_REFERENCE,
parameters: vec![
annotation::T::Id(annotation::Id {
location: Location::dummy(),
module_reference: ModuleReference::DUMMY,
id: Id::from(heap.alloc_str_for_test("name")),
type_arguments: None,
}),
annotation::T::Id(annotation::Id {
location: Location::dummy(),
module_reference: ModuleReference::DUMMY,
id: Id::from(heap.alloc_str_for_test("name")),
type_arguments: Some(annotation::TypeArguments {
location: Location::dummy(),
start_associated_comments: NO_COMMENT_REFERENCE,
ending_associated_comments: NO_COMMENT_REFERENCE,
arguments: vec![annotation::T::Primitive(
Location::dummy(),
NO_COMMENT_REFERENCE,
annotation::PrimitiveTypeKind::Any,
)],
}),
}),
],
},
return_type: Box::new(annotation::T::Primitive(
Location::dummy(),
NO_COMMENT_REFERENCE,
Expand All @@ -413,7 +434,7 @@ mod tests {
let mut heap = Heap::new();
assert_eq!(
"name",
TypeParameter {
annotation::TypeParameter {
loc: Location::dummy(),
name: Id::from(heap.alloc_str_for_test("name")),
bound: None
Expand Down Expand Up @@ -447,7 +468,7 @@ mod tests {
associated_comments: NO_COMMENT_REFERENCE,
private: false,
name: Id::from(PStr::LOWER_A),
type_parameters: vec![],
type_parameters: None,
extends_or_implements_nodes: vec![],
type_definition: (),
members: vec![ClassMemberDeclaration {
Expand All @@ -456,7 +477,12 @@ mod tests {
is_public: true,
is_method: true,
name: Id::from(PStr::LOWER_A),
type_parameters: Rc::new(vec![]),
type_parameters: Some(annotation::TypeParameters {
location: Location::dummy(),
start_associated_comments: NO_COMMENT_REFERENCE,
ending_associated_comments: NO_COMMENT_REFERENCE,
parameters: vec![]
}),
type_: builder.fn_annot_unwrapped(vec![], builder.int_annot()),
parameters: Rc::new(vec![AnnotatedId {
name: Id::from(PStr::LOWER_A),
Expand All @@ -467,7 +493,7 @@ mod tests {
}
.clone()
.type_parameters
.is_empty());
.is_none());
assert!(ModuleMembersImport {
loc: Location::dummy(),
imported_members: vec![],
Expand Down Expand Up @@ -505,15 +531,28 @@ mod tests {
type_: (),
annotation: builder.int_annot(),
}));
assert!(TypeParameter { loc: Location::dummy(), name: Id::from(PStr::LOWER_A), bound: None }
.eq(&TypeParameter { loc: Location::dummy(), name: Id::from(PStr::LOWER_A), bound: None }));
assert!(annotation::TypeParameter {
loc: Location::dummy(),
name: Id::from(PStr::LOWER_A),
bound: None
}
.eq(&annotation::TypeParameter {
loc: Location::dummy(),
name: Id::from(PStr::LOWER_A),
bound: None
}));

let class = Toplevel::Class(InterfaceDeclarationCommon {
loc: Location::dummy(),
associated_comments: NO_COMMENT_REFERENCE,
private: false,
name: Id::from(heap.alloc_str_for_test("name")),
type_parameters: vec![],
type_parameters: Some(annotation::TypeParameters {
location: Location::dummy(),
start_associated_comments: NO_COMMENT_REFERENCE,
ending_associated_comments: NO_COMMENT_REFERENCE,
parameters: vec![],
}),
extends_or_implements_nodes: vec![],
type_definition: TypeDefinition::Struct {
loc: Location::dummy(),
Expand All @@ -530,7 +569,12 @@ mod tests {
is_public: true,
is_method: true,
name: Id::from(PStr::LOWER_A),
type_parameters: Rc::new(vec![]),
type_parameters: Some(annotation::TypeParameters {
location: Location::dummy(),
start_associated_comments: NO_COMMENT_REFERENCE,
ending_associated_comments: NO_COMMENT_REFERENCE,
parameters: vec![],
}),
type_: builder.fn_annot_unwrapped(vec![], builder.int_annot()),
parameters: Rc::new(vec![]),
},
Expand All @@ -547,7 +591,12 @@ mod tests {
associated_comments: NO_COMMENT_REFERENCE,
private: false,
name: Id::from(heap.alloc_str_for_test("name")),
type_parameters: vec![],
type_parameters: Some(annotation::TypeParameters {
location: Location::dummy(),
start_associated_comments: NO_COMMENT_REFERENCE,
ending_associated_comments: NO_COMMENT_REFERENCE,
parameters: vec![],
}),
extends_or_implements_nodes: vec![],
type_definition: (),
members: vec![ClassMemberDeclaration {
Expand All @@ -556,7 +605,12 @@ mod tests {
is_public: true,
is_method: true,
name: Id::from(PStr::LOWER_A),
type_parameters: Rc::new(vec![]),
type_parameters: Some(annotation::TypeParameters {
location: Location::dummy(),
start_associated_comments: NO_COMMENT_REFERENCE,
ending_associated_comments: NO_COMMENT_REFERENCE,
parameters: vec![],
}),
type_: builder.fn_annot_unwrapped(vec![], builder.int_annot()),
parameters: Rc::new(vec![]),
}],
Expand Down
3 changes: 2 additions & 1 deletion crates/samlang-checker/src/global_signature.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub fn build_module_signature(
for member in toplevel.members_iter() {
let type_info = MemberSignature {
is_public: member.is_public,
type_parameters: TypeParameterSignature::from_list(&member.type_parameters),
type_parameters: TypeParameterSignature::from_list(member.type_parameters.as_ref()),
type_: FunctionType::from_annotation(&member.type_),
};
if member.is_method {
Expand All @@ -50,6 +50,7 @@ pub fn build_module_signature(
type_arguments: class
.type_parameters
.iter()
.flat_map(|it| &it.parameters)
.map(|it| Rc::new(Type::Generic(Reason::new(it.loc, Some(it.loc)), it.name.name)))
.collect_vec(),
}));
Expand Down
22 changes: 14 additions & 8 deletions crates/samlang-checker/src/main_checker.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1412,13 +1412,17 @@ fn check_class_member_conformance_with_signature(
expected: &MemberSignature,
actual: &ClassMemberDeclaration,
) {
if expected.type_parameters.len() != actual.type_parameters.len() {
let actual_type_params_len =
if let Some(tparams) = &actual.type_parameters { tparams.parameters.len() } else { 0 };
if expected.type_parameters.len() != actual_type_params_len {
let mut error = StackableError::new();
error.add_type_params_arity_error(actual.type_parameters.len(), expected.type_parameters.len());
error.add_type_params_arity_error(actual_type_params_len, expected.type_parameters.len());
error_set.report_stackable_error(actual.type_.location, error);
}
let mut has_type_parameter_conformance_errors = false;
for (e, a) in expected.type_parameters.iter().zip(actual.type_parameters.deref()) {
for (e, a) in
expected.type_parameters.iter().zip(actual.type_parameters.iter().flat_map(|it| &it.parameters))
{
if e.name != a.name.name {
has_type_parameter_conformance_errors = true;
}
Expand Down Expand Up @@ -1486,6 +1490,7 @@ pub fn type_check_module(
type_arguments: toplevel
.type_parameters()
.iter()
.flat_map(|it| &it.parameters)
.map(|it| Rc::new(Type::Generic(Reason::new(it.loc, Some(it.loc)), it.name.name)))
.collect_vec(),
};
Expand Down Expand Up @@ -1543,13 +1548,13 @@ pub fn type_check_module(
for member in toplevel.members_iter() {
let tparam_sigs = if member.is_method {
let mut sigs = TypeParameterSignature::from_list(toplevel.type_parameters());
sigs.append(&mut TypeParameterSignature::from_list(&member.type_parameters));
sigs.append(&mut TypeParameterSignature::from_list(member.type_parameters.as_ref()));
sigs
} else {
if !toplevel.is_class() {
error_set.report_illegal_function_in_interface(member.loc);
}
TypeParameterSignature::from_list(&member.type_parameters)
TypeParameterSignature::from_list(member.type_parameters.as_ref())
};
let has_interface_def = if member.is_method {
let resolved = global_signature::resolve_all_method_signatures(
Expand Down Expand Up @@ -1580,7 +1585,7 @@ pub fn type_check_module(
toplevel.name().name,
tparam_sigs,
);
for tparam in member.type_parameters.iter() {
for tparam in member.type_parameters.iter().flat_map(|it| &it.parameters) {
if let Some(bound) = &tparam.bound {
member_cx.validate_type_instantiation_allow_abstract_types(&Type::Nominal(
NominalType::from_annotation(bound),
Expand Down Expand Up @@ -1633,11 +1638,12 @@ pub fn type_check_module(
for member in &c.members {
let tparam_sigs = if member.decl.is_method {
let mut sigs = TypeParameterSignature::from_list(toplevel.type_parameters());
let mut local_sigs = TypeParameterSignature::from_list(&member.decl.type_parameters);
let mut local_sigs =
TypeParameterSignature::from_list(member.decl.type_parameters.as_ref());
sigs.append(&mut local_sigs);
sigs
} else {
TypeParameterSignature::from_list(&member.decl.type_parameters)
TypeParameterSignature::from_list(member.decl.type_parameters.as_ref())
};
let mut cx = TypingContext::new(
global_cx,
Expand Down
Loading

0 comments on commit a25dbee

Please sign in to comment.