From a2f897eb991b49d0ab5f8bc6bee4191ec1e4ea2f Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Tue, 5 Nov 2024 15:23:44 -0800 Subject: [PATCH 1/4] Signature help fix --- .../languageService/signatureHelpProvider.ts | 40 +++++++---- .../signature.dataclassAlias.fourslash.ts | 72 +++++++++++++++++++ 2 files changed, 99 insertions(+), 13 deletions(-) create mode 100644 packages/pyright-internal/src/tests/fourslash/signature.dataclassAlias.fourslash.ts diff --git a/packages/pyright-internal/src/languageService/signatureHelpProvider.ts b/packages/pyright-internal/src/languageService/signatureHelpProvider.ts index 2ca494ae8390..e39943780a8f 100644 --- a/packages/pyright-internal/src/languageService/signatureHelpProvider.ts +++ b/packages/pyright-internal/src/languageService/signatureHelpProvider.ts @@ -21,6 +21,7 @@ import { } from 'vscode-languageserver'; import { getFileInfo } from '../analyzer/analyzerNodeInfo'; +import { getParamListDetails } from '../analyzer/parameterUtils'; import * as ParseTreeUtils from '../analyzer/parseTreeUtils'; import { getCallNodeAndActiveParamIndex } from '../analyzer/parseTreeUtils'; import { SourceMapper } from '../analyzer/sourceMapper'; @@ -35,6 +36,7 @@ import { Position } from '../common/textRange'; import { Uri } from '../common/uri/uri'; import { CallNode, NameNode, ParseNodeType } from '../parser/parseNodes'; import { ParseFileResults } from '../parser/parser'; +import { Tokenizer } from '../parser/tokenizer'; import { getDocumentationPartsForTypeAndDecl, getFunctionDocStringFromType } from './tooltipUtils'; export class SignatureHelpProvider { @@ -232,8 +234,10 @@ export class SignatureHelpProvider { getFunctionDocStringFromType(functionType, this._sourceMapper, this._evaluator) ?? this._getDocStringFromCallNode(callNode); const fileInfo = getFileInfo(callNode); + const paramListDetails = getParamListDetails(functionType); let label = '('; + let isFirstParamInLabel = true; let activeParameter: number | undefined; const params = functionType.shared.parameters; @@ -245,21 +249,31 @@ export class SignatureHelpProvider { paramName = params[params.length - 1].name || ''; } - parameters.push({ - startOffset: label.length, - endOffset: label.length + paramString.length, - text: paramString, - }); + const isKeywordOnly = + paramListDetails.firstKeywordOnlyIndex !== undefined && + paramIndex >= paramListDetails.firstKeywordOnlyIndex; - // Name match for active parameter. The set of parameters from the function - // may not match the actual string output from the typeEvaluator (kwargs for TypedDict is an example). - if (paramName && signature.activeParam && signature.activeParam.name === paramName) { - activeParameter = paramIndex; - } + if (!isKeywordOnly || Tokenizer.isPythonIdentifier(paramName)) { + if (!isFirstParamInLabel) { + label += ', '; + } + isFirstParamInLabel = false; + + parameters.push({ + startOffset: label.length, + endOffset: label.length + paramString.length, + text: paramString, + }); + + const effectiveParamIndex = parameters.length - 1; + + // Name match for active parameter. The set of parameters from the function + // may not match the actual string output from the typeEvaluator (kwargs for TypedDict is an example). + if (paramName && signature.activeParam && signature.activeParam.name === paramName) { + activeParameter = effectiveParamIndex; + } - label += paramString; - if (paramIndex < stringParts[0].length - 1) { - label += ', '; + label += paramString; } }); diff --git a/packages/pyright-internal/src/tests/fourslash/signature.dataclassAlias.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/signature.dataclassAlias.fourslash.ts new file mode 100644 index 000000000000..abc1d7dbe6b1 --- /dev/null +++ b/packages/pyright-internal/src/tests/fourslash/signature.dataclassAlias.fourslash.ts @@ -0,0 +1,72 @@ +/// + +// @filename: test.py +//// from typing import Any, dataclass_transform +//// +//// def model_field(*, kw_only: bool = False, alias: str = "") -> Any: +//// ... +//// +//// @dataclass_transform(field_specifiers=(model_field,)) +//// class ModelBase: +//// ... +//// +//// class DC1(ModelBase): +//// before: int = model_field() +//// env: int = model_field(alias='Invalid Identifier') +//// +//// DC1([|/*dc1*/|]) +//// +//// class DC2(ModelBase): +//// before: int = model_field(kw_only=True) +//// env: int = model_field(kw_only=True, alias='Invalid Identifier') +//// +//// DC2([|/*dc2*/|]) +//// +//// class DC3(ModelBase): +//// before: int = model_field(kw_only=True) +//// env: int = model_field(kw_only=True, alias='Invalid Identifier') +//// after: int = model_field(kw_only=True) +//// +//// DC3([|/*dc3*/|]) +//// DC3(after=[|/*dc3_with_after*/|]) + +{ + helper.verifySignature('plaintext', { + dc1: { + signatures: [ + { + label: '(before: int, Invalid Identifier: int) -> DC1', + parameters: ['before: int', 'Invalid Identifier: int'], + }, + ], + activeParameters: [0], + }, + dc2: { + signatures: [ + { + label: '(*, before: int) -> DC2', + parameters: ['*', 'before: int'], + }, + ], + activeParameters: [undefined], + }, + dc3: { + signatures: [ + { + label: '(*, before: int, after: int) -> DC3', + parameters: ['*', 'before: int', 'after: int'], + }, + ], + activeParameters: [undefined], + }, + dc3_with_after: { + signatures: [ + { + label: '(*, before: int, after: int) -> DC3', + parameters: ['*', 'before: int', 'after: int'], + }, + ], + activeParameters: [2], + }, + }); +} From e841a28df640892af0371432dab4538b2632690a Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Tue, 5 Nov 2024 17:03:37 -0800 Subject: [PATCH 2/4] Cleanup --- .../src/languageService/signatureHelpProvider.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/pyright-internal/src/languageService/signatureHelpProvider.ts b/packages/pyright-internal/src/languageService/signatureHelpProvider.ts index e39943780a8f..a7de562a5dbf 100644 --- a/packages/pyright-internal/src/languageService/signatureHelpProvider.ts +++ b/packages/pyright-internal/src/languageService/signatureHelpProvider.ts @@ -265,12 +265,10 @@ export class SignatureHelpProvider { text: paramString, }); - const effectiveParamIndex = parameters.length - 1; - // Name match for active parameter. The set of parameters from the function // may not match the actual string output from the typeEvaluator (kwargs for TypedDict is an example). if (paramName && signature.activeParam && signature.activeParam.name === paramName) { - activeParameter = effectiveParamIndex; + activeParameter = parameters.length - 1; } label += paramString; From 04000c876962301068bee4b90072c40406cd9bf1 Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 6 Nov 2024 16:49:09 -0800 Subject: [PATCH 3/4] Don't use indices to identify keyword-only params --- .../src/languageService/signatureHelpProvider.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/pyright-internal/src/languageService/signatureHelpProvider.ts b/packages/pyright-internal/src/languageService/signatureHelpProvider.ts index a7de562a5dbf..ce54815885e7 100644 --- a/packages/pyright-internal/src/languageService/signatureHelpProvider.ts +++ b/packages/pyright-internal/src/languageService/signatureHelpProvider.ts @@ -21,7 +21,7 @@ import { } from 'vscode-languageserver'; import { getFileInfo } from '../analyzer/analyzerNodeInfo'; -import { getParamListDetails } from '../analyzer/parameterUtils'; +import { getParamListDetails, ParamKind } from '../analyzer/parameterUtils'; import * as ParseTreeUtils from '../analyzer/parseTreeUtils'; import { getCallNodeAndActiveParamIndex } from '../analyzer/parseTreeUtils'; import { SourceMapper } from '../analyzer/sourceMapper'; @@ -250,8 +250,7 @@ export class SignatureHelpProvider { } const isKeywordOnly = - paramListDetails.firstKeywordOnlyIndex !== undefined && - paramIndex >= paramListDetails.firstKeywordOnlyIndex; + paramListDetails.params.find((param) => param.param.name === paramName)?.kind === ParamKind.Keyword; if (!isKeywordOnly || Tokenizer.isPythonIdentifier(paramName)) { if (!isFirstParamInLabel) { From ccb475389079667f689458b39e6998dddcf313db Mon Sep 17 00:00:00 2001 From: Erik De Bonte Date: Wed, 6 Nov 2024 17:31:56 -0800 Subject: [PATCH 4/4] Take Eric's suggested keyword-only check --- .../src/languageService/signatureHelpProvider.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/pyright-internal/src/languageService/signatureHelpProvider.ts b/packages/pyright-internal/src/languageService/signatureHelpProvider.ts index ce54815885e7..92cf8547d132 100644 --- a/packages/pyright-internal/src/languageService/signatureHelpProvider.ts +++ b/packages/pyright-internal/src/languageService/signatureHelpProvider.ts @@ -249,8 +249,9 @@ export class SignatureHelpProvider { paramName = params[params.length - 1].name || ''; } - const isKeywordOnly = - paramListDetails.params.find((param) => param.param.name === paramName)?.kind === ParamKind.Keyword; + const isKeywordOnly = paramListDetails.params.some( + (param) => param.param.name === paramName && param.kind === ParamKind.Keyword + ); if (!isKeywordOnly || Tokenizer.isPythonIdentifier(paramName)) { if (!isFirstParamInLabel) {