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

Make VariablyOccurringTypeArgument compatible with TypeArgument #283

Open
tjcsrc16 opened this issue Aug 8, 2023 · 3 comments
Open

Make VariablyOccurringTypeArgument compatible with TypeArgument #283

tjcsrc16 opened this issue Aug 8, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@tjcsrc16
Copy link

tjcsrc16 commented Aug 8, 2023

In the Ion Schema 2.0 definition Variably Occurring Type Arguments are defined as

<VARIABLY_OCCURRING_TYPE_ARGUMENT> ::= { <OCCURS>, <CONSTRAINT>... } | <TYPE_ARGUMENT>

Based on this, it would be useful to be able to substitute TypeArgument any place a VariablyOccurringTypeArgument is required or at least be able to construct a VariablyOccurringTypeArgument without passing occurs, which will use the default for the given TypeArgument.

@tjcsrc16 tjcsrc16 added the enhancement New feature or request label Aug 8, 2023
@popematt
Copy link
Contributor

it would be useful to be able to substitute TypeArgument any place a VariablyOccurringTypeArgument is required

As long as you are referring to substituting List<TypeArgument> in place of List<VariablyOccurringTypeArgument>, then this should be a simple matter of adding a secondary constructor to the Fields and OrderedElements constraints. Would that solve your problem? E.g.:

data class OrderedElements(val types: List<VariablyOccurringTypeArgument>) : Constraint {
    
    constructor(types: List<TypeArgument>): 
        this(types.map { VariablyOccurringTypeArgument(OCCURS_REQUIRED, it) })
}

@tjcsrc16
Copy link
Author

tjcsrc16 commented Aug 23, 2023

I think that is useful but it doesn't solve my problem. But now I see the issue with what I want. I want to store TypeArguments and VariablyOccurringTypeArguments in the same container. I can't promote the TypeArgument to a VariablyOccurringTypeArgument because occurs is not optional. If I demote the VariablyOccurringTypeArgument to a TypeArgument, I lose the occurs which I don't want.

It seems like this should be allowed by the BNF. For now I have wrapped TypeArgument and VariablyOccurringTypeArgument in another type hierarchy that can be one or the other, e.g.:

sealed class EffectiveType {
    data class DefaultOccurrenceType(val type: TypeArgument) : EffectiveType()
    data class VariableOccurrenceType(val type: VariablyOccurringTypeArgument) : EffectiveType()
}

I am still experimenting for now.

@popematt
Copy link
Contributor

Thanks for providing more details about your use case.

I want to store TypeArguments and VariablyOccurringTypeArguments in the same container.

This is particularly interesting to me. Can you explain more about why you want to do this? Do you have some TypeArgument that you want to pass around and use as an argument for Fields and for OrderedElements?

It seems like this should be allowed by the BNF.

I agree that it does seem that way, but the BNF is specifically for describing the ISL syntax. When it comes to modeling Ion Schema, neither a TypeArgument or a VariablyOccurringTypeArgument can be substituted for each other. However, as an argument to a constructor/factory function, I think it would make sense to allow them to be substituted for each other.

I think there are at least two ways we could improve this. One would be to create a builder/DSL for the model that looks more like ISL syntax. (This is something that I would like to do regardless, but I haven't had the opportunity yet because it's been a lower priority than some of our other work.) The second you have already mentioned—we can introduce a union type that is able to hold either a TypeArgument or a VariablyOccurringTypeArgument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants