Use newer Roslyn APIs in the language service by drewnoakes · Pull Request #8499 · dotnet/project-system

drewnoakes · 2022-09-17T03:16:50Z

@tmat and I designed a new API for the interaction between the .NET Project System and Roslyn when creating instances of IWorkspaceProjectContext.

This new API:

Allows Roslyn to (generally) request additional properties from the project system, without requiring a corresponding change in the project system.
Avoids having to create a wasteful batched update immediately after constructing the workspace.

This PR updates the API we use.

Bumping that package pulled the thread on a whole bunch of other package upgrades, including analyzer changes that lead to .editorconfig changes, some naming guideline changes, and some suppressions.

Microsoft Reviewers: Open in CodeFlow

drewnoakes · 2022-09-17T03:17:44Z

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Workspace.cs

-            snapshot.Properties.TryGetValue(ConfigurationGeneral.TargetPathProperty, out string? binOutputPath);
            snapshot.Properties.TryGetValue(ConfigurationGeneral.MSBuildProjectFullPathProperty, out string? projectFilePath);
-            snapshot.Properties.TryGetValue(ConfigurationGeneral.AssemblyNameProperty, out string? assemblyName);
-            snapshot.Properties.TryGetValue(ConfigurationGeneral.CommandLineArgsForDesignTimeEvaluationProperty, out string? commandLineArgsForDesignTimeEvaluation);

-            if (string.IsNullOrEmpty(languageName) || string.IsNullOrEmpty(binOutputPath) || string.IsNullOrEmpty(projectFilePath))
+            if (string.IsNullOrEmpty(languageName) || string.IsNullOrEmpty(projectFilePath))


We no longer pull these properties out on our side. Roslyn will obtain them via an object we provide.

drewnoakes · 2022-09-17T03:18:18Z

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Workspace.cs


            try
            {
+                EvaluationDataAdapter evaluationData = new(snapshot.Properties);


This object allows Roslyn to call us back for any property that's in ConfigurationGeneral. In future we may also allow requesting arbitrary project properties.

drewnoakes · 2022-09-17T03:18:29Z

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Workspace.cs

                _context = await _workspaceProjectContextFactory.Value.CreateProjectContextAsync(
-                    languageName,
-                    _contextId,
-                    projectFilePath,
                    _projectGuid,
+                    _contextId,
+                    languageName,
+                    evaluationData,
                    hostObject,
-                    binOutputPath,
-                    assemblyName,
                    cancellationToken);


Calling the new overload.

drewnoakes · 2022-09-17T03:18:50Z

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Workspace.cs

-
-                // Update additional properties within a batch to avoid thread pool starvation.
-                // https://github.com/dotnet/project-system/issues/8027
-                _context.StartBatch();
-
-                try
-                {
-                    _context.LastDesignTimeBuildSucceeded = false; // By default, turn off diagnostics until the first design time build succeeds for this project.
-
-                    // Pass along any early approximation we have of the command line options
-#pragma warning disable CS0618 // This was obsoleted in favor of the one that takes an array, but here just the string is easier; we'll un-Obsolete this API
-                    _context.SetOptions(commandLineArgsForDesignTimeEvaluation ?? "");
-#pragma warning restore CS0618 // Type or member is obsolete
-                }
-                finally
-                {
-                    await _context.EndBatchAsync();
-                }


This whole post-construction update is no longer needed.

...S/ProjectSystem/VS/PropertyPages/Editors/ImplicitUsingsEditor/CSharpImplicitUsingsControl.cs

drewnoakes · 2022-09-17T03:20:28Z

...isualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Rename/FileMoveNotificationListener.cs

+#pragma warning disable CS0618 // Type or member is obsolete
                    Renamer.RenameDocumentActionSet documentAction = await Renamer.RenameDocumentAsync(oldDocument, null!, documentFolders);
+#pragma warning restore CS0618 // Type or member is obsolete


Roslyn has deprecated a bunch of these rename APIs.

@ocallesp do we have a work item tracking the update of this? If not, could you file one please?

I filed #8591

drewnoakes · 2022-09-17T03:21:50Z

I struggled making the package references and versions work here. It builds on my machine, though I half expect it to fail in CI. I'm not convinced the approach here is right, and it seems like some of the packages we need are incorrectly authored or missing from the feeds we source them from. I will revisit this next soon.

tmat · 2022-09-17T16:06:07Z

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Workspace.cs

-                    await _context.EndBatchAsync();
-                }
            }
            catch (Exception ex)


Exception

Roslyn throws InvalidProjectPropertyValueException if a property that is required isn't present or has invalid value, in case you want to handle it in a different way.

...Studio.ProjectSystem.Managed.VS/ProjectSystem/VS/PropertyPages/Editors/PropertyEditorBase.cs

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Workspace.cs

jacdavis

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Workspace.cs

This new API: - allows Roslyn to request additional properties in future, without requiring a corresponding change in the Project System. - avoids having to create a wasteful batched update immediately after constructing the workspace

Tracked in dotnet#8591

Historically, the set of project properties we passed to Roslyn when initialising an `IWorkspaceProjectContext` was hard-coded in the project system. We changed this recently in dotnet#8499, and introduced a means for Roslyn to request whatever properties they wanted, allowing Roslyn to make changes without requiring a synchronised change in the project system. With that change, we serviced property value requests using `ProjectRuleSource` data. This limited Roslyn to only those properties present in our `ConfigurationGeneral` XAML rule file. This commit uses a `ProjectInstance` to service such requests instead. This means that Roslyn can add new projects in their own props/targets, and not need the project system to make any changes in order to provide those values. Motivated by a bug exposed through dotnet/roslyn#64749 which added required property `OutputAssemblyForDesignTimeEvaluation` which did not exist in our rules, leading to runtime errors.

drewnoakes added Feature-Language-Service Populating the Roslyn workspace with references, source files, analyzers, etc Tenet-Performance This issue affects the "Performance" tenet. Performance-Scenario-Solution-Open This issue affects solution open performance. labels Sep 17, 2022

drewnoakes added this to the 17.4 milestone Sep 17, 2022

drewnoakes requested a review from a team as a code owner September 17, 2022 03:16

drewnoakes commented Sep 17, 2022

View reviewed changes

...S/ProjectSystem/VS/PropertyPages/Editors/ImplicitUsingsEditor/CSharpImplicitUsingsControl.cs Outdated Show resolved Hide resolved

drewnoakes commented Sep 17, 2022

View reviewed changes

tmat reviewed Sep 17, 2022

View reviewed changes

jacdavis reviewed Sep 19, 2022

View reviewed changes

...Studio.ProjectSystem.Managed.VS/ProjectSystem/VS/PropertyPages/Editors/PropertyEditorBase.cs Outdated Show resolved Hide resolved

jacdavis reviewed Sep 19, 2022

View reviewed changes

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Workspace.cs Show resolved Hide resolved

jacdavis approved these changes Sep 19, 2022

View reviewed changes

MiYanni approved these changes Sep 20, 2022

View reviewed changes

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Workspace.cs Outdated Show resolved Hide resolved

src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Workspace.cs Outdated Show resolved Hide resolved

drewnoakes force-pushed the update-lang-svc-api branch from c65ff00 to 4b49a49 Compare September 20, 2022 01:30

tmeschter mentioned this pull request Sep 23, 2022

Report a better exception message #8519

Closed

drewnoakes force-pushed the update-lang-svc-api branch 2 times, most recently from 66cb773 to bd63af1 Compare October 11, 2022 22:47

drewnoakes mentioned this pull request Oct 12, 2022

Replace use of obsolete Roslyn APIs for renaming code #8591

Closed

drewnoakes force-pushed the update-lang-svc-api branch 2 times, most recently from 9920029 to b9a7494 Compare October 12, 2022 01:59

drewnoakes added 6 commits October 13, 2022 15:15

Bump LanguageService

35cdf2c

Update packages in response to earlier Roslyn package update

e2deded

Fix namespace conflict following package updates

a54bcc6

Fix obsolete code warning

540fcd4

Suppress obsolete code warnings following package updates

ba901f0

Tracked in dotnet#8591

drewnoakes force-pushed the update-lang-svc-api branch from b9a7494 to ba901f0 Compare October 13, 2022 04:15

drewnoakes modified the milestones: 17.4, 17.5 Oct 13, 2022

drewnoakes merged commit f5a4241 into dotnet:main Oct 13, 2022

drewnoakes deleted the update-lang-svc-api branch October 13, 2022 09:55

tmat mentioned this pull request Oct 14, 2022

Read required project properties from evaluation data dotnet/roslyn#64749

Merged

drewnoakes mentioned this pull request Oct 19, 2022

Allow Roslyn to request arbitrary project properties #8622

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use newer Roslyn APIs in the language service#8499

Use newer Roslyn APIs in the language service#8499
drewnoakes merged 6 commits intodotnet:mainfrom
drewnoakes:update-lang-svc-api

drewnoakes commented Sep 17, 2022 •

edited by ghost

Loading

Uh oh!

drewnoakes Sep 17, 2022

Uh oh!

drewnoakes Sep 17, 2022

Uh oh!

drewnoakes Sep 17, 2022

Uh oh!

drewnoakes Sep 17, 2022

Uh oh!

Uh oh!

drewnoakes Sep 17, 2022

Uh oh!

drewnoakes Oct 12, 2022

Uh oh!

drewnoakes commented Sep 17, 2022

Uh oh!

tmat Sep 17, 2022

Uh oh!

Uh oh!

Uh oh!

jacdavis left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

drewnoakes commented Sep 17, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

drewnoakes Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

drewnoakes Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

drewnoakes Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

drewnoakes Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

drewnoakes Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

drewnoakes Oct 12, 2022

Choose a reason for hiding this comment

Uh oh!

drewnoakes commented Sep 17, 2022

Uh oh!

tmat Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jacdavis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

drewnoakes commented Sep 17, 2022 •

edited by ghost

Loading