From 7b2afdba0e613a8af7bce3e6e5f478217a258892 Mon Sep 17 00:00:00 2001 From: egamma Date: Tue, 7 Jul 2026 17:09:43 +0200 Subject: [PATCH] fix #246019: lsp signature_helper Activity parameter highlight position error --- .../browser/parameterHintsWidget.ts | 58 ++++++--- .../test/browser/parameterHintsWidget.test.ts | 120 ++++++++++++++++++ 2 files changed, 163 insertions(+), 15 deletions(-) create mode 100644 src/vs/editor/contrib/parameterHints/test/browser/parameterHintsWidget.test.ts diff --git a/src/vs/editor/contrib/parameterHints/browser/parameterHintsWidget.ts b/src/vs/editor/contrib/parameterHints/browser/parameterHintsWidget.ts index 15af4b01e4d65..345da701b3ea4 100644 --- a/src/vs/editor/contrib/parameterHints/browser/parameterHintsWidget.ts +++ b/src/vs/editor/contrib/parameterHints/browser/parameterHintsWidget.ts @@ -311,21 +311,7 @@ export class ParameterHintsWidget extends Disposable implements IContentWidget { } private getParameterLabelOffsets(signature: languages.SignatureInformation, paramIdx: number): [number, number] { - const param = signature.parameters[paramIdx]; - if (!param) { - return [0, 0]; - } else if (Array.isArray(param.label)) { - return param.label; - } else if (!param.label.length) { - return [0, 0]; - } else { - const regex = new RegExp(`(\\W|^)${escapeRegExpCharacters(param.label)}(?=\\W|$)`, 'g'); - regex.test(signature.label); - const idx = regex.lastIndex - param.label.length; - return idx >= 0 - ? [idx, regex.lastIndex] - : [0, 0]; - } + return getParameterLabelOffsets(signature, paramIdx); } next(): void { @@ -364,4 +350,46 @@ export class ParameterHintsWidget extends Disposable implements IContentWidget { } } +/** + * Computes the [start, end) offsets of the parameter label within the signature label string. + * Accounts for earlier parameters so that duplicate names are resolved to the correct occurrence. + */ +export function getParameterLabelOffsets(signature: languages.SignatureInformation, paramIdx: number): [number, number] { + const param = signature.parameters[paramIdx]; + if (!param) { + return [0, 0]; + } else if (Array.isArray(param.label)) { + return param.label; + } else if (!param.label.length) { + return [0, 0]; + } else { + // Determine a search start offset by finding the end of the previous parameter's position. + // This prevents matching a parameter name that appears in an earlier part of the label. + let searchStart = 0; + for (let i = 0; i < paramIdx; i++) { + const prev = signature.parameters[i]; + if (!prev) { + break; + } + if (Array.isArray(prev.label)) { + searchStart = Math.max(searchStart, prev.label[1]); + } else if (prev.label.length) { + const prevRegex = new RegExp(`(\\W|^)${escapeRegExpCharacters(prev.label)}(?=\\W|$)`, 'g'); + prevRegex.lastIndex = searchStart; + if (prevRegex.test(signature.label)) { + searchStart = prevRegex.lastIndex; + } + } + } + + const regex = new RegExp(`(\\W|^)${escapeRegExpCharacters(param.label)}(?=\\W|$)`, 'g'); + regex.lastIndex = searchStart; + regex.test(signature.label); + const idx = regex.lastIndex - param.label.length; + return idx >= searchStart + ? [idx, regex.lastIndex] + : [0, 0]; + } +} + registerColor('editorHoverWidget.highlightForeground', listHighlightForeground, nls.localize('editorHoverWidgetHighlightForeground', 'Foreground color of the active item in the parameter hint.')); diff --git a/src/vs/editor/contrib/parameterHints/test/browser/parameterHintsWidget.test.ts b/src/vs/editor/contrib/parameterHints/test/browser/parameterHintsWidget.test.ts new file mode 100644 index 0000000000000..cc6606e37e6ac --- /dev/null +++ b/src/vs/editor/contrib/parameterHints/test/browser/parameterHintsWidget.test.ts @@ -0,0 +1,120 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import * as languages from '../../../../common/languages.js'; +import { getParameterLabelOffsets } from '../../browser/parameterHintsWidget.js'; + +suite('ParameterHintsWidget - getParameterLabelOffsets', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + test('should return correct offsets for simple parameter', () => { + const signature: languages.SignatureInformation = { + label: 'foo(x, y)', + parameters: [ + { label: 'x', documentation: '' }, + { label: 'y', documentation: '' }, + ] + }; + + assert.deepStrictEqual(getParameterLabelOffsets(signature, 0), [4, 5]); + assert.deepStrictEqual(getParameterLabelOffsets(signature, 1), [7, 8]); + }); + + test('should return tuple label as-is', () => { + const signature: languages.SignatureInformation = { + label: 'foo(x, y)', + parameters: [ + { label: [4, 5] as [number, number], documentation: '' }, + { label: [7, 8] as [number, number], documentation: '' }, + ] + }; + + assert.deepStrictEqual(getParameterLabelOffsets(signature, 0), [4, 5]); + assert.deepStrictEqual(getParameterLabelOffsets(signature, 1), [7, 8]); + }); + + test('should highlight correct occurrence when parameter name appears earlier in signature', () => { + // "len" appears in "strlen" and also as a parameter name + const signature: languages.SignatureInformation = { + label: 'strlen(len, len)', + parameters: [ + { label: 'len', documentation: '' }, + { label: 'len', documentation: '' }, + ] + }; + + // First "len" parameter should match at position 7-10 + assert.deepStrictEqual(getParameterLabelOffsets(signature, 0), [7, 10]); + // Second "len" parameter should match at position 12-15 + assert.deepStrictEqual(getParameterLabelOffsets(signature, 1), [12, 15]); + }); + + test('should highlight correct parameter when name is substring of function name', () => { + // Issue #246019: "len" appears in function name "strlen" + const signature: languages.SignatureInformation = { + label: 'strlen(str, len)', + parameters: [ + { label: 'str', documentation: '' }, + { label: 'len', documentation: '' }, + ] + }; + + assert.deepStrictEqual(getParameterLabelOffsets(signature, 0), [7, 10]); + assert.deepStrictEqual(getParameterLabelOffsets(signature, 1), [12, 15]); + }); + + test('should handle duplicate parameter names correctly', () => { + const signature: languages.SignatureInformation = { + label: 'func(a, a, a)', + parameters: [ + { label: 'a', documentation: '' }, + { label: 'a', documentation: '' }, + { label: 'a', documentation: '' }, + ] + }; + + assert.deepStrictEqual(getParameterLabelOffsets(signature, 0), [5, 6]); + assert.deepStrictEqual(getParameterLabelOffsets(signature, 1), [8, 9]); + assert.deepStrictEqual(getParameterLabelOffsets(signature, 2), [11, 12]); + }); + + test('should return [0, 0] for missing parameter', () => { + const signature: languages.SignatureInformation = { + label: 'foo(x)', + parameters: [ + { label: 'x', documentation: '' }, + ] + }; + + assert.deepStrictEqual(getParameterLabelOffsets(signature, 5), [0, 0]); + }); + + test('should return [0, 0] for empty parameter label', () => { + const signature: languages.SignatureInformation = { + label: 'foo(x)', + parameters: [ + { label: '', documentation: '' }, + ] + }; + + assert.deepStrictEqual(getParameterLabelOffsets(signature, 0), [0, 0]); + }); + + test('should handle mixed tuple and string labels', () => { + const signature: languages.SignatureInformation = { + label: 'foo(len, len)', + parameters: [ + { label: [4, 7] as [number, number], documentation: '' }, + { label: 'len', documentation: '' }, + ] + }; + + // Second parameter should find "len" after position 7 + assert.deepStrictEqual(getParameterLabelOffsets(signature, 1), [9, 12]); + }); +});