Skip to content

Commit

Permalink
fix(dojo-bindgen): ensure typescript plugin output deterministic order (
Browse files Browse the repository at this point in the history
#2359)

* feat: ensure models and contracts are processed in order by Typescript plugin

* fix: run clippy on the whole workspace

* feat: add ordering to the unity plugin
  • Loading branch information
glihm authored Aug 29, 2024
1 parent cf1d044 commit a2fb8ce
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 32 deletions.
8 changes: 8 additions & 0 deletions crates/dojo-bindgen/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::cmp::Ordering;
use std::collections::HashMap;
use std::fs;
use std::path::PathBuf;
Expand Down Expand Up @@ -211,6 +212,13 @@ fn filter_model_tokens(tokens: &TokenizedAbi) -> TokenizedAbi {
TokenizedAbi { structs, enums, ..Default::default() }
}

/// Compares two tokens by their type name.
pub fn compare_tokens_by_type_name(a: &Token, b: &Token) -> Ordering {
let a_name = a.to_composite().expect("composite expected").type_name_or_alias();
let b_name = b.to_composite().expect("composite expected").type_name_or_alias();
a_name.cmp(&b_name)
}

#[cfg(test)]
mod tests {
use dojo_test_utils::compiler::CompilerTestSetup;
Expand Down
42 changes: 21 additions & 21 deletions crates/dojo-bindgen/src/plugins/typescript/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use dojo_world::contracts::naming;

use crate::error::BindgenResult;
use crate::plugins::BuiltinPlugin;
use crate::{DojoContract, DojoData, DojoModel};
use crate::{compare_tokens_by_type_name, DojoContract, DojoData, DojoModel};

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -277,36 +277,28 @@ export const {name}Definition = {{
for model in models {
let tokens = &model.tokens;

for token in &tokens.enums {
let mut sorted_structs = tokens.structs.clone();
sorted_structs.sort_by(compare_tokens_by_type_name);

let mut sorted_enums = tokens.enums.clone();
sorted_enums.sort_by(compare_tokens_by_type_name);

for token in &sorted_enums {
handled_tokens.push(token.to_composite().unwrap().to_owned());
}
for token in &tokens.structs {
for token in &sorted_structs {
handled_tokens.push(token.to_composite().unwrap().to_owned());
}

let mut structs = tokens.structs.to_owned();
structs.sort_by(|a, b| {
if a.to_composite()
.unwrap()
.inners
.iter()
.any(|field| field.token.type_name() == b.type_name())
{
std::cmp::Ordering::Greater
} else {
std::cmp::Ordering::Less
}
});

for token in &tokens.enums {
for token in &sorted_enums {
if handled_tokens.iter().filter(|t| t.type_name() == token.type_name()).count() > 1
{
continue;
}
out += TypescriptPlugin::format_enum(token.to_composite().unwrap()).as_str();
}

for token in &structs {
for token in &sorted_structs {
if handled_tokens.iter().filter(|t| t.type_name() == token.type_name()).count() > 1
{
continue;
Expand Down Expand Up @@ -601,14 +593,22 @@ impl BuiltinPlugin for TypescriptPlugin {

// Handle codegen for models
let models_path = Path::new("models.gen.ts").to_owned();
let models = data.models.values().collect::<Vec<_>>();
let mut models = data.models.values().collect::<Vec<_>>();

// Sort models based on their tag to ensure deterministic output.
models.sort_by(|a, b| a.tag.cmp(&b.tag));

let code = self.handle_model(models.as_slice(), &mut handled_tokens);

out.insert(models_path, code.as_bytes().to_vec());

// Handle codegen for contracts & systems
let contracts_path = Path::new("contracts.gen.ts").to_owned();
let contracts = data.contracts.values().collect::<Vec<_>>();
let mut contracts = data.contracts.values().collect::<Vec<_>>();

// Sort contracts based on their tag to ensure deterministic output.
contracts.sort_by(|a, b| a.tag.cmp(&b.tag));

let code = self.handle_contracts(contracts.as_slice(), &handled_tokens);

out.insert(contracts_path, code.as_bytes().to_vec());
Expand Down
25 changes: 20 additions & 5 deletions crates/dojo-bindgen/src/plugins/unity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use dojo_world::contracts::naming::{self, get_namespace_from_tag};

use crate::error::BindgenResult;
use crate::plugins::BuiltinPlugin;
use crate::{DojoContract, DojoData, DojoModel};
use crate::{compare_tokens_by_type_name, DojoContract, DojoData, DojoModel};

#[derive(Debug)]
pub struct UnityPlugin {}
Expand Down Expand Up @@ -237,7 +237,14 @@ namespace {namespace} {{

let mut model_struct: Option<&Composite> = None;
let tokens = &model.tokens;
for token in &tokens.structs {

let mut sorted_structs = tokens.structs.clone();
sorted_structs.sort_by(compare_tokens_by_type_name);

let mut sorted_enums = tokens.enums.clone();
sorted_enums.sort_by(compare_tokens_by_type_name);

for token in &sorted_structs {
if handled_tokens.contains_key(&token.type_path()) {
continue;
}
Expand All @@ -253,7 +260,7 @@ namespace {namespace} {{
out += UnityPlugin::format_struct(token.to_composite().unwrap()).as_str();
}

for token in &tokens.enums {
for token in &sorted_enums {
if handled_tokens.contains_key(&token.type_path()) {
continue;
}
Expand Down Expand Up @@ -542,8 +549,12 @@ impl BuiltinPlugin for UnityPlugin {
let mut out: HashMap<PathBuf, Vec<u8>> = HashMap::new();
let mut handled_tokens = HashMap::<String, Composite>::new();

let mut models = data.models.iter().collect::<Vec<_>>();
// Sort models based on their tag to ensure deterministic output.
models.sort_by(|(_, a), (_, b)| a.tag.cmp(&b.tag));

// Handle codegen for models
for (name, model) in &data.models {
for (name, model) in &models {
let models_path = Path::new(&format!("Models/{}.gen.cs", name)).to_owned();

println!("Generating model: {}", name);
Expand All @@ -552,8 +563,12 @@ impl BuiltinPlugin for UnityPlugin {
out.insert(models_path, code.as_bytes().to_vec());
}

let mut contracts = data.contracts.iter().collect::<Vec<_>>();
// Sort contracts based on their tag to ensure deterministic output.
contracts.sort_by(|(_, a), (_, b)| a.tag.cmp(&b.tag));

// Handle codegen for systems
for (name, contract) in &data.contracts {
for (name, contract) in &contracts {
let contracts_path = Path::new(&format!("Contracts/{}.gen.cs", name)).to_owned();

println!("Generating contract: {}", name);
Expand Down
7 changes: 1 addition & 6 deletions scripts/clippy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,4 @@ run_clippy() {
cargo +nightly clippy --all-targets "$@" -- -D warnings -D future-incompatible -D nonstandard-style -D rust-2018-idioms -D unused -D missing-debug-implementations
}

run_clippy --all-features --workspace --exclude katana --exclude katana-executor

run_clippy -p katana-executor --all
run_clippy -p katana
# TODO(kariy): uncomment this line when the `sir` support Cairo 2.6.3
# run_clippy -p katana --no-default-features --features sir
run_clippy --all-features --workspace

0 comments on commit a2fb8ce

Please sign in to comment.