Fix Pylance settings handling on Dev18 - #8587
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b6a821e-0a88-49fa-807e-874551b5a398
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b6a821e-0a88-49fa-807e-874551b5a398
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
🔒 Automated review in progress — Heejae Chang (@heejaechang) is auto-reviewing this PR. |
There was a problem hiding this comment.
Pull request overview
This PR fixes Dev18 (System.Text.Json-based) configuration/serialization mismatches in the PTVS ↔ Pylance boundary so Pylance actually receives persisted settings like “Automatically add brackets to functions” and can return snippet completions (e.g., getcwd($0)) as intended.
Changes:
- Updates Dev18 signature-help interception to use
JsonDocumentinstead ofJTokento avoid forcing a Newtonsoft formatter. - Fixes custom StreamJsonRpc handling to support whole-object parameter binding and serializer-neutral deserialization for
workspace/configurationand related messages. - Ensures Pylance settings serialize correctly under System.Text.Json by converting DTO fields to properties and nesting fallback config under
settings.python.
Show a summary per file
| File | Description |
|---|---|
| Python/Product/PythonTools/PythonTools/LanguageServerClient/PythonSignatureHelpMiddleLayer.cs | Switches Dev18 middle-layer contract to JsonDocument while preserving Dev17 JToken behavior. |
| Python/Product/PythonTools/PythonTools/LanguageServerClient/PythonLanguageClientCustomTarget.cs | Adjusts custom RPC handlers to use whole-object parameter binding and adds serializer-neutral payload deserialization. |
| Python/Product/PythonTools/PythonTools/LanguageServerClient/PythonLanguageClient.cs | Nests fallback didChangeConfiguration payload under settings.python to match Pylance expectations. |
| Python/Product/PythonTools/PythonTools/LanguageServerClient/LanguageServerSettings.cs | Converts outbound settings DTO members from fields to properties so STJ emits the expected JSON. |
| Python/Product/PythonTools/PythonTools.csproj | Adds a Dev18-conditional reference to System.Text.Json. |
| Build/18.0/packages.config | Adds System.Text.Json NuGet package for Dev18 build inputs. |
Review details
Comments suppressed due to low confidence (1)
Python/Product/PythonTools/PythonTools/LanguageServerClient/PythonLanguageClientCustomTarget.cs:179
- OnWorkspaceConfiguration no longer guards against deserialization failures. If Deserialize throws (malformed payload / unexpected formatter value), the RPC request will fail with an exception rather than safely returning null like the previous implementation.
public async Task<object> OnWorkspaceConfiguration(object arg) {
var reqParams = Deserialize<WorkspaceConfiguration.ConfigurationParams>(arg);
if (this.WorkspaceConfiguration != null && reqParams != null) {
var eventArgs = new ConfigurationArgs { requestParams = reqParams, requestResult = null };
await this.WorkspaceConfiguration.InvokeAsync(this, eventArgs);
return eventArgs.requestResult;
}
return null;
}
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Low
| // Special case language_server/analysis_complete. We need this for testing so we | ||
| // know when it's okay to try to bring up intellisense | ||
| if (telemetry.EventName == "language_server/analysis_complete") { | ||
| AnalysisComplete?.Invoke(this, EventArgs.Empty); |
| public void OnRegisterCapability(object arg) { | ||
| var regParams = Deserialize<VisualStudio.LanguageServer.Protocol.RegistrationParams>(arg); | ||
| if (regParams?.Registrations == null) { | ||
| return; | ||
| } | ||
|
|
||
| if (regParams.Registrations.Any(p => p.Method == "workspace/didChangeWorkspaceFolders")) { | ||
| _joinableTaskContext.Factory.RunAsync(async () => this.WorkspaceFolderChangeRegistered.Invoke(this, EventArgs.Empty)); | ||
| } | ||
| var watchedFilesReg = regParams.Registrations.FirstOrDefault(p => p.Method == "workspace/didChangeWatchedFiles"); | ||
| if (watchedFilesReg != null) { | ||
| var optionsObj = watchedFilesReg.RegisterOptions as JObject; | ||
| if (watchedFilesReg?.RegisterOptions is JObject optionsObj) { | ||
| var options = optionsObj.ToObject<DidChangeWatchedFilesRegistrationOptions>(); | ||
| if (options != null) { | ||
| _joinableTaskContext.Factory.RunAsync(async () => this.WatchedFilesRegistered.Invoke(this, options)); | ||
| } | ||
| } | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
There was a problem hiding this comment.
Review details
Comments suppressed due to low confidence (1)
Python/Product/PythonTools/PythonTools/LanguageServerClient/PythonLanguageClientCustomTarget.cs:123
- The AnalysisComplete invocation is missing indentation, which makes the block harder to read and looks like a formatting slip.
if (telemetry.EventName == "language_server/analysis_complete") {
AnalysisComplete?.Invoke(this, EventArgs.Empty);
}
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Low
| private static T Deserialize<T>(object arg) where T : class { | ||
| if (arg == null) { | ||
| return null; | ||
| } | ||
|
|
||
| // Dev18 supplies System.Text.Json values; older formatters supply JToken. | ||
| return arg is JToken token | ||
| ? token.ToObject<T>() | ||
| : JsonConvert.DeserializeObject<T>(arg.ToString()); | ||
| } |
Heejae Chang (heejaechang)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
Heejae Chang (heejaechang)
left a comment
There was a problem hiding this comment.
Approved via Review Center.



Summary
Fixes #8578.
Checking Automatically add brackets to functions persisted correctly in Visual Studio, but Dev18 did not deliver the enabled setting to Pylance. As a result, Pylance continued returning plain completion text such as
getcwdinstead of a snippet such asgetcwd($0).Root cause
Dev18 uses System.Text.Json for eligible language clients, exposing three serialization and binding mismatches in the PTVS/Pylance configuration path:
workspace/configurationparameters as one object ({ "items": [...] }), but the custom StreamJsonRpc handler did not opt into whole-object parameter binding, so the request was rejected with-32602.{}even when the option was enabled.workspace/didChangeConfigurationpayload placed analysis settings directly undersettings.analysis, while Pylance expects the section undersettings.python.analysis.Latest
mainalso introduced aJTokensignature-help middle layer. On Dev18, that type selects the Newtonsoft formatter for the whole language client, which conflicts with the STJ-compatible path needed for PTVS's initialize interception. The Dev18 implementation therefore usesJsonDocument, while the existing Dev17JTokenimplementation remains unchanged.Fix
workspace/configuration.pythonsection.Risk
Risk is low because the changes are limited to the Pylance language-client serialization and custom RPC boundary:
Validation
The fix was manually verified on Dev18: after enabling the option, function completions included parentheses as expected. No tests were added for this change.