[XSG] Refactor ProjectItem #31427
Merged
[XSG] Refactor ProjectItem #31427
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the ProjectItem class to eliminate the need for passing boolean flags (EnableLineInfo, EnableDiagnostics) and file path information repeatedly throughout the source generation pipeline. The main change converts ProjectItem from a record with mutable properties to a record with computed properties that derive their values from AnalyzerConfigOptions.
Key Changes
- Refactored ProjectItem constructor to take AnalyzerConfigOptions and compute properties dynamically
- Removed redundant property assignments in SourceGenContext creation
- Updated all file path references to use ProjectItem.RelativePath instead of context.FilePath
- Simplified the ProjectItem creation logic in GeneratorHelpers
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ProjectItem.cs | Complete refactor from mutable record to computed properties using AnalyzerConfigOptions |
| SourceGenContext.cs | Simplified constructor to include ProjectItem parameter and removed redundant properties |
| SetPropertiesVisitor.cs | Updated all file path and flag references to use ProjectItem properties |
| ExpandMarkupsVisitor.cs | Updated file path references to use ProjectItem.RelativePath |
| CreateValuesVisitor.cs | Updated file path references to use ProjectItem.RelativePath |
| NodeSGExtensions.cs | Updated file path and diagnostic flag references to use ProjectItem |
| KnownTypeConverters.cs | Updated file path references to use ProjectItem.RelativePath |
| KnownMarkups.cs | Updated file path references to use ProjectItem.RelativePath |
| InitializeComponentCodeWriter.cs | Simplified SourceGenContext creation by removing redundant property assignments |
| GeneratorHelpers.cs | Simplified ProjectItem creation logic using new constructor |
| CompiledBindingMarkup.cs | Updated file path reference to use ProjectItem.RelativePath |
Comment on lines
+39
to
46
| public static ProjectItem? ComputeProjectItem((AdditionalText additionalText, AnalyzerConfigOptionsProvider optionsProvider) tuple, CancellationToken cancellationToken) | ||
| { | ||
| var (additionalText, optionsProvider) = tuple; | ||
| var fileOptions = optionsProvider.GetOptions(additionalText); | ||
| if (!fileOptions.TryGetValue("build_metadata.additionalfiles.GenKind", out string? kind) || kind is null) | ||
| if (cancellationToken.IsCancellationRequested) | ||
| return null; | ||
|
|
||
| fileOptions.TryGetValue("build_metadata.additionalfiles.TargetPath", out var targetPath); | ||
| fileOptions.TryGetValue("build_metadata.additionalfiles.ManifestResourceName", out var manifestResourceName); | ||
| fileOptions.TryGetValue("build_metadata.additionalfiles.RelativePath", out var relativePath); | ||
| fileOptions.TryGetValue("build_property.targetframework", out var targetFramework); | ||
| fileOptions.TryGetValue("build_property.Configuration", out var configuration); | ||
|
|
||
| bool enableDiagnostics = false; | ||
| if (fileOptions.TryGetValue("build_property.EnableMauiXamlDiagnostics", out var enDiag) && string.Compare(enDiag, "true", StringComparison.OrdinalIgnoreCase) == 0) | ||
| enableDiagnostics = true; | ||
| if (fileOptions.TryGetValue("build_property.additionalfiles.EnableDiagnostics", out enDiag) && string.Compare(enDiag, "true", StringComparison.OrdinalIgnoreCase) == 0) | ||
| enableDiagnostics = true; | ||
| if (fileOptions.TryGetValue("build_property.additionalfiles.EnableDiagnostics", out enDiag) && string.Compare(enDiag, "false", StringComparison.OrdinalIgnoreCase) == 0) | ||
| enableDiagnostics = false; | ||
|
|
||
| var xamlinflator = 0; | ||
| if (fileOptions.TryGetValue("build_metadata.additionalfiles.Inflator", out var inflator) && !string.IsNullOrEmpty(inflator)) | ||
| { | ||
| var parts = inflator!.Split(','); | ||
| for (int i = 0; i < parts.Length; i++) | ||
| { | ||
| var trimmed = parts[i].Trim(); | ||
| if (!Enum.TryParse<XamlInflator>(trimmed, true, out var xinfl)) | ||
| throw new InvalidOperationException($"Invalid inflator '{trimmed}' for {additionalText.Path}."); | ||
| xamlinflator |= (int)xinfl; | ||
| } | ||
| } | ||
|
|
||
| var enableLineInfo = true; | ||
| if (fileOptions.TryGetValue("build_property.MauiXamlLineInfo", out var lineInfo) && string.Compare(lineInfo, "disable", StringComparison.OrdinalIgnoreCase) == 0) | ||
| enableLineInfo = false; | ||
| if (fileOptions.TryGetValue("build_metadata.additionalfiles.LineInfo", out lineInfo) && string.Compare(lineInfo, "enable", StringComparison.OrdinalIgnoreCase) == 0) | ||
| enableLineInfo = true; | ||
| if (fileOptions.TryGetValue("build_metadata.additionalfiles.LineInfo", out lineInfo) && string.Compare(lineInfo, "disable", StringComparison.OrdinalIgnoreCase) == 0) | ||
| enableLineInfo = false; | ||
|
|
||
| string noWarn = ""; | ||
| if (fileOptions.TryGetValue("build_property.MauiXamlNoWarn", out var noWarnValue)) | ||
| noWarn = noWarnValue; | ||
| if (fileOptions.TryGetValue("build_metadata.additionalfiles.NoWarn", out noWarnValue)) | ||
| noWarn = noWarnValue; | ||
|
|
||
| return new ProjectItem | ||
| { | ||
| AdditionalText = additionalText, | ||
| TargetPath = targetPath, | ||
| RelativePath = relativePath, | ||
| ManifestResourceName = manifestResourceName, | ||
| Kind = kind, | ||
| Inflator = (XamlInflator)xamlinflator, | ||
| EnableLineInfo = enableLineInfo, | ||
| EnableDiagnostics = enableDiagnostics, | ||
| NoWarn = noWarn, | ||
| TargetFramework = targetFramework, | ||
| Configuration = configuration!, | ||
| }; | ||
| var projectItem = new ProjectItem(tuple.additionalText, tuple.optionsProvider.GetOptions(tuple.additionalText)); | ||
| return projectItem.Kind == "None" ? null : projectItem; | ||
| } |
There was a problem hiding this comment.
The method parameter name tuple doesn't match the usage inside the method. Line 44 uses tuple.optionsProvider but the parameter is destructured as (AdditionalText additionalText, AnalyzerConfigOptionsProvider optionsProvider), so it should be optionsProvider.GetOptions(additionalText).
simonrozsival
previously approved these changes
Sep 2, 2025
avoid passing flags over and over
StephaneDelcroix
force-pushed
the
dev/stdelc/xsg/projitem
branch
from
353db45 to
12a0769
Compare
jfversluis
approved these changes
Sep 3, 2025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
avoid passing flags over and over