[http-client-csharp] the post processor should always keep customized code as root documents #5481
Conversation
microsoft-github-policy-service
bot
added
the
emitter:client:csharp
ArcturusZhang
force-pushed
the
post-processor-should-always-keep-customized-types
branch
from
c4a9678 to
668c68b
Compare
|
API change check API changes are not detected in this pull request. |
…-always-keep-customized-types
| private async Task<Compilation> GetProjectCompilationAsync() | ||
| { | ||
| _compilation ??= await _project.GetCompilationAsync(); | ||
| return _compilation!; | ||
| } |
There was a problem hiding this comment.
this is a duplicate with GetCompilationAsync method in this same class.
...ent-csharp/generator/Microsoft.Generator.CSharp/src/PostProcessing/GeneratedCodeWorkspace.cs
Outdated
Show resolved
Hide resolved
| var compilation = await project.GetCompilationAsync(); | ||
| return compilation!; |
There was a problem hiding this comment.
nit: return it instead of declaring a variable
There was a problem hiding this comment.
I would like to do that but the return of this method is Task<Compilation?> and here we want a Task<Compilation>, we must have a local variable and assert it is not null.
There was a problem hiding this comment.
otherwise it might look like this:
return (await project.GetCompilationAsync())!;
I prefer local variable rather than the above
There was a problem hiding this comment.
So it will be null if SupportsCompilation is false. It's not clear to me when that would happen but perhaps we should add some assertion that it is not null rather than using the null-forgiving operator.
There was a problem hiding this comment.
Ah I see this is specifically for the test helper, so probably less of an issue here. But for the GetCompilationAsync method we should add validation with a helpful error message that says SupportsCompilation must be true.
…rp/src/PostProcessing/GeneratedCodeWorkspace.cs Co-authored-by: Wei Hu <[email protected]>
| @@ -29,7 +29,7 @@ public async Task ExecuteAsync() | |||
| var generatedTestOutputPath = CodeModelPlugin.Instance.Configuration.TestGeneratedDirectory; | |||
|
|
|||
| GeneratedCodeWorkspace workspace = await GeneratedCodeWorkspace.Create(); | |||
| await CodeModelPlugin.Instance.InitializeSourceInputModelAsync(); | |||
| CodeModelPlugin.Instance.SourceInputModel = new SourceInputModel(await workspace.GetCompilationAsync()); | |||
There was a problem hiding this comment.
Just curious why we change the initializing here.
Fixes #5441
Previously in our generator, we have two instances of
GeneratedCodeWorkspace: one for the project that is being generated right now (the generated code project), one for the existing part of the generated library (the customized code project).This leads to an issue that in the post processor, only the generated documents are passed into the post processor, therefore the post processor actually does not know about the existence of the customized files.
This is not very correct because the generated files must need those customized files to work properly therefore they should be in the same project.
This PR refactors this part to change the structure of
GeneratedCodeWorkspace: now we only create one instance ofGeneratedCodeWorkspace, and the project inside it will be initialized with shared files and all the customized files in it.In this way, when we get to the post processor, it should be able to access all the necessary documents.