Support source-generated files #193

jjonescz · 2025-08-16T19:14:27Z

Fixes missing source generated code. For example, go to AliasAndExternAliasDirective and observe missing link to ExternAliasDirectiveSyntax because that class is entirely source-generated, etc. See also https://source.dot.net/Microsoft.CodeAnalysis.CSharp/diagnostics.txt for a sample list of errors caused by source generated files missing.

Outdated info about commit 1

This fix includes the source generator DLLs to be packed in stage1output via BinLogToSln. That means they are loaded in the second stage. I guess that might cause errors (shouldn't be blocking, like no errors are during source indexing) when the Roslyn referenced by a source generator is newer (e.g., 5.0.0) than the Roslyn referenced by source indexer (e.g., 4.14.0). An alternative might be to just emit the source-generated files during the first stage (via something like complog generated) but that seems more complicated and probably wouldn't prevent the same errors happening in stage1 (which runs as a separate job in official builds). (We should probably just replace BinLogToSln with complog anyway, but that's larger work.)

This fix includes the source generated files in the BinLogToSln stage by extracting them from the PDB.

jjonescz · 2025-08-18T11:41:15Z

@jaredpar @ericstj for reviews, thanks

src/SourceBrowser/src/BinLogToSln/Program.cs

jaredpar

Comment about analyzer tearing

jjonescz · 2025-08-19T13:23:10Z

src/SourceBrowser/src/BinLogToSln/Program.cs

+                }
+
+#nullable enable
+                // From https://github.com/jaredpar/complog/blob/a629fb3c05e40ebe673410144e8911bd5f86bdf2/src/Basic.CompilerLog.Util/RoslynUtil.cs#L440.


📝 I've also considered using Basic.CompilerLog.Util directly here, but currently BinLogToSln is using more information (properties of the MSBuild evaluated project) than the complog library makes available.

What all properties are being used here?

Also should we just make that method in CompilerLog public?

What all properties are being used here?

UseForSourceIndex, IsPlatformNotSupportedAssembly, TargetFramework. But I guess it doesn't matter which specific properties are being used, it would be best to have the complog util expose all project properties like the BinLogParser utility in this repo does:

source-indexer/src/SourceBrowser/src/BinLogParser/BinLogReader.cs

Line 213 in 07dd0cc

ProjectProperties = project?.GetEvaluation(build)?.GetProperties() ?? new Dictionary<string, string>(),

Also should we just make that method in CompilerLog public?

Good idea. Here's a PR: jaredpar/complog#261

I had to add two implementations of this, one for the in-memory model and a different one for the replay. Replay was a bit trickier, but necessary since we use that to be able to handle large binlogs.

I added open-ended support for project properties because a) I wanted to learn b) I thought it might be useful in the future. Probably the information we need could be deduced from other things (like attributes on the assembly).

I see that the complog tool is already doing it's own sort of Replay implementation and already capturing some properties -- https://github.com/jaredpar/complog/blob/a629fb3c05e40ebe673410144e8911bd5f86bdf2/src/Basic.CompilerLog.Util/BinaryLogUtil.cs#L164

So probably you'd want to port logic similar to what I added here. We could capture all the properties, if small enough, or just those that the caller wants to preserve. I bet all the properties is small enough compared to the overall size of these.

jaredpar

Feedback

src/SourceBrowser/src/BinLogToSln/Program.cs

jaredpar · 2025-08-21T05:55:14Z

src/SourceBrowser/src/BinLogToSln/Program.cs

+                }
+
+#nullable enable
+                // From https://github.com/jaredpar/complog/blob/a629fb3c05e40ebe673410144e8911bd5f86bdf2/src/Basic.CompilerLog.Util/RoslynUtil.cs#L440.


What all properties are being used here?

Also should we just make that method in CompilerLog public?

ericstj · 2025-08-22T17:57:07Z

src/SourceBrowser/src/BinLogToSln/Program.cs

-                    string refPath = DedupeReference(output, path);
-                    project.WriteLine($"    <ReferencePath Include=\"{Path.Join(projToOutputPath, refPath)}\"/>");
+                    includeFile(filePath, out string projectRelativePath, out string link);
+                    project.WriteLine($"    <Compile Include=\"{projectRelativePath}\"{(link != null ? $" Link=\"{link}\"" : "")}/>");


Do all callers to includeFile need to write the item? If so, maybe we can move that into the function like it is for includeReference?

No, only this one. All the other callers work with existing files.

Probably we're looking at different things. I saw the same write for Compile item on line 287 - I guess you still have KeyOriginatorFile. Consider adding includeCompile to be consistent with includeReference.

You're right, I've misunderstood, thanks.

src/SourceBrowser/src/BinLogToSln/Program.cs

jjonescz · 2025-08-26T08:04:05Z

@ericstj can you merge this if it looks good to you? Thanks!

ericstj · 2025-08-26T15:26:45Z

src/SourceBrowser/src/BinLogToSln/Program.cs

                project.WriteLine("    <GenerateAssemblyInfo>false</GenerateAssemblyInfo>");
                project.WriteLine("    <GenerateTargetFrameworkAttribute>false</GenerateTargetFrameworkAttribute>");
                project.WriteLine("    <DisableImplicitFrameworkReferences>true</DisableImplicitFrameworkReferences>");
+                project.WriteLine("    <_SkipAnalyzers>true</_SkipAnalyzers>"); // we only need source generators


is this comment still correct? Is it skipping built in analyzers? Maybe we could use some public property to do that?

I don't think there is a public property for this, and yes, I believe we should still set this to avoid running analyzers unnecessarily, but you're right the comment is outdated - we don't need even source generators now (but there is no simple property to set to exclude them).

ericstj · 2025-08-26T15:26:52Z

src/SourceBrowser/src/Directory.Packages.props

    <PackageVersion Include="Shouldly" Version="4.3.0" />

    <!-- other dependencies -->
+    <PackageVersion Include="Basic.CompilerLog.Util" Version="0.9.17" />


Can you please follow up and make sure https://www.nuget.org/packages/Basic.CompilerLog.Util/ gets a license and source repository added to it's NuGet package? You might even get a component governance alert from this after merging into official build.

Hm, I can, but I believe this is already used in other repos, e.g., in sdk (although perhaps only in tests there).

Most repos don't build tests during official builds - official builds are what require CG to pass.

You can check out https://aka.ms/opensource to see what requirements we have for using new dependencies in OSS. I don't object to this dependency, but it would be good to make sure we're checking all the right boxes when taking such a dependency for tools which run in official builds.

Is it fine to follow up separately or is this blocking merge of the PR?

jjonescz added 2 commits August 16, 2025 21:14

Support source-generated files

ae2f6d7

Fix message

73cea20

jaredpar reviewed Aug 18, 2025

View reviewed changes

src/SourceBrowser/src/BinLogToSln/Program.cs Outdated Show resolved Hide resolved

jaredpar reviewed Aug 18, 2025

View reviewed changes

ericstj requested a review from joperezr August 18, 2025 23:28

Include source-generated files in stage 1

a849cd2

jjonescz commented Aug 19, 2025

View reviewed changes

jaredpar reviewed Aug 19, 2025

View reviewed changes

src/SourceBrowser/src/BinLogToSln/Program.cs Outdated Show resolved Hide resolved

jjonescz requested a review from jaredpar August 20, 2025 08:18

jaredpar approved these changes Aug 21, 2025

View reviewed changes

jjonescz mentioned this pull request Aug 21, 2025

Make ReadGeneratedFilesFromPdb public jaredpar/complog#261

Merged

ericstj reviewed Aug 22, 2025

View reviewed changes

src/SourceBrowser/src/BinLogToSln/Program.cs Show resolved Hide resolved

ericstj approved these changes Aug 22, 2025

View reviewed changes

Use complog util

5f54950

ericstj reviewed Aug 26, 2025

View reviewed changes

jjonescz added 2 commits August 26, 2025 17:35

Include <Compile> by default

6c9f334

Remove outdated comment

4f4d097

jjonescz merged commit c4876a5 into dotnet:main Sep 5, 2025
2 checks passed

jjonescz deleted the sg branch September 5, 2025 13:46

This was referenced Sep 8, 2025

Update source-indexer tools dotnet/arcade#16114

Merged

[main] Update dependencies from dotnet/arcade dotnet/roslyn#80076

Merged

Support source-generated files #193

Support source-generated files #193

Uh oh!

Conversation

jjonescz commented Aug 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jjonescz commented Aug 18, 2025

Uh oh!

Uh oh!

jaredpar left a comment

Choose a reason for hiding this comment

Uh oh!

jjonescz Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jjonescz Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaredpar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jjonescz commented Aug 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jjonescz commented Aug 16, 2025 •

edited

Loading

jjonescz Aug 19, 2025 •

edited

Loading

jjonescz Aug 21, 2025 •

edited

Loading