search

Azure / autorest.csharp

Extension for AutoRest (https://github.com/Azure/autorest) that generates C# code
MIT License
140 stars 160 forks source link

Check the usage of readonly property type in RoundTrip model for DPG #4820

Open live1206 opened 3 weeks ago

live1206 commented 3 weeks ago

TSP:

  model ResultModel {
    name: string;
  }

  model RoundTripModel {
    @visibility("read")
    result: ResultModel;
  }

  @route("/modelInReadOnlyProperty")
  @put
  op modelInReadOnlyProperty(@body body: RoundTripModel): {
    @body body: RoundTripModel;
  };

detailed case can be found in this pr: https://github.com/Azure/cadl-ranch/pull/587

In this case, usage for ResultModel should be Output instead of RoundTrip, because this model is readonly.

ArcturusZhang commented 3 weeks ago

In current DPG's implementation, ResultModel is generated as a RoundTrip model. Therefore we need a fix

ArcturusZhang commented 3 weeks ago

Also in today's offline sync up, we kind of have an agreement on the patch convenience methods, therefore we have a way to use the usage from TCGC directly therefore we could expect a final fix for this.

ArcturusZhang commented 2 weeks ago

This should have fixed this issue despite we did not know quite well which part introduces the fix. But from the result's perspective, this is fixed.

live1206 commented 2 weeks ago

This should have fixed this issue despite we did not know quite well which part introduces the fix. But from the result's perspective, this is fixed.

Are you saying csharp emitter has not consumed TCGC usage yet, but the usage still got fixed?

ArcturusZhang commented 2 weeks ago

This should have fixed this issue despite we did not know quite well which part introduces the fix. But from the result's perspective, this is fixed.

Are you saying csharp emitter has not consumed TCGC usage yet, but the usage still got fixed?

that statement is not fully correct. We converted the usage value from TCGC to ours (like this) - therefore when tcgc has the fix, we get the changes in DPG as well.

live1206 commented 2 weeks ago

This should have fixed this issue despite we did not know quite well which part introduces the fix. But from the result's perspective, this is fixed.

Are you saying csharp emitter has not consumed TCGC usage yet, but the usage still got fixed?

that statement is not fully correct. We converted the usage value from TCGC to ours (like this) - therefore when tcgc has the fix, we get the changes in DPG as well.

OK. Then we know the fix is from TCGC.