Sync FBA sources - #85065
Conversation
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR syncs updated File Based App (FBA) source logic into Roslyn’s C# Workspaces “SyncedSource” area, expanding file-level #: directive parsing (quoting + metadata) and updating the virtual project generator to carry that richer directive data into generated MSBuild project files.
Changes:
- Extend directive parsing to support quoted values (with C# string-literal escapes) and trailing
Name=Valuemetadata, plus new/updated diagnostics. - Update virtual project emission to use
FileBasedAppArtifactsPathand to write directive-provided metadata ontoPackageReference/ProjectReferenceitems. - Add shared low-level helpers for directive token name validation and value quoting.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/VirtualProjectBuilder.cs | Updates virtual project generation (artifacts property + item metadata emission). |
| src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/FileLevelDirectiveHelpers.cs | Major directive parsing/tokenization changes (quoting, metadata parsing, new diagnostics). |
| src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/FileBasedProgramsResources.resx | Adds/removes resource strings to support new directive diagnostics. |
| src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/FileBasedProgramDirectiveValueHelpers.cs | New shared helper primitives for directive value/name processing. |
| src/Workspaces/CSharp/Portable/SyncedSource/commitid.txt | Advances the synced source commit ID. |
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (14)
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.de.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.fr.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.zh-Hans.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.zh-Hant.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.ja.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.it.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.ko.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.pl.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.pt-BR.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.tr.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.es.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.cs.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/xlf/FileBasedProgramsResources.ru.xlf:108
- The {Locked="…"} note contains unescaped double quotes (e.g.
Name="a b"), which makes the lock annotation ambiguous and inconsistent with the RESX comment form. Use XML escaping (") so localization tooling can reliably parse the locked substrings.
<note>{Locked="Name="a b""}{Locked=""a b""}</note>
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/FileLevelDirectiveHelpers.cs:432
- This change adds substantial new directive parsing behavior (quoted tokens, string-literal escaping, and trailing item metadata) but the repo doesn’t appear to have any tests exercising these new cases. Please add targeted coverage (e.g., in FileBasedProgramsWorkspaceTests) for: quoted values with whitespace, invalid/mismatched quotes, invalid escape sequences, and metadata emission into the generated project file.
private static ImmutableArray<DirectiveToken>? Tokenize(in ParseContext context, char? nameSeparator, bool allowMetadata)
{
var text = context.DirectiveText;
var tokens = ImmutableArray.CreateBuilder<DirectiveToken>();
var current = new StringBuilder();
|
Does this need to be updated or anything once dotnet/sdk#55970 is merged? |
I will update the commit hash to point to the actually merged commit instead of the commit from the PR, but otherwise no changes should be needed (the sources should remain the same). |
There was a problem hiding this comment.
🟡 Changes recommended
The new XLF lock note for InvalidQuoteInDirective appears malformed, and the newly introduced directive quoting/metadata parsing paths lack corresponding test coverage in this repo.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/Workspaces/CSharp/Portable/SyncedSource/FileBasedPrograms/FileLevelDirectiveHelpers.cs:628
- This PR introduces substantial new directive parsing behavior (quoted tokens, C# string-literal lexing, and trailing Name=Value metadata). There don't appear to be repo tests covering these new cases (valid quoting, invalid quote placement/unterminated quotes, raw/verbatim literal rejection, duplicate/conflicting metadata, invalid metadata names), so regressions here would be hard to catch.
private static ImmutableArray<DirectiveToken>? TokenizeWithLegacyFallback(
in ParseContext context,
char? nameSeparator,
bool allowMetadata,
out bool isLegacy)
- Files reviewed: 18/18 changed files
- Comments generated: 1
- Review effort level: Lite
Brings in:
:in property directives sdk#55671Microsoft Reviewers: Open in CodeFlow