Expose registered project file extensions - #85106
Conversation
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
4641429 to
288bc3c
Compare
288bc3c to
3bf7054
Compare
3bf7054 to
639cc89
Compare
639cc89 to
cccc725
Compare
cccc725 to
5e4077d
Compare
5e4077d to
c5022f7
Compare
c5022f7 to
9afef8b
Compare
9afef8b to
0c7bc7f
Compare
0c7bc7f to
602d180
Compare
602d180 to
1bcb8dc
Compare
1bcb8dc to
4e2613c
Compare
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Changes are small and low-risk, and the only feedback is minor clarity/documentation improvements rather than correctness issues.
Review tier: Lite
Findings: 3
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
src/Workspaces/MSBuild/Test/NetCoreTests.cs — This test change is specifically validating that leading-dot extensions are accepted/normalized.… |
Pre-existing issues (2)
| Severity | Finding |
|---|---|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — TryGetLanguageNameFromExtension() is a new query API but doesn’t document whether callers should… View comment |
|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — This change adds new query APIs (GetRegisteredProjectFileExtensions/TryGetLanguageNameFromExtension)… View comment |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — GetRegisteredProjectFileExtensions() returns dictionary key enumeration order, which is not… View resolved comment |
Suppressed comments (1)
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs:60
- TryGetLanguageNameFromExtension is a newly exposed query API but doesn’t document the accepted extension format (leading '.' allowed/ignored) or the intent that it matches Path.GetExtension inputs. Adding an XML doc comment here would help internal consumers avoid re-learning the normalization rules.
public bool TryGetLanguageNameFromExtension(string extension, [NotNullWhen(true)] out string? languageName)
{
72170b9 to
3462c3a
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The updated test drops coverage for the historically-supported non-dotted extension form and should validate both "cs" and ".cs" for backward compatibility.
Review tier: Lite
Findings: 3
Pre-existing issues (3)
| Severity | Finding |
|---|---|
src/Workspaces/MSBuild/Test/NetCoreTests.cs — This test change is specifically validating that leading-dot extensions are accepted/normalized.… View comment |
|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — TryGetLanguageNameFromExtension() is a new query API but doesn’t document whether callers should… View comment |
|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — This change adds new query APIs (GetRegisteredProjectFileExtensions/TryGetLanguageNameFromExtension)… View comment |
Suppressed comments (1)
src/Workspaces/MSBuild/Test/NetCoreTests.cs:753
- This test now only exercises the ".cs" form of AssociateFileExtensionWithLanguage(). Since the API historically accepted extensions without a leading dot, it would be good to cover both "cs" and ".cs" here to protect backward compatibility while also validating the new normalization behavior.
using var workspace = CreateMSBuildWorkspace();
workspace.AssociateFileExtensionWithLanguage(".cs", LanguageNames.CSharp);
await workspace.OpenProjectAsync(sourceFilePath);
// [Failure] Msbuild failed when processing the file 'Program.cs' with message:
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
WorkspaceFolderTracker now reads shared state without a synchronization/visibility mechanism, which can lead to stale workspace-folder sets being observed across threads.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
src/LanguageServer/Protocol/Handler/WorkspaceFolderTracker.cs — GetRequiredWorkspaceFolderPaths() now reads _workspaceFolderPaths without any synchronization, but… |
Pre-existing issues (3)
| Severity | Finding |
|---|---|
src/Workspaces/MSBuild/Test/NetCoreTests.cs — This test change is specifically validating that leading-dot extensions are accepted/normalized.… View comment |
|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — TryGetLanguageNameFromExtension() is a new query API but doesn’t document whether callers should… View comment |
|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — This change adds new query APIs (GetRegisteredProjectFileExtensions/TryGetLanguageNameFromExtension)… View comment |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
75eda8e to
ebe1514
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Extension normalization currently treats the single-dot string "." as an empty extension, which is a subtle behavioral change worth fixing or explicitly deciding on before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — RemoveLeadingDot currently treats the single-dot string "." as an empty extension (it returns… |
|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — This adds new query APIs (GetRegisteredProjectFileExtensions / TryGetLanguageNameFromExtension)… |
Pre-existing issues (3)
| Severity | Finding |
|---|---|
src/Workspaces/MSBuild/Test/NetCoreTests.cs — This test change is specifically validating that leading-dot extensions are accepted/normalized.… View comment |
|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — TryGetLanguageNameFromExtension() is a new query API but doesn’t document whether callers should… View comment |
|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — This change adds new query APIs (GetRegisteredProjectFileExtensions/TryGetLanguageNameFromExtension)… View comment |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
src/LanguageServer/Protocol/Handler/WorkspaceFolderTracker.cs — GetRequiredWorkspaceFolderPaths() now reads _workspaceFolderPaths without any synchronization, but… View resolved comment |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d6a35c4-0f49-4671-b352-8cac98e9eaa4
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
It introduces a couple of correctness/maintainability issues (notably an over-strong XML doc contract and a brittle test assertion) that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 6
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — The XML doc for GetRegisteredProjectFileExtensions() promises every returned extension has a… |
|
src/Workspaces/MSBuild/Test/NetCoreTests.cs — This test hard-codes the current number of default registered extensions (3), which makes it… |
Pre-existing issues (4)
| Severity | Finding |
|---|---|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — This adds new query APIs (GetRegisteredProjectFileExtensions / TryGetLanguageNameFromExtension)… View comment |
|
src/Workspaces/MSBuild/Test/NetCoreTests.cs — This test change is specifically validating that leading-dot extensions are accepted/normalized.… View comment |
|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — TryGetLanguageNameFromExtension() is a new query API but doesn’t document whether callers should… View comment |
|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — This change adds new query APIs (GetRegisteredProjectFileExtensions/TryGetLanguageNameFromExtension)… View comment |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
src/Workspaces/MSBuild/Core/MSBuild/ProjectFileExtensionRegistry.cs — RemoveLeadingDot currently treats the single-dot string "." as an empty extension (it returns… View resolved comment |
|
/azp run roslyn-CI |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
jasonmalinowski
left a comment
There was a problem hiding this comment.
Approved -- comments can be done in a mop-up.


Summary
Expose focused query APIs on
ProjectFileExtensionRegistryfor consumers that need to enumerate registered project extensions or resolve a language directly from an extension.Existing project-path lookup retains file-based application compatibility; this PR only separates reusable registry queries from that behavior.
Stack
Depends on Centralize Language Server workspace folder tracking.
Validation
Microsoft.CodeAnalysis.Workspaces.MSBuildbuild succeeded with 0 errors.Microsoft Reviewers: Open in CodeFlow