From 140f6e4e867801d9a860086135b6bfb50364de40 Mon Sep 17 00:00:00 2001
From: William Killerud <william@killerud.com>
Date: Sat, 28 Sep 2024 15:16:00 +0200
Subject: [PATCH] fix: signature help no longer flickers on and off

---
 .../__tests__/do-signature-help.test.ts       | 72 ++++++++++++++++++-
 .../src/features/do-signature-help.ts         | 40 ++++++++---
 2 files changed, 100 insertions(+), 12 deletions(-)

diff --git a/packages/language-services/src/features/__tests__/do-signature-help.test.ts b/packages/language-services/src/features/__tests__/do-signature-help.test.ts
index a078fc65..6d1b8fb1 100644
--- a/packages/language-services/src/features/__tests__/do-signature-help.test.ts
+++ b/packages/language-services/src/features/__tests__/do-signature-help.test.ts
@@ -311,7 +311,11 @@ test("signature help when given more parameters than are supported", async () =>
 	});
 	const result = await ls.doSignatureHelp(document, Position.create(0, 18));
 
-	assert.deepStrictEqual(result, null);
+	assert.deepStrictEqual(result, {
+		activeParameter: 2,
+		activeSignature: 0,
+		signatures: [],
+	});
 });
 
 test("is not confused by using a function as a parameter", async () => {
@@ -557,3 +561,69 @@ test("provides signature help for sass built-ins with named parameters in signat
 		activeSignature: 0,
 	});
 });
+
+test("provides signature help with numeric parameter", async () => {
+	const document = fileSystemProvider.createDocument(
+		[
+			"@use 'sass:color';",
+			".foo {",
+			"  $_primary: #303030;",
+			"  background-color: color.adjust($_primary, 100, 1);",
+			"}",
+		],
+		{ uri: "builtins.scss" },
+	);
+
+	const help = await ls.doSignatureHelp(document, Position.create(3, 50));
+
+	assert.deepStrictEqual(help, {
+		signatures: [
+			{
+				documentation: {
+					kind: "markdown",
+					value:
+						"Increases or decreases one or more properties of `$color` by fixed amounts. All optional arguments must be numbers.\n\nIt's an error to specify an RGB property at the same time as an HSL property, or either of those at the same time as an HWB property.\n\n[Sass reference](https://sass-lang.com/documentation/modules/color#adjust)",
+				},
+				label:
+					"adjust($color, $red: null, $green: null, $blue: null, $hue: null, $saturation: null, $lightness: null, $whiteness: null, $blackness: null, $alpha: null, $space: null)",
+				parameters: [
+					{
+						label: "$color",
+					},
+					{
+						label: "$red",
+					},
+					{
+						label: "$green",
+					},
+					{
+						label: "$blue",
+					},
+					{
+						label: "$hue",
+					},
+					{
+						label: "$saturation",
+					},
+					{
+						label: "$lightness",
+					},
+					{
+						label: "$whiteness",
+					},
+					{
+						label: "$blackness",
+					},
+					{
+						label: "$alpha",
+					},
+					{
+						label: "$space",
+					},
+				],
+			},
+		],
+		activeParameter: 2,
+		activeSignature: 0,
+	});
+});
diff --git a/packages/language-services/src/features/do-signature-help.ts b/packages/language-services/src/features/do-signature-help.ts
index 2b38f24b..628a732d 100644
--- a/packages/language-services/src/features/do-signature-help.ts
+++ b/packages/language-services/src/features/do-signature-help.ts
@@ -1,4 +1,4 @@
-import { getNodeAtOffset } from "@somesass/vscode-css-languageservice";
+import { getNodeAtOffset, Range } from "@somesass/vscode-css-languageservice";
 import { sassBuiltInModules } from "../facts/sass";
 import { LanguageFeature } from "../language-feature";
 import {
@@ -33,25 +33,39 @@ export class DoSignatureHelp extends LanguageFeature {
 			node.type !== NodeType.Function &&
 			node.type !== NodeType.MixinReference
 		) {
-			if (
-				!node.parent ||
-				(node.parent.type !== NodeType.Function &&
-					node.parent.type !== NodeType.MixinReference)
-			) {
+			const parent = node.findAParent(
+				NodeType.Function,
+				NodeType.MixinReference,
+			);
+			if (!parent) {
 				return null;
 			}
-
-			node = node.parent as Function | MixinReference;
+			node = parent as Function | MixinReference;
 		}
 
-		const result: SignatureHelp = {
+		const result: Required<SignatureHelp> = {
 			activeSignature: 0,
 			activeParameter: 0,
 			signatures: [],
 		};
 		const identifier = node.getIdentifier()!.getText();
 		const parameters = node.getArguments().getChildren();
-		result.activeParameter = parameters.length;
+		if (parameters.length) {
+			result.activeParameter = parameters.length - 1;
+
+			// Figure out how to se if we have a , after the last parameter. If so, add one to result.activeParameter.
+			const lastParamEndOffset = parameters[parameters.length - 1].end;
+			const lastParamEndPosition = document.positionAt(lastParamEndOffset);
+			const characterAfterLastParam = document.getText(
+				Range.create(lastParamEndPosition, {
+					line: lastParamEndPosition.line,
+					character: lastParamEndPosition.character + 1,
+				}),
+			);
+			if (characterAfterLastParam === ",") {
+				result.activeParameter = result.activeParameter + 1;
+			}
+		}
 
 		const definition = await this.ls.findDefinition(
 			document,
@@ -63,7 +77,11 @@ export class DoSignatureHelp extends LanguageFeature {
 			if (!symbol) return result;
 
 			const allParameters = getParametersFromDetail(symbol.detail);
-			if (allParameters.length >= (result.activeParameter || 0)) {
+			// activeParameter is 0 index
+			if (
+				allParameters.length === 0 ||
+				allParameters.length > result.activeParameter
+			) {
 				const signatureInfo = SignatureInformation.create(
 					`${identifier}${symbol.detail || "()"}`,
 				);