fix the required + nullable issue

[ArcturusZhang]

Fixes #2340
Fixes #3363
Fixes #3440
Fixes #3600

Description

This PR fixes multiple issues around nullable properties, especially when the property is required.

This PR also updates the InputType to make IsNullable property no longer optional. This optional property IsNullable could be problematic because of its optionality, sometimes we forget to give it a value and it causes hidden issues.

Checklist

To ensure a quick review and merge, please ensure:

• The PR has a understandable title and description explaining the why and what.
• The PR is opened in draft if not ready for review yet.
  • If opened in draft, please allocate sufficient time (24 hours) after moving out of draft for review
• The branch is recent enough to not have merge conflicts upon creation.

Ready to Land?

• Build is completely green
  • Submissions with test failures require tracking issue and approval of a CODEOWNER
• At least one +1 review by a CODEOWNER
• All -1 reviews are confirmed resolved by the reviewer
  • Override/Marking reviews stale must be discussed with CODEOWNERS first

[ArcturusZhang]

The assignment for nullable arrays is breaking test cases like this:

autorest.csharp/test/AutoRest.TestServer.Tests/ModelShapeTests.cs

Line 183 in 0df9bf7

public void NullablePropertiesCanBeInitializedWithNull()

[pshao25]

Let's remove all the Ignore in test\CadlRanchProjects.Tests\type-property-nullable.cs to see if it passes.

[pshao25]

Could you add tests in cadl-ranch for cases that the element of array is nullable and non-nullable?

[ArcturusZhang]

I am finding more and more issues when I am trying to "ensure the collection property can never be null".
Here is one related issue: #3440

[ArcturusZhang]

Add back the null check for collections

[ArcturusZhang]

Note - need to revert the changes on property bags since it is working right now

[ArcturusZhang]

Hi @m-nash I added a configuration deserialize-null-collection-as-null-value in this PR and it defaults to false.
When set to true, we will have previous behavior.
I have updated the GA libraries in the corresponding regen PR by adding this flag to them, also I fixed issues in openai library, please also take a look.

The changes in generated files of openai I believe is expected, here is the corresponding typespec file: https://github.com/Azure/azure-rest-api-specs/blob/a89c3854f2d1cca8c996e00c97af3179ecb9dd72/specification/cognitiveservices/OpenAI.Inference/models/chat.completions.tsp#L196

[ArcturusZhang]

@m-nash
Last time decision on sync up meeting:
since changing the behavior of nullable collections is a behavioral breaking change, we should add a configuration to control that behavior, and I added a new configuration deserialize-null-collection-as-null-value which defaults to false (true corresponds to the old behavior).

Therefore in the regen PR,

• There are quite a few RPs only have changes on autorest.md or tspConfig.yml without code changes, this is adding the config to stay on the old behavior to avoid behavior breaking changes.
• For those RPs with code changes, they are all in beta or preview therefore I did not add the new flag and breaking changes for them should be fine.

Other RPs might have API changes (for instance OpenAI) which is expected, because the initial purpose of this PR is fixing some issue around the required nullable properties. I added the corresponding typespec link in a comment in the regen to describe this change is expected.

[AlexanderSher]

Can we please add a test that will do alias spread with required collections?

[ArcturusZhang]

Can we please add a test that will do alias spread with required collections?

Added

[m-nash]

@ArcturusZhang please create a follow up issue to reduce the granularity of the new flag deserialize-null-collection-as-null-value. I imagine there is a small set of models / properties that have this issue and we don't want to proliferate it any further.

The granularity should probably be a list of ModelName.PropertyName to grandfather in.

[ArcturusZhang]

@ArcturusZhang please create a follow up issue to reduce the granularity of the new flag deserialize-null-collection-as-null-value. I imagine there is a small set of models / properties that have this issue and we don't want to proliferate it any further.

The granularity should probably be a list of ModelName.PropertyName to grandfather in.

issue created for this: #3613