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

Adds changes for setting up integration roundtrip tests for generated code #94

Merged
merged 6 commits into from
Apr 5, 2024

Conversation

desaikd
Copy link
Contributor

@desaikd desaikd commented Apr 3, 2024

Issue #88:

Description of changes:

This PR works on adding set-up changes for integration roundtrip tests on generated code

List of changes:

  • Adds some changes for struct templates, that were resulting in specific errors with roundtrip testing
    • Replaces to_string()? with to_str()?.to_string(). (to_string() would return a raw string e.g. ""hello"" whereas to_str() returns a string value as "hello")
    • Adds isl_type_name property in Field this helps store symbol as Rust/Java string but when writing it as Ion it will use write_symbol using write_{{ field.isl_type_name}}.
  • Add a namespace option for java code generation (This option is only required when --language option is set as java).
  • Adds changes to use only a single module with all generated data models in Rust. Generated module is named as ion_generated_code.rs. This will help in importing entire file using include! macro and also simplifies attaching a single file as a module in any Rust crate. More information can be found in Rust guide on code generation: https://doc.rust-lang.org/cargo/reference/build-script-examples.html#code-generation
  • Adds changes to generate code for all schema files in a directory/authority.
  • Adds changes to current tests and clippy changes.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

* Add ISL type name in the Field to call read/write APIs for
symbol/string or blob/clob accordingly.
* Adds manifest directory in the path for the templates
* Adds `CodeGenerator::generate_code_for_authorities` which generates
code for each schema in given authorities
* Makes `--schema` optional for code generation
* Removed Import statements from template as those were used for
reference to other generated data models
* Removes mod.templ as the generated code now uses single module
* Removes generating `ion_data_model` module for generated code instead
uses a single file `ion_generated_code`
@@ -19,27 +21,72 @@ pub(crate) struct CodeGenerator<'a, L: Language> {
// more information: https://docs.rs/tera/latest/tera/
pub(crate) tera: Tera,
output: &'a Path,
// This field is used by Java code generation to get the namespace for generated code.
// For Rust code generation, this will be set to None.
namespace: Option<&'a str>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(🗺️ PR tour) This adds a new namespace property to code generator which will only be used while Java code generation to specify namespace of generated code and is collected from --namespace option from CLI command.

// Represents a counter for naming anonymous type definitions
pub(crate) anonymous_type_counter: usize,
phantom: PhantomData<L>,
}

impl<'a> CodeGenerator<'a, RustLanguage> {
pub fn new(output: &'a Path) -> CodeGenerator<RustLanguage> {
let tera = Tera::new(&format!(
"{}/src/bin/ion/commands/beta/generate/templates/rust/*.templ",
env!("CARGO_MANIFEST_DIR")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(🗺️ PR tour) Tera is the templating engine sued for generating code and this sets the path to all template files using CARGO_MANIFEST_DIR so that it can be found from anywhere in a cargo project.

.unwrap();

// Render the imports into output file
let rendered = tera.render("import.templ", &Context::new()).unwrap();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(🗺️ PR tour) This separates the import statement which will be common for all generated data models and since Rust code generation has single file being generated with all the data models, the next step here writes this import to that file ion_generated_code.

phantom: PhantomData,
}
}

/// Generates code in Rust for all the schemas in given authorities
pub fn generate_code_for_authorities(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(🗺️ PR tour) This API is used to generate code for all schema files in given authorities, it uses generate() to generate code for a single schema file.

// this will be used for Rust to create mod.rs which lists all the generated modules
let mut modules = vec![];
let mut module_context = tera::Context::new();
pub fn generate_code_for_schema(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(🗺️ PR tour) Similar to generate_code_for_authorities but only generates code for given schema file using schema_id.

@@ -121,8 +115,8 @@ impl Language for RustLanguage {
"rust".to_string()
}

fn file_name_for_type(name: &str) -> String {
name.to_case(Case::Snake)
fn file_name_for_type(_name: &str) -> 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.

(🗺️ PR tour) Since Rust code generation uses a single file for all generated data models this method outputs a constant file name ion_generated_code

{% else %}
reader.read_{{ target_kind_name }}()?;
reader.read_{% if fields[0].isl_type_name == "symbol" %}symbol()?.text().unwrap(){% else %}{{ fields[0].value | replace(from="string", to ="str") }}()?{% endif %}{% if fields[0].value | lower == "string" %} .to_string() {% endif %};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(🗺️ PR tour) Replaces to_string()? -> to_str()?.to_string() and uses isl_type_name to read symbol.

@@ -1,3 +0,0 @@
{% for module in modules -%}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(🗺️ PR tour) This has been removed as its no longer needed due to single file being generated for Rust.

@@ -0,0 +1 @@
use ion_rs::{IonResult, IonReader, Reader, IonWriter, StreamItem};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(🗺️ PR tour) This template points to the common import statement for all generated data models which has imports for Ion reader, writer etc.

{% if import %}
import ion_data_model.{{ import.name | upper_camel }};
{% endif %}
package {{ namespace }};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(🗺️ PR tour) These changes were found during roundtrip testing locally.

  • Adds namespace declaration statement.
  • Adds ArrayList import.
  • Add changes to use camel casing for field names in Java.

@desaikd desaikd requested a review from nirosys April 3, 2024 22:50
src/bin/ion/commands/beta/generate/generator.rs Outdated Show resolved Hide resolved
src/bin/ion/commands/beta/generate/generator.rs Outdated Show resolved Hide resolved
src/bin/ion/commands/beta/generate/generator.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@nirosys nirosys left a comment

Choose a reason for hiding this comment

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

Minor comments/questions, but otherwise looks good to me.

@desaikd desaikd merged commit 425db12 into master Apr 5, 2024
7 checks passed
@desaikd desaikd deleted the desaikd-tests-set-up branch August 21, 2024 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants