Move code off of HostLanguageServices in favor of HostProjectServices.#62949
Move code off of HostLanguageServices in favor of HostProjectServices.#62949CyrusNajmabadi merged 16 commits intodotnet:mainfrom
Conversation
jasonmalinowski
left a comment
There was a problem hiding this comment.
As much as I am saddened by having all the fields/properties still called language services...I'm not sure I want to review that diff.
| { | ||
| #if CODE_STYLE | ||
| // Remove once Solution.Services becomes public: https://github.com/dotnet/roslyn/issues/62914 | ||
| #pragma warning disable RS0030 // Do not used banned APIs. This is the shim method to use until the api becomes public. |
There was a problem hiding this comment.
Might be worth clarifying this comment a bit more -- not entirely sure what you mean by "the API' here.
There was a problem hiding this comment.
sure. can do.
| SnapshotSpan selectionSpan) | ||
| { | ||
| var syntaxKinds = document.LanguageServices.GetRequiredService<ISyntaxKindsService>(); | ||
| var syntaxKinds = document.ProjectServices.GetRequiredService<ISyntaxKindsService>(); |
There was a problem hiding this comment.
We have both document.ProjectServices and document.Project.Services? Do we really need both? Is there a shortage of .s these days?
There was a problem hiding this comment.
so this is not a normal document. this is the internal ParsedDocument helper guy. So this is how that guy bridges to this new api.
| { | ||
| var document = text.GetOpenDocumentInCurrentContextWithChanges(); | ||
| var languageServices = document?.Project.LanguageServices ?? _services.GetLanguageServices(Language); | ||
| var languageServices = document?.Project.Services ?? _services.GetLanguageServices(Language).ProjectServices; |
There was a problem hiding this comment.
Renaming the local or no?
There was a problem hiding this comment.
generally no. because it's just going to make these even large. and, frankly, 'language services' isn't a name that bothers me. it still is the thing you ask for for ILanguageService's.
| new("FormattingOptions", "WrappingColumn", CodeActionOptions.DefaultWrappingColumn); | ||
|
|
||
| public static CodeActionOptions GetCodeActionOptions(this IGlobalOptionService globalOptions, HostLanguageServices languageServices) | ||
| public static CodeActionOptions GetCodeActionOptions(this IGlobalOptionService globalOptions, HostProjectServices languageServices) |
| // We have different suppression fixers for every language. | ||
| // So we need to group diagnostics by the containing project language and apply fixes separately. | ||
| var languageServices = new HashSet<HostLanguageServices>(projectDiagnosticsToFixMap.Select(p => p.Key.LanguageServices).Concat(documentDiagnosticsToFixMap.Select(kvp => kvp.Key.Project.LanguageServices))); | ||
| var languageServices = projectDiagnosticsToFixMap.Select(p => p.Key.Services).Concat(documentDiagnosticsToFixMap.Select(kvp => kvp.Key.Project.Services)).ToHashSet(); |
There was a problem hiding this comment.
Should this just be using language name here? This is really funky we're using the language service type this way?
There was a problem hiding this comment.
no disagreement. but would like to keep mechanical for now.
Followup to #62947.