Spread of Record<T> into a model

[timotheeguerin]

Taking this from this issue which was about intersecting a model with a Record #1649

Should this be allowed?

model FOo {
 i: int32;
 ...Record<string>;
}

Playground showing current behaviors

There is also some question on what does it change if we implement this proposal where ... would allow to override previous properties #1292 Does doing i: int32, ...Record<string> basically erase the i property.

[bterlson]

I think it should not be allowed. The only time it makes sense is if it's the first item in the model and there is only one, but at that point why not use is. From a bit of a deeper perspective, spread works on concrete properties, and I would be surprised if spreading something with an indexer on the property container brought along the indexer. It feels akin to bringing along decorators on the container, to me.

[timotheeguerin]

The issue is we then have this difference in behavior between Record<string> & { i: int32} which works and has a property that i that is an int32 and the rest string but you can't have a named model for that as model is Record<T> will enforce all properties to respect the indexer

[bterlson]

More support for a declare keyword?

[bterlson]

Also, I'm not entirely sure it makes sense for is to validate the indexer constraint? It makes sense to do so for extends which is a subtyping operation, but is doesn't have to result in subtypes necessarily. I suppose similar logic could argue for overriding properties with whatever type, something I know we discussed allowing at one point (and probably rejected for good reasons I'm forgetting).

[timotheeguerin]

I believe it is because typescript doesn't allow it

and same without extends

[timotheeguerin]

And intersection does the same as we have now I believe

[qiaozha]

Yeah, I also found typescript can't express with different types between the known properties and additional properties.
and the original ask is from an external customer Azure/autorest.typescript#1940 (comment)

[timotheeguerin]

Typescript does if you use type = and intersect the record with the other properties. But if you use in an interface it is not happy.

[qiaozha]

My concern is if typescript can not express this, I don't know how other languages could possibly do that. so it may turn out to be a feature none of the SDK emitters can support :(

[bterlson]

Replying to past Brian, who was wrong:

I think it should not be allowed. The only time it makes sense is if it's the first item in the model and there is only one, but at that point why not use is.

1. Spreading multiple things can make conceptual sense (e.g. by unioning the index type), though it's unclear how useful this is in real APIs.
2. is presently creates a subtype constraint, so doesn't allow declared properties to exist that aren't assignable to the index type, which doesn't help certain scenarios where known properties can be of any type but additional properties must have a specific type. If is didn't validate the index constraint, then sure, you could use is. But this isn't a very strong argument against making spread do the expected thing.

From a bit of a deeper perspective, spread works on concrete properties, and I would be surprised if spreading something with an indexer on the property container brought along the indexer. It feels akin to bringing along decorators on the container, to me.

Spread works on properties period, the fact that a set of properties is unbounded doesn't mean they are not properties. Indexer is an implementation detail of how to represent this unbounded set of properties.

[timotheeguerin]

Record<T> and additional properties

Current behavior

Today you can specify type for properties on a model via is or extends of a Record

model WithIs is Record<string> {
  string: string;

  nonString: int32;
             ^ error int32 is not assignable to string
}

model WithExtends extends Record<string> {
  string: string;

  nonString: int32;
             ^ error int32 is not assignable to string
}

Additionally you can use intersection to get some behavior that is similar to what we want in this proposal but as we'll see later should actually be an error.

// This one is good and is similar-ish to `is Record<string>` it would be different if `Record<T>` had annotations on it.
alias MergedGood = Record<string> & {name: string};

// This one works today but what it should really mean is that `name` is the intersection of `int32` and `string` which is `never`
alias MergedBad = Record<int32> & {name: string};

Spreading Record<T>: This today doesn't fail at the compiler level but the record information is lost

model WithSpread {
  name: string;
  age: int32;
  ...Record<string>;
}

Proposal

1. Make & intersection actually mean intersection of properties

This means the following code should be an error because we cannot intersect string and int32(what we do if you intersect explicitly)

alias MergedBad = Record<int32> & {name: string};
                                 ^ cannot intersect record ...

2. Allow spreading Record<T>

This would make & and ... meaning diverge:

• & would mean intersection of types(as described above) and so would merge the properties and if types are not able to be merged it would be an error/resolve to never
• ... would mean spread the properties and so would add extra properties. This means if you spread extra properties then those get added, if properties name conflict then it is an error. If you spread a Record, it describe the remaining properties.

model WithSpread {
  name: string;
  age: int32;
  ...Record<string>;
}

The code above would mean you have a model with a property name of type string, a property age of type int32 and any other property of type string.

Multiple ...Record<T>

As ... allows multiple instance in the same model it is possible to have multiple ...Record<T> in the same model. This would mean that each ...Record<T> would be a union of each other.
If you spread Record<string> and Record<int32> it means all extra properties could either be a string or int32. You are basically saying I am spreading some string properties and some int32 properties.

model WithSpread {
  name: string;
  enabled: boolean;
  ...Record<string>;
  ...Record<itn32>;
}

// is equivalent to
model WithSpread {
  name: string;
  enabled: boolean;
  ...Record<int32 | string>;
}

Difference with is Record<T>(or extends Record<T>)

model Foo is Record<T> will be defining the indexer for Foo which means all properties added to Foo must be of type T. This also means that Foo can be used when a constraints of type Record<T> is needed.

However when spreading Record<T> it will not be setting the indexer as some properties could technically be of a different type.

model Template<T extends Record<string>> {t: T}

model WithSpread {
  age: int32;
  ...Record<string>;
}
model WithIs is Record<string> {
  string: string;

}

model Test {
  good: Template<Record<string>>;
  goodWithIs: Template<WithIs>;
  bad: Template<WithSpread>;
        ^ error WithSpread is not assignable to Record<string>
}

Technically having this would also pass the constraint above as all the extra properties are also assignable to the constraint.

```tsp
model WithSpread {
  name: string;
  ...Record<string>;
}

is or extends a model with ... Record<T>

At this point it should be an error if you try to define a property that is incompatible with the base model spread indexer.

[ArthurMa1978]

@timotheeguerin is Multiple ...Record<T> a valid scenario? It is equivalent to Unknow if it supports multiple T.

[timotheeguerin]

This was talked with the brainstorming design meeting yersteday and it feels like we should allow it. Design above explain what is means. This is just a proposal though, we might not allow it in the final design.

[timotheeguerin]

Design good.

1. file implementation issue
2. file issue deciding if linter rule in azure-core

[timotheeguerin]

implementation in #2785