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

Jump to bottom

Adds changes for sequence data type #144

Merged
merged 3 commits into from
Sep 25, 2024
Merged

Adds changes for sequence data type #144

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d716c37
Adds changes for sequence data type
desaikd Sep 18, 2024
3bda35b
Adds changes to fully_qualified_type_ref() for sequences
desaikd Sep 19, 2024
2a2cdd7
Adds suggested changes
desaikd Sep 20, 2024
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: 5 additions & 0 deletions code-gen-projects/schema/sequence.isl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type::{
name: sequence,
type: list,
element: string
}
136 changes: 132 additions & 4 deletions src/bin/ion/commands/generate/generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::commands::generate::context::CodeGenContext;
use crate::commands::generate::context::{CodeGenContext, SequenceType};
use crate::commands::generate::model::{
AbstractDataType, DataModelNode, FieldPresence, FieldReference, FullyQualifiedTypeReference,
ScalarBuilder, StructureBuilder, WrappedScalarBuilder,
ScalarBuilder, SequenceBuilder, StructureBuilder, WrappedScalarBuilder, WrappedSequenceBuilder,
};
use crate::commands::generate::result::{
invalid_abstract_data_type_error, invalid_abstract_data_type_raw_error, CodeGenResult,
Expand Down Expand Up @@ -351,14 +351,27 @@ impl<'a, L: Language + 'static> CodeGenerator<'a, L> {
.any(|it| matches!(it.constraint(), IslConstraintValue::Fields(_, _)))
{
self.build_structure_from_constraints(constraints, code_gen_context, isl_type)?
} else if constraints
.iter()
.any(|it| matches!(it.constraint(), IslConstraintValue::Element(_, _)))
{
if is_nested_type {
desaikd marked this conversation as resolved.
Show resolved Hide resolved
self.build_sequence_from_constraints(constraints, code_gen_context, isl_type)?
} else {
self.build_wrapped_sequence_from_constraints(
constraints,
code_gen_context,
isl_type,
)?
}
} else if Self::contains_scalar_constraints(constraints) {
if is_nested_type {
self.build_scalar_from_constraints(constraints, code_gen_context, isl_type)?
} else {
self.build_wrapped_scalar_from_constraints(constraints, code_gen_context, isl_type)?
}
} else {
todo!("Support for sequences, maps, scalars, and tuples not implemented yet.")
todo!("Support for maps and tuples not implemented yet.")
};

let data_model_node = DataModelNode {
Expand Down Expand Up @@ -587,7 +600,6 @@ impl<'a, L: Language + 'static> CodeGenerator<'a, L> {
isl_type
)))?;

// by default fields aren't closed
wrapped_scalar_builder.base_type(type_name);
found_base_type = true;
}
Expand Down Expand Up @@ -656,6 +668,122 @@ impl<'a, L: Language + 'static> CodeGenerator<'a, L> {

Ok(AbstractDataType::Scalar(scalar_builder.build()?))
}

/// Builds `AbstractDataType::WrappedSequence` from the given constraints.
/// ```
/// type::{
/// name: foo,
/// type: list,
/// element: string,
/// }
/// ```
/// This method builds `AbstractDataType`as following:
/// ```
/// AbstractDataType::WrappedSequence(
/// WrappedSequence {
/// name: vec!["org", "example", "Foo"] // assuming the namespace here is `org.example`
/// element_type: FullyQualifiedTypeReference { type_name: vec!["String"], parameters: vec![] } // Represents the element type for the list
/// sequence_type: SequenceType::List, // Represents list type for the given sequence
/// doc_comment: None // There is no doc comment defined in above ISL type def
/// source: IslType { .. } // Represents the `IslType` that is getting converted to `AbstractDataType`
/// }
/// )
/// ```
fn build_wrapped_sequence_from_constraints(
&mut self,
constraints: &[IslConstraint],
code_gen_context: &mut CodeGenContext,
parent_isl_type: &IslType,
) -> CodeGenResult<AbstractDataType> {
let mut wrapped_sequence_builder = WrappedSequenceBuilder::default();
wrapped_sequence_builder
.name(self.current_type_fully_qualified_name.to_owned())
.source(parent_isl_type.to_owned());
for constraint in constraints {
match constraint.constraint() {
IslConstraintValue::Element(isl_type_ref, _) => {
let type_name = self
desaikd marked this conversation as resolved.
Show resolved Hide resolved
.fully_qualified_type_ref_name(isl_type_ref, code_gen_context)?
.ok_or(invalid_abstract_data_type_raw_error(format!(
"Could not determine `FullQualifiedTypeReference` for type {:?}",
isl_type_ref
)))?;

wrapped_sequence_builder.element_type(type_name);
}
IslConstraintValue::Type(isl_type_ref) => {
if isl_type_ref.name() == "sexp" {
desaikd marked this conversation as resolved.
Show resolved Hide resolved
wrapped_sequence_builder.sequence_type(SequenceType::SExp);
} else if isl_type_ref.name() == "list" {
wrapped_sequence_builder.sequence_type(SequenceType::List);
}
}
_ => {
return invalid_abstract_data_type_error(
desaikd marked this conversation as resolved.
Show resolved Hide resolved
"Could not determine the abstract data type due to conflicting constraints",
);
}
}
}
Ok(AbstractDataType::WrappedSequence(
wrapped_sequence_builder.build()?,
))
}

/// Builds `AbstractDataType::Sequence` from the given constraints.
/// ```
/// {
/// type: list,
/// element: string,
/// }
/// ```
/// This method builds `AbstractDataType`as following:
/// ```
/// AbstractDataType::Sequence(
/// Sequence {
/// element_type: FullyQualifiedTypeReference { type_name: vec!["String"], parameters: vec![] } // Represents the element type for the list
/// sequence_type: SequenceType::List, // Represents list type for the given sequence
/// doc_comment: None // There is no doc comment defined in above ISL type def
/// source: IslType { .. } // Represents the `IslType` that is getting converted to `AbstractDataType`
/// }
/// )
/// ```
fn build_sequence_from_constraints(
&mut self,
constraints: &[IslConstraint],
code_gen_context: &mut CodeGenContext,
parent_isl_type: &IslType,
) -> CodeGenResult<AbstractDataType> {
let mut sequence_builder = SequenceBuilder::default();
sequence_builder.source(parent_isl_type.to_owned());
for constraint in constraints {
match constraint.constraint() {
IslConstraintValue::Element(isl_type_ref, _) => {
let type_name = self
.fully_qualified_type_ref_name(isl_type_ref, code_gen_context)?
.ok_or(invalid_abstract_data_type_raw_error(format!(
"Could not determine `FullQualifiedTypeReference` for type {:?}",
isl_type_ref
)))?;

sequence_builder.element_type(type_name);
}
IslConstraintValue::Type(isl_type_ref) => {
if isl_type_ref.name() == "sexp" {
sequence_builder.sequence_type(SequenceType::SExp);
} else if isl_type_ref.name() == "list" {
sequence_builder.sequence_type(SequenceType::List);
}
}
_ => {
return invalid_abstract_data_type_error(
"Could not determine the abstract data type due to conflicting constraints",
);
}
}
}
Ok(AbstractDataType::Sequence(sequence_builder.build()?))
}
}

#[cfg(test)]
Expand Down
61 changes: 49 additions & 12 deletions src/bin/ion/commands/generate/model.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,13 @@ impl FullyQualifiedTypeReference {
#[derive(Debug, Clone, PartialEq, Serialize)]
pub enum AbstractDataType {
// Represents a scalar type which also has a name attached to it and is nominally distinct from its base type.
#[allow(dead_code)]
WrappedScalar(WrappedScalar),
// Represents a scalar value (e.g. a string or integer or user defined type)
#[allow(dead_code)]
Scalar(Scalar),
// A series of zero or more values whose type is described by the nested `element_type`
#[allow(dead_code)]
Sequence(Sequence),
// Represents a sequence type which also has anme attached to it and is nominally distinct from its element type.
desaikd marked this conversation as resolved.
Show resolved Hide resolved
WrappedSequence(WrappedSequence),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to have to create corresponding Wrapped* variants for all of the non-wrapped variants? Why/why not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I think its in general there are only 3 types structure, sequence and scalar. Where only nested structure would have a name but scalar and sequence would not have name. Hence to separate out whether they are named or not I have them as a separate variant. I think having them separate allows us to perform specific operations for these types which will be stored as property instead of entirely different class in the generated code.

e.g. For below ISL:

type::{
   name: my_type,
   fields: {
        foo: {type: list, element: string} // this will not create a new nested class in the generated code
   }
}

Here's the generated code in Java:

class MyType {
   private java.util.ArrayList<String> foo;
   ...
}

whereas for structure type like following:

type::{
   name: my_type,
   fields: {
        foo: {
           fields: {
               bar: string
           }
        } 
   }
}

Here's the generated code in Java, which creates a new nested type for this nested structure:

class MyType {
   private NestedType1 foo;
   ...
   
   class NestedType1 {
       private String bar;
       ...
   }
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need the non-wrapped variant of sequence? We have parameterized, fully-qualified type references, so we should be able to model the non-wrapped lists as e.g. java.util.List<org.example.MyGeneratedClass>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WrpapedSequence use FullyQualifiedTypeName (not FullyqualifiedTypeReference). And WrappedSequence have a name, for example:

type::{
   name: foo, // this is the name of the wrapped sequence
   type: list,
   element: string
}

Above wrapped sequence's name will be stored as org.example.Foo .
Whereas Sequence would not have a name, for example:

{type: list, element: string} // does not have a name

We can surely store it as fully-qualified type references and thats what we already we store the element property as FullyWualfiiedTypeReference and then when storing this type into TypeStore we use java.util.ArrayList<String> but it still wouldn't have name (like the wrapped sequence has name as foo)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but the non-wrapped version (Sequence) does not have a name and does not have a class generated for it. Therefore, why do we need to have the unnamed Sequence model? You don't need a reference to the Sequence when you can just use a parameterized reference such as java.util.ArrayList<java.lang.String>.

For example, if you have this schema, the { type: list, element: string } does not need a Sequence to be created to represent it. Instead, the field bar should have the data model equivalent of a reference to java.util.ArrayList<java.lang.String>.

type::{
  name: foo,
  fields: {
    bar: { type: list, element: string }
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created an issue for this: #145. Need to look at if removing the non-wrapped variants has any requirements for how they are rendered in templates.

// A collection of field name/value pairs (e.g. a map)
Structure(Structure),
}
Expand All @@ -192,7 +191,12 @@ impl AbstractDataType {
AbstractDataType::Scalar(Scalar { doc_comment, .. }) => {
doc_comment.as_ref().map(|s| s.as_str())
}
AbstractDataType::Sequence(Sequence { doc_comment, .. }) => Some(doc_comment.as_str()),
AbstractDataType::Sequence(Sequence { doc_comment, .. }) => {
doc_comment.as_ref().map(|s| s.as_str())
}
AbstractDataType::WrappedSequence(WrappedSequence { doc_comment, .. }) => {
doc_comment.as_ref().map(|s| s.as_str())
}
AbstractDataType::Structure(Structure { doc_comment, .. }) => {
doc_comment.as_ref().map(|s| s.as_str())
}
Expand All @@ -206,6 +210,7 @@ impl AbstractDataType {
}
AbstractDataType::Scalar(s) => Some(s.base_type.to_owned()),
AbstractDataType::Sequence(seq) => Some(seq.element_type.to_owned()),
AbstractDataType::WrappedSequence(seq) => Some(seq.element_type.to_owned()),
AbstractDataType::Structure(structure) => Some(structure.name.to_owned().into()),
}
}
Expand Down Expand Up @@ -289,8 +294,6 @@ impl WrappedScalar {

/// Represents series of zero or more values whose type is described by the nested `element_type`
/// and sequence type is described by nested `sequence_type` (e.g. List or SExp).
/// If there is no `element` constraint present in schema type then `element_type` will be None.
/// If there is no `type` constraint present in schema type then `sequence_type` will be None.
/// e.g. Given below ISL,
/// ```
/// type::{
Expand All @@ -308,11 +311,12 @@ impl WrappedScalar {
#[allow(dead_code)]
#[derive(Debug, Clone, Builder, PartialEq, Serialize)]
#[builder(setter(into))]
pub struct Sequence {
pub struct WrappedSequence {
// Represents the fully qualified name for this data model
name: FullyQualifiedTypeName,
// Represents doc comment for the generated code
doc_comment: String,
#[builder(default)]
doc_comment: Option<String>,
// Represents the fully qualified name with namespace where each element of vector stores a module name or class/struct name.
// _Note: that a hashmap with (FullQualifiedTypeReference, DataModel) pairs will be stored in code generator to get information on the element_type name used here._
element_type: FullyQualifiedTypeReference,
Expand All @@ -326,6 +330,41 @@ pub struct Sequence {
source: IslType,
}

/// Represents series of zero or more values whose type is described by the nested `element_type`
/// and sequence type is described by nested `sequence_type` (e.g. List or SExp).
/// e.g. Given below ISL,
/// ```
/// type::{
/// name: sequence_type,
/// element: int,
/// type: list
/// }
/// ```
/// Corresponding generated code in Rust would look like following:
/// ```
/// struct SequenceType {
/// value: Vec<i64>
/// }
/// ```
#[derive(Debug, Clone, Builder, PartialEq, Serialize)]
#[builder(setter(into))]
pub struct Sequence {
// Represents doc comment for the generated code
#[builder(default)]
pub(crate) doc_comment: Option<String>,
// Represents the fully qualified name with namespace where each element of vector stores a module name or class/struct name.
// _Note: that a hashmap with (FullQualifiedTypeReference, DataModel) pairs will be stored in code generator to get information on the element_type name used here._
pub(crate) element_type: FullyQualifiedTypeReference,
// Represents the type of the sequence which is either `sexp` or `list`.
pub(crate) sequence_type: SequenceType,
// Represents the source ISL type which can be used to get other constraints useful for this type.
// For example, getting the length of this sequence from `container_length` constraint or getting a `regex` value for string type.
// This will also be useful for `text` type to verify if this is a `string` or `symbol`.
// TODO: `IslType` does not implement `Serialize`, define a custom implementation or define methods on this field that returns values which could be serialized.
#[serde(skip_serializing)]
pub(crate) source: IslType,
}

/// Represents a collection of field name/value pairs (e.g. a map)
/// e.g. Given below ISL,
/// ```
Expand Down Expand Up @@ -448,8 +487,7 @@ mod model_tests {
#[test]
fn sequence_builder_test() {
let expected_seq = Sequence {
name: vec![],
doc_comment: "This is sequence type of strings".to_string(),
doc_comment: Some("This is sequence type of strings".to_string()),
element_type: FullyQualifiedTypeReference {
type_name: vec!["String".to_string()],
parameters: vec![],
Expand All @@ -465,8 +503,7 @@ mod model_tests {

// sets all the information about the sequence except the `element_type`
seq_builder
.name(vec![])
.doc_comment("This is sequence type of strings")
.doc_comment(Some("This is sequence type of strings".to_string()))
.sequence_type(SequenceType::List)
.source(anonymous_type(vec![
type_constraint(named_type_ref("list")),
Expand Down
9 changes: 9 additions & 0 deletions src/bin/ion/commands/generate/result.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::commands::generate::model::{
ScalarBuilderError, SequenceBuilderError, StructureBuilderError, WrappedScalarBuilderError,
WrappedSequenceBuilderError,
};
use ion_schema::result::IonSchemaError;
use thiserror::Error;
Expand Down Expand Up @@ -71,6 +72,14 @@ impl From<SequenceBuilderError> for CodeGenError {
}
}

impl From<WrappedSequenceBuilderError> for CodeGenError {
fn from(value: WrappedSequenceBuilderError) -> Self {
CodeGenError::DataModelBuilderError {
description: value.to_string(),
}
}
}

impl From<StructureBuilderError> for CodeGenError {
fn from(value: StructureBuilderError) -> Self {
CodeGenError::DataModelBuilderError {
Expand Down
2 changes: 1 addition & 1 deletion src/bin/ion/commands/generate/templates/java/nested_type.templ
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
{{ sequence_info["element_type"] | fully_qualified_type_name }} value
{% elif inline_type.code_gen_type is containing("Scalar") %}
{% set scalar_info = model.code_gen_type["WrappedScalar"] %}
{% set base_type = scalar_info["name"]["parameters"][0] | fully_qualified_type_name %}
{% set base_type = scalar_info["base_type"] | fully_qualified_type_name %}
{{ base_type }} value
{% endif %}
{% endmacro %}
Loading
Loading