Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[http-client-csharp] the post processor should always keep customized code as root documents #5481

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public async Task ExecuteAsync()
var generatedTestOutputPath = CodeModelPlugin.Instance.Configuration.TestGeneratedDirectory;

GeneratedCodeWorkspace workspace = await GeneratedCodeWorkspace.Create();
await CodeModelPlugin.Instance.InitializeSourceInputModelAsync();
await CodeModelPlugin.Instance.InitializeSourceInputModelAsync(workspace);

var output = CodeModelPlugin.Instance.OutputLibrary;
Directory.CreateDirectory(Path.Combine(generatedSourceOutputPath, "Models"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,15 @@ public void AddSharedSourceDirectory(string sharedSourceDirectory)

private SourceInputModel? _sourceInputModel;

internal async Task InitializeSourceInputModelAsync()
/// <summary>
/// This method initializes the source input model for the plugin.
/// This method must be called before any generated documents are added into the workspace.
/// </summary>
/// <param name="workspace"></param>
/// <returns></returns>
internal async Task InitializeSourceInputModelAsync(GeneratedCodeWorkspace workspace)
{
GeneratedCodeWorkspace existingCode = GeneratedCodeWorkspace.CreateExistingCodeProject([Instance.Configuration.ProjectDirectory], Instance.Configuration.ProjectGeneratedDirectory);
_sourceInputModel = new SourceInputModel(await existingCode.GetCompilationAsync());
_sourceInputModel = new SourceInputModel(await workspace.GetCompilationAsync());
}

internal HashSet<string> TypesToKeep { get; } = new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ internal class GeneratedCodeWorkspace
private static readonly string[] _sharedFolders = [SharedFolder];

private Project _project;
private Compilation? _compilation;
private Dictionary<string, string> PlainFiles { get; }

private GeneratedCodeWorkspace(Project generatedCodeProject)
Expand Down Expand Up @@ -107,8 +106,11 @@ public async Task AddGeneratedFile(CodeFile codefile)
private async Task<Document> ProcessDocument(Document document)
{
var syntaxTree = await document.GetSyntaxTreeAsync();
var compilation = await GetProjectCompilationAsync();
if (syntaxTree != null)
var compilation = await _project.GetCompilationAsync();

Debug.Assert(compilation is not null);

if (syntaxTree != null && compilation != null)
{
var semanticModel = compilation.GetSemanticModel(syntaxTree);
var modelRemoveRewriter = new MemberRemoverRewriter(_project, semanticModel);
Expand Down Expand Up @@ -143,12 +145,15 @@ private static Project CreateGeneratedCodeProject()

internal static async Task<GeneratedCodeWorkspace> Create()
{
// prepare the generated code project
var projectTask = Interlocked.Exchange(ref _cachedProject, null);
var generatedCodeProject = projectTask != null ? await projectTask : CreateGeneratedCodeProject();
var project = projectTask != null ? await projectTask : CreateGeneratedCodeProject();

var outputDirectory = CodeModelPlugin.Instance.Configuration.OutputDirectory;
var projectDirectory = CodeModelPlugin.Instance.Configuration.ProjectDirectory;
var generatedDirectory = CodeModelPlugin.Instance.Configuration.ProjectGeneratedDirectory;

// add all the documents but except for the documents from the generated directory
ArcturusZhang marked this conversation as resolved.
Show resolved Hide resolved
if (Path.IsPathRooted(projectDirectory) && Path.IsPathRooted(outputDirectory))
{
projectDirectory = Path.GetFullPath(projectDirectory);
Expand All @@ -157,37 +162,15 @@ internal static async Task<GeneratedCodeWorkspace> Create()
Directory.CreateDirectory(projectDirectory);
Directory.CreateDirectory(outputDirectory);

generatedCodeProject = AddDirectory(generatedCodeProject, projectDirectory, skipPredicate: sourceFile => sourceFile.StartsWith(outputDirectory));
project = AddDirectory(project, projectDirectory, skipPredicate: sourceFile => sourceFile.StartsWith(generatedDirectory));
}

foreach (var sharedSourceFolder in CodeModelPlugin.Instance.SharedSourceDirectories)
{
generatedCodeProject = AddDirectory(generatedCodeProject, sharedSourceFolder, folders: _sharedFolders);
}

generatedCodeProject = generatedCodeProject.WithParseOptions(new CSharpParseOptions(preprocessorSymbols: new[] { "EXPERIMENTAL" }));
return new GeneratedCodeWorkspace(generatedCodeProject);
}

internal static GeneratedCodeWorkspace CreateExistingCodeProject(IEnumerable<string> projectDirectories, string generatedDirectory)
{
var workspace = new AdhocWorkspace();
var newOptionSet = workspace.Options.WithChangedOption(FormattingOptions.NewLine, LanguageNames.CSharp, NewLine);
workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(newOptionSet));
Project project = workspace.AddProject("ExistingCode", LanguageNames.CSharp);

foreach (var projectDirectory in projectDirectories)
{
if (Path.IsPathRooted(projectDirectory))
{
project = AddDirectory(project, Path.GetFullPath(projectDirectory), skipPredicate: sourceFile => sourceFile.StartsWith(generatedDirectory));
}
project = AddDirectory(project, sharedSourceFolder, folders: _sharedFolders);
}

project = project
.AddMetadataReferences(_assemblyMetadataReferences.Value.Concat(CodeModelPlugin.Instance.AdditionalMetadataReferences))
.WithCompilationOptions(new CSharpCompilationOptions(
OutputKind.DynamicallyLinkedLibrary, metadataReferenceResolver: _metadataReferenceResolver.Value, nullableContextOptions: NullableContextOptions.Disable));
project = project.WithParseOptions(new CSharpParseOptions(preprocessorSymbols: new[] { "EXPERIMENTAL" }));

return new GeneratedCodeWorkspace(project);
}
Expand Down Expand Up @@ -248,11 +231,5 @@ public async Task PostProcessAsync()
break;
}
}

private async Task<Compilation> GetProjectCompilationAsync()
{
_compilation ??= await _project.GetCompilationAsync();
return _compilation!;
}
Comment on lines -252 to -256
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a duplicate with GetCompilationAsync method in this same class.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,28 +103,28 @@ protected virtual bool ShouldIncludeDocument(Document document) =>

/// <summary>
/// This method marks the "not publicly" referenced types as internal if they are previously defined as public. It will do this job in the following steps:
/// 1. This method will read all the public types defined in the given <paramref name="project"/>, and build a cache for those symbols
/// 1. This method will read all the public types defined in the given <paramref name="generatedProject"/>, and build a cache for those symbols
/// 2. Build a public reference map for those symbols
/// 3. Finds all the root symbols, please override the <see cref="IsRootDocument(Document)"/> to control which document you would like to include
/// 4. Visit all the symbols starting from the root symbols following the reference map to get all unvisited symbols
/// 5. Change the accessibility of the unvisited symbols in step 4 to internal
/// </summary>
/// <param name="project">The project to process</param>
/// <param name="generatedProject">The project to process</param>
/// <returns>The processed <see cref="Project"/>. <see cref="Project"/> is immutable, therefore this should usually be a new instance </returns>
public async Task<Project> InternalizeAsync(Project project)
public async Task<Project> InternalizeAsync(Project generatedProject)//, Project existingProject)
{
var compilation = await project.GetCompilationAsync();
var compilation = await generatedProject.GetCompilationAsync();
if (compilation == null)
return project;
return generatedProject;

// first get all the declared symbols
var definitions = await GetTypeSymbolsAsync(compilation, project, true);
var definitions = await GetTypeSymbolsAsync(compilation, generatedProject, true);
// build the reference map
var referenceMap =
await new ReferenceMapBuilder(compilation, project).BuildPublicReferenceMapAsync(
await new ReferenceMapBuilder(compilation, generatedProject).BuildPublicReferenceMapAsync(
definitions.DeclaredSymbols, definitions.DeclaredNodesCache);
// get the root symbols
var rootSymbols = await GetRootSymbolsAsync(project, definitions);
var rootSymbols = await GetRootSymbolsAsync(generatedProject, definitions);
// traverse all the root and recursively add all the things we met
var publicSymbols = VisitSymbolsFromRootAsync(rootSymbols, referenceMap);

Expand All @@ -135,20 +135,20 @@ public async Task<Project> InternalizeAsync(Project project)
{
foreach (var node in definitions.DeclaredNodesCache[symbol])
{
nodesToInternalize[node] = project.GetDocumentId(node.SyntaxTree)!;
nodesToInternalize[node] = generatedProject.GetDocumentId(node.SyntaxTree)!;
}
}

foreach (var (model, documentId) in nodesToInternalize)
{
project = MarkInternal(project, model, documentId);
generatedProject = MarkInternal(generatedProject, model, documentId);
}

var modelNamesToRemove =
nodesToInternalize.Keys.Select(item => item.Identifier.Text);
project = await RemoveMethodsFromModelFactoryAsync(project, definitions, modelNamesToRemove.ToHashSet());
generatedProject = await RemoveMethodsFromModelFactoryAsync(generatedProject, definitions, modelNamesToRemove.ToHashSet());

return project;
return generatedProject;
}

private async Task<Project> RemoveMethodsFromModelFactoryAsync(Project project,
Expand Down Expand Up @@ -385,6 +385,7 @@ private async Task<Project> RemoveModelsFromDocumentAsync(Project project,
return document.Project;
}

// TODO -- here we should add the customization project in here as well otherwise this post processor will not know the existence of the customization code.
private async Task<HashSet<INamedTypeSymbol>> GetRootSymbolsAsync(Project project, TypeSymbols modelSymbols)
{
var result = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Formatting;

namespace Microsoft.Generator.CSharp.Tests.Common
{
Expand Down Expand Up @@ -43,8 +46,35 @@ public static async Task<Compilation> GetCompilationFromDirectoryAsync(
{
var directory = GetAssetFileOrDirectoryPath(false, parameters, method, filePath);
var codeGenAttributeFiles = Path.Combine(_assemblyLocation, "..", "..", "..", "..", "..", "Microsoft.Generator.CSharp.Customization", "src");
var workspace = GeneratedCodeWorkspace.CreateExistingCodeProject([directory, codeGenAttributeFiles], Path.Combine(directory, "Generated"));
return await workspace.GetCompilationAsync();
var project = CreateExistingCodeProject([directory, codeGenAttributeFiles], Path.Combine(directory, "Generated"));
var compilation = await project.GetCompilationAsync();
return compilation!;
Comment on lines +50 to +51
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: return it instead of declaring a variable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise it might look like this:

return (await project.GetCompilationAsync())!;

I prefer local variable rather than the above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean that we should add an assertion here on compilation cannot be null?
This method GetCompilationAsync is not our method, this method comes from roslyn API.
In our version of GetCompilationAsync, we have Debug.Assert(compilation is not null) and the method does not return a nullable result.

}

private static Project CreateExistingCodeProject(IEnumerable<string> projectDirectories, string generatedDirectory)
{
var workspace = new AdhocWorkspace();
var newOptionSet = workspace.Options.WithChangedOption(FormattingOptions.NewLine, LanguageNames.CSharp, "\n");
workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(newOptionSet));
Project project = workspace.AddProject("ExistingCode", LanguageNames.CSharp);

foreach (var projectDirectory in projectDirectories)
{
if (Path.IsPathRooted(projectDirectory))
{
project = GeneratedCodeWorkspace.AddDirectory(project, Path.GetFullPath(projectDirectory), skipPredicate: sourceFile => sourceFile.StartsWith(generatedDirectory));
}
}

project = project
.AddMetadataReferences([
MetadataReference.CreateFromFile(typeof(object).Assembly.Location),
..CodeModelPlugin.Instance.AdditionalMetadataReferences
])
.WithCompilationOptions(new CSharpCompilationOptions(
OutputKind.DynamicallyLinkedLibrary, metadataReferenceResolver: new WorkspaceMetadataReferenceResolver(), nullableContextOptions: NullableContextOptions.Disable));

return project;
}
}
}
Loading