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

Fix/id 104 unnecessary option wrapping #237

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

nemo83
Copy link
Collaborator

@nemo83 nemo83 commented Mar 5, 2025

fixes: #194

not a super critical ticket, but quick to implement while waiting

Comment on lines 965 to 1005
pub type Types {
MyType
NotMyType
SomeType { value: Int }
}

pub type OtherTypes {
DefinitelyNotMyType
MyOtherType
}

test test_my_type() fail {
let my_type = MyType
let not_my_type = NotMyType
// MyType == NotMyType
trace my_type
trace not_my_type
my_type == not_my_type
}

test test_some_type() fail {
MyType == SomeType(42)
}

test test_definitely_not_my_type() {
let my_type: Redeemer = MyType
let definitely_not_my_type: Redeemer = MyOtherType
my_type == definitely_not_my_type
}

test test_void_is_constr_0() {
let it_should_be_void: Redeemer = Void
expect not_void_anymore: Types = it_should_be_void
not_void_anymore == MyType
}

test test_void_is_not_constr_0() {
let it_should_be_void: Redeemer = Void
expect not_void_anymore: Types = it_should_be_void
not_void_anymore == NotMyType
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

some supporting tests to check out how the validate_handler_redeemer works.

It's just a coincidence that the test validate_handler_redeemer_fail_found_redeemer_is_not_handler_operator worked before. The desired outcome was to fail, but the assertion was wrong. The test, in fact passes.

This happens because types, once cast to Redeemer , lose their type information, and as long as the constr(index) matches, the validation will pass.

Unsure what's the impact of this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ignore previous

@nemo83 nemo83 marked this pull request as ready for review March 5, 2025 12:44
@nemo83 nemo83 requested a review from fabianbormann March 5, 2025 12:45
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.

🚨ID-104 Informational: Unnecessary Option Wrapping
2 participants