fix: Support deriving Print for enums

[0xicosahedron]

Fix: #1084

[0xicosahedron]

for implementation i am thinking of using this enum as reference:

#[derive(Print, Copy, Drop, Serde)]
enum Enemy {
    Unknown,
    Bot: felt252,
    OtherPlayer: ContractAddress,
}

are there any more complicated type that we might need to support? Struct variant doesn't seem to test for user type so i am not considering that here as well.

cc: @tarrencev

[tarrencev]

@0xicosahedron thanks for the work here. ideally we do support user defined types but happy to follow up with that PR. do you mind adding a test here: https://github.com/dojoengine/dojo/blob/main/crates/dojo-lang/src/plugin_test_data/print

[0xicosahedron]

for the above enum does this pseudocode implementation makes sense? for variants with no value its not an issue. but for variants that contain other types i need to figure out someway to get that type information.

match enemy {
  Unkown => print("unknown"); print("()");
  Bot(value) => print("Bot"); print(value);,
  OtherPlayer(addr) => print("OtherPlayer"); print(addr);,
}

is there any good documentation on variants, attributes, ElementList in cairo-lang-syntax?

[0xicosahedron]

figured it out

[0xicosahedron]

review please @tarrencev

[0xicosahedron]

@0xicosahedron thanks for the work here. ideally we do support user defined types but happy to follow up with that PR. do you mind adding a test here: https://github.com/dojoengine/dojo/blob/main/crates/dojo-lang/src/plugin_test_data/print

i think this PR should already support user defined type if they itself derive Print/Debug trait

[tarrencev]

Minor feedback otherwise lgtm

[0xicosahedron]

@tarrencev actually i am confused.

In the test generated_cairo_code the expanded code for struct Player is kept but we are not actually using it.

In the test we verify the expanded_cairo_code which doesn't contain the expanded code for struct Player so essentially the test is doing nothing.

[0xicosahedron]

so i am not sure if even deriving Print on struct is working because the related code is not generated where it should be.

[0xicosahedron]

for context this change was made in #1090

[ponderingdemocritus]

closing as was merged

[0xicosahedron]

@ponderingdemocritus i am confused, what was merged?

[0xicosahedron]

@tarrencev actually i am confused.

In the test generated_cairo_code the expanded code for struct Player is kept but we are not actually using it.

In the test we verify the expanded_cairo_code which doesn't contain the expanded code for struct Player so essentially the test is doing nothing.

bump

[0xicosahedron]

@glihm check this comment:

#1091 (comment)

I will get time to work on this tomorrow, will see if its still the case.

[glihm]

@glihm check this comment:

#1091 (comment)

I will get time to work on this tomorrow, will see if its still the case.

Yup, but not sure to follow, everything seems normal here. In the print test file, you added the enum, but remaining structs are still tested.

Or I misunderstood you sorry.

[0xicosahedron]

@glihm So in print test file there are this 5 sections.

• test_runner_name
• cairo_code
• generated_cairo_code
• expected_diagnostics
• expanded_cairo_code

generated_cairo_code contains the actual expansion of Print traits, expanded_cairo_code only contains expansion of Serde trait (which I have removed in this PR to reduce noise in the test).

In the test we only check the expanded_cairo_code section which contains serde methods which are not relevant for this test and we are never checking that expansion of Print trait is correct.

I am not sure why we don't see methods from Print trait in expanded_cairo_code.

You can check this by completely removing generated_cairo_code section and see the test still passing.

dojo/crates/dojo-lang/src/plugin_test.rs

Lines 113 to 119 in 3c55d86

	TestRunnerResult {
	outputs: OrderedHashMap::from([
	("expanded_cairo_code".into(), expanded_module),
	("expected_diagnostics".into(), joined_diagnostics),
	]),
	error,
	}

Does it make sense now?

[glihm]

Did you run the command I mentioned in my previous comment here? You should be all good once you run it.

[0xicosahedron]

okay merged with main and ran the command you provided it updated few class hash.

[0xicosahedron]

okay just realized why the tests where failing, i overrode the Cargo.lock file and it updated cairo compiler version from 2.4.0 to 2.4.1, reverted it back should be fixed now.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (41a48b8) 67.62% compared to head (c18e624) 67.62%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1091      +/-   ##
==========================================
- Coverage   67.62%   67.62%   -0.01%     
==========================================
  Files         218      218              
  Lines       21054    21047       -7     
==========================================
- Hits        14238    14233       -5     
+ Misses       6816     6814       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[glihm]

okay just realized why the tests where failing, i overrode the Cargo.lock file and it updated cairo compiler version from 2.4.0 to 2.4.1, reverted it back should be fixed now.

Thank you for all this follow up. The reason why the code of the test is not generated is because it can't compile.
In the implementation you provided, you only support variant with an inner type, and Cairo compilation fails for variant without any value like Enemy::Unknown.

You can leverage the OptionTypeClause to do so in order to know which variant type is it. Based on this, you only print the variant's name, or the name + it's value.

[0xicosahedron]

okay will take a look soon, thanks!

[0xicosahedron]

@glihm even after doing this changes and running the command you provided it didn't update the test files.

can you let me know how did you figure out that compilation is failing, so i can see the error locally?

[glihm]

@glihm even after doing this changes and running the command you provided it didn't update the test files.

Sorry it was my fault, I didn't update correctly the print test file, which must be executed under test configuration.

can you let me know how did you figure out that compilation is failing, so i can see the error locally?

I tested with a real project, compiling with sozo on this branch.

Just pushed the update + refacto the plugin test at the same occasion.

You're doing good, thanks to your comment I was able to identify the problem.

[0xicosahedron]

thank you sir!