Support additional properties in non-legacy Modular

[MaryGao]

fixes #2506
fixes #2471
fixes #2400

design doc

We would have two styles:

• V1: extend Record<T>
• V2: additionalProperties bag

And we will have following conclusions:

• We have a feature flag for whether we use V1 or V2 for Modular.
  • legacyAdditionalProperties: true -> V1
  • legacyAdditionalProperties: false -> V2
• For the new service, we will enable V2 for Modular by default

Any complex ser/deser logic is not in-scope

• Additional properties follow-ups in Modular #3122

[Copilot]

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

[qiaozha]

Could you help double check what the additionalProperties type is for model B? I just wonder if tcgc or compiler has already done the union here or is it something emitter should do?

model A {
  ...Record<string>
}

model B extends A {
  ...Record<bytes>
}

[MaryGao]

Back to your question compiler would take care of any union work and you could see cases https://github.com/marygao/autorest.typescript/blob/support-additional-properties/packages/typespec-ts/test/modularUnit/scenarios/models/serialization/additionalProperties.md#typespec.

But to the specific extend case I am confirming with typespec team with correct behavior: microsoft/typespec#6983.

Currently it is compiled as overriding the parent one and see test case https://github.com/Azure/autorest.typescript/blob/b511d31069383facd6e4c60d9a82e56fbf91bb5b/packages/typespec-ts/test/modularUnit/scenarios/models/serialization/additionalProperties.md#skip-should-generate-additionalproperties-bag-for-non-legacy-code-if-additional-property-is-overrode.

[MaryGao]

Our generation is correct but typespec would report errors for non-compile code, let's wait their fix and turn on this case.

[qiaozha]

I didn't see we enable compatibilityMode option in mgmt plane, will it cause problem?

[MaryGao]

No problem for us, I searched in current spec there is no any real additional properties cases yet, that means no extends Record<..> or ...Record<...>.

The case we received for ImpactReporting is a fake case where the payload has the extra layer of additionalProperties explicitly - https://github.com/Azure/azure-rest-api-specs/pull/33352/files#r2011565369.

For long term I would prefer to never turn on compatibilityMode if possible, so finally we could remove this logic and only support for new style which better handles the type check stuff.