demergent-labs/cdk_framework

is_type_alias needs refinement

Closed this issue · 1 comments

@bdemann commented on Thu Feb 23 2023

Determining if something is a type alias is challenging with our new system. Currently there are three things that all look like candid type aliases.

  1. Genuine candid type aliases
    MyCandidTypeAlias = MyRecord;
    class MyRecord(Record):
         ...
  2. Variable assignments
    1. This includes a variable assigned to another variable and you can't discern that it will eventually be a constant just looking at it
    my_var = 3
    your_var = my_var
  3. Python type aliases
    1. This includes aliases to aliases that eventually resolve to be a python type alias but isn't discernible just by looking at it.
    my_float = float
    your_float = my_float

I think the base cases (like my_var and my_float) would be pretty simple to sort out. For the variable assignments we can just check to see if the right hand side of a type alias could be a candid data type annotation. For the type alias we could just check that the right hand side isn't a built in python type or a non-record non-variant class. But the other cases (your_var, and your_float) are going to be more challenging to figure out.

On a related note, we also need to fix this issue with the no None constant enforcement

ExprKind::Constant { value, .. } => match value {
    Constant::None => {
        // TODO the problem is that we need to have None in the actual null and void type alias
        eprintln!("{}", self.none_cant_be_a_type_error());
        Primitive::Null.to_data_type()
    }
    _ => {
        todo!()
    }
},

@bdemann commented on Thu Feb 23 2023

I think the quickest way to fix this would be to have a kybra wrapper to denote that a type alias should be a candid type alias
For example

from kybra import TypeAlias
my_type_alias = TypeAlias[MyRecord]
class MyRecord(Record):
     ...

@dansteren commented on Thu Feb 23 2023

Alright, if we decide to do a tree-traversal type algorithm to make sure type aliases point to correct types. The key idea would be to take the list of all type aliases and shrink it down to things that just resolve to valid candid types. This happens very first, before we do any of the validation for canister methods or what not.

Algorithm:

while(type_aliases.len() hasn't changed) {
    for each item in the type_aliases
        if(the right hand side (or traverse records/variants down to their inner type) is a typeref)
            if so, then make sure the ref's name is in the list of all types (including type_aliases, and records, etc.)
            if the name isn't in the list then remove it from the list, Meaning, that Four isn't a valid type because it references not a valid type
}
// If the size of the list is smaller, then rerun algorithm. You're done when the list size doesn't change.

Here's some test cases in Azle: https://gist.github.com/dansteren/9eb3244101a2bd799a5a79669123249a
And in Kybra: https://gist.github.com/dansteren/b7ecf40c31c80d450c831701705f7426


@bdemann commented on Thu Mar 02 2023

We decided to move forward with a wrapper type. I'm going to leave this ticket open for future consideration, but lower the priority


@bdemann commented on Sat Mar 04 2023

See also demergent-labs/kybra#226

I opened this thinking it belonged to the CDK Framework not kybra. Really it belongs in kybra though so this is just a duplicate.