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

Remove dependency from on LanguageClient on LanguageServer #10096

Closed
2 tasks done
Tracked by #9806
DustinCampbell opened this issue Mar 13, 2024 · 0 comments · Fixed by #10183
Closed
2 tasks done
Tracked by #9806

Remove dependency from on LanguageClient on LanguageServer #10096

DustinCampbell opened this issue Mar 13, 2024 · 0 comments · Fixed by #10183
Assignees
Milestone

Comments

@DustinCampbell
Copy link
Member

DustinCampbell commented Mar 13, 2024

MIcrosoft.VisualStudio.LanguageServerClient.Razor has a dependency on MIcrosoft.AspNetCore.Razor.LanguageServer. This needs to be removed, which will require refactoring code to push it into the shared Workspaces layer.

  • Find or create analyzer to only allow hosting types in MS.ANC.Razor.LanguageServer to be referenced externally.
  • Use analyzer to break dependencies on disallowed types.
@DustinCampbell DustinCampbell self-assigned this Mar 13, 2024
@phil-allen-msft phil-allen-msft added this to the 17.11 P1 milestone Mar 28, 2024
DustinCampbell added a commit that referenced this issue Apr 1, 2024
Fixes #10096

`Microsoft.VisualStudio.Razor.LanguageServerClient` has a reference to
`Microsoft.AspNetCore.Razor.LanguageServer` along with
`InternalsVisibleTo` access. This is unavoidable because we currently
run the LanguageServer in-proc in VS when the LSP editor is active.
However, over time `LanguageServerClient` has relied on more and more
code in `LanguageServer`, which can cause `MS.ANC.Razor.LanguageServer`
to be loaded in VS when it shouldn't be. This change addresses this
issue by limiting the types that `LanguageServerClient` has access to.

Access is restricted using the `RestrictedInternalsVisibleToAttribute`,
which is already supported by
`Microsoft.CodeAnalysis.CSharp.BannedApiAnalyzers`. Other than adding
our own copy of this attribute and updating the `AssemblyInfo.cs` for
`MS.ANC.Razor.LanguageServer`, this change is entirely mechanical.
Essentially, I've just moved types around, and those types fall into two
categories:

1. Types that are needed to host the language server. These have been
moved to a `Microsoft.AspNetCore.Razor.LanguageServer.Hosting`
namespace, and IVT is restricted to that namespace.
2. Protocol types that are needed by both the client and server. These
have been moved to `MS.CA.Razor.Workspaces` and organized in a
`Protocol` folder.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants