Repository: stylelint/vscode-stylelint (ts)

Issue:
Server only returns `FixAll` actions even when client requests both `FixAll` & `QuickFixes`
> [!WARNING]  
> I've created a Zed extension that relies on the server from this repo. What I'm reporting as a "bug" here does not exist with VSCode but I assume the server should be more "client agnostic" (which is debatable since this project is initially for VSCode).
If you don't agree with that, feel free to close this issue & the associated PR, I'll handle it in a fork.

This Issue can be seen as a bug report or a feature request because it aims at changing the behavior documented here: https://github.com/stylelint/vscode-stylelint/blob/main/src/server/modules/code-action.ts#L215.

## The issue

- When the requested actions include FixAll, the server ignores other requested actions and only returns the FixAll action,
   - The part of the source code responsible for this behavior is here: https://github.com/stylelint/vscode-stylelint/blob/main/src/server/modules/code-action.ts#L215.
- It does match the VSCode behavior because quickfixes and source / document actions are separate features, triggered separately,
- But some clients (like Zed), send requests for both Quickfixes and source / document actions (FixAll) within the same `only` code actions array.

## The suggested change

- Let the server return all the requested code action types (fix all, quickfixes) at the same time because it's up to the client to decide what it needs.
- This shouldn't impact VSCode because quickfixes and fix all are requested separately (the `only` array just includes `fixAll` or `quickfixes` but not both at the same time (I'm not sure, I just tested this theory quickly, I could definitely be wrong about that)

Pull request description:
fix: return `FixAll` actions as well as quickfixes
> [!WARNING]  
> I've created a Zed extension that relies on the server from this repo. Everything works fine except quickfixes.
What I'm reporting as a "bug" here does not exist with VSCode but I assume the server should be more "client agnostic".
This is debatable and it's not up to me to decide of course. Maybe you want to keep the server coupled to VSCode, which is understandable.
Feel free to close this PR & the associated issue, I'll handle it in my fork and I won't bother you in the future :wink:.

> Which issue, if any, is this issue related to?

Fixes #649

> Is there anything in the PR that needs further explanation?

This can be seen as a bug fix or a feature request because it aims at changing the behavior documented here: https://github.com/stylelint/vscode-stylelint/blob/main/src/server/modules/code-action.ts#L215.

- When the requested actions include FixAll, the server ignores other requested actions and only returns the FixAll action,
   - The part of the source code responsible for this behavior is here: https://github.com/stylelint/vscode-stylelint/blob/main/src/server/modules/code-action.ts#L215.
- It does match the VSCode behavior because quickfixes and source / document actions are separate features, triggered separately,
- But some clients (like Zed), send requests for both Quickfixes and source / document actions (FixAll) within the same `only` code actions array.

## What does this PR do?

- Lets the server return all the requested code action types (fix all, quickfixes) at the same time because it's up to the client to decide what it needs.
- This shouldn't impact VSCode because quickfixes and fix all are requested separately (the `only` array just includes `fixAll` or `quickfixes` but not both at the same time (I'm not sure, I just tested this theory quickly, I could definitely be wrong about that)


Patch:
diff --git a/src/server/modules/code-action.ts b/src/server/modules/code-action.ts
index 7daf427..eb26a85 100644
--- a/src/server/modules/code-action.ts
+++ b/src/server/modules/code-action.ts
@@ -211,9 +211,8 @@ export class CodeActionModule implements LanguageServerModule {
 		const only = context.only && new Set(context.only);
 
 		this.#logger?.debug('Creating code actions', { only: context.only });
+		const fixAllActions = [];
 
-		// If asked to provide source or source-fix-all actions, only provide
-		// actions for the whole document.
 		if (
 			only?.has(LSP.CodeActionKind.SourceFixAll) ||
 			only?.has(StylelintCodeActionKind.StylelintSourceFixAll)
@@ -222,23 +221,37 @@ export class CodeActionModule implements LanguageServerModule {
 
 			const action = await this.#getAutoFixAllAction(document);
 
-			return action ? [action] : [];
+			if (action) {
+				fixAllActions.push(action);
+			}
+		} else {
+			this.#logger?.debug(
+				'No source-fix-all actions requested, skipping source-fix-all action creation',
+			);
 		}
 
 		if (only?.has(LSP.CodeActionKind.Source)) {
 			this.#logger?.debug('Creating "source" code action');
 
-			return [this.#getAutoFixAllCommandAction(document)];
+			fixAllActions.push(this.#getAutoFixAllCommandAction(document));
+		} else {
+			this.#logger?.debug('No source actions requested, skipping source action creation');
 		}
 
-		if (only && !only.has(LSP.CodeActionKind.QuickFix)) {
-			this.#logger?.debug('No quick fix actions requested, skipping action creation');
+		if (
+			only &&
+			!only.has(LSP.CodeActionKind.QuickFix) &&
+			!only.has(LSP.CodeActionKind.SourceFixAll) &&
+			!only.has(StylelintCodeActionKind.StylelintSourceFixAll) &&
+			!only.has(LSP.CodeActionKind.Source)
+		) {
+			this.#logger?.debug('No quick fix actions requested, skipping quick fix action creation');
 
-			return [];
+			return fixAllActions;
 		}
 
 		// Otherwise, provide specific actions for each problem.
-		const actions = new RuleCodeActionsCollection();
+		const ruleCodeActions = new RuleCodeActionsCollection();
 
 		for (const diagnostic of context.diagnostics) {
 			const { source, code } = diagnostic;
@@ -260,30 +273,33 @@ export class CodeActionModule implements LanguageServerModule {
 					location,
 				});
 
-				actions.get(code).disableLine = createDisableRuleLineCodeAction(
+				ruleCodeActions.get(code).disableLine = createDisableRuleLineCodeAction(
 					document,
 					diagnostic,
 					location,
 				);
 
-				if (!actions.get(code).disableFile) {
+				if (!ruleCodeActions.get(code).disableFile) {
 					this.#logger?.debug('Creating disable rule for file code action', {
 						rule: code,
 					});
 
-					actions.get(code).disableFile = createDisableRuleFileCodeAction(document, diagnostic);
+					ruleCodeActions.get(code).disableFile = createDisableRuleFileCodeAction(
+						document,
+						diagnostic,
+					);
 				}
 			}
 
-			if (!actions.get(code).documentation) {
+			if (!ruleCodeActions.get(code).documentation) {
 				this.#logger?.debug('Creating documentation code action', {
 					rule: code,
 				});
 
-				actions.get(code).documentation = this.#getOpenRuleDocAction(diagnostic);
+				ruleCodeActions.get(code).documentation = this.#getOpenRuleDocAction(diagnostic);
 			}
 		}
 
-		return [...this.#getQuickFixActions(document, context), ...actions];
+		return [...this.#getQuickFixActions(document, context), ...ruleCodeActions, ...fixAllActions];
 	}
 }


Test patch:
diff --git a/src/server/modules/__tests__/code-action.ts b/src/server/modules/__tests__/code-action.ts
index 8c19df5..eb1fa8a 100644
--- a/src/server/modules/__tests__/code-action.ts
+++ b/src/server/modules/__tests__/code-action.ts
@@ -296,8 +296,16 @@ describe('CodeActionModule', () => {
 		expect(result).toStrictEqual([]);
 		expect(mockContext.getFixes).not.toHaveBeenCalled();
 		expect(mockLogger.debug).toHaveBeenCalledWith(
-			'No quick fix actions requested, skipping action creation',
+			'No source-fix-all actions requested, skipping source-fix-all action creation',
 		);
+		expect(mockLogger.debug).toHaveBeenCalledWith(
+			'No source actions requested, skipping source action creation',
+		);
+		expect(mockLogger.debug).toHaveBeenCalledWith(
+			'No quick fix actions requested, skipping quick fix action creation',
+		);
+		expect(mockLogger.debug).toHaveBeenLastCalledWith('Returning code actions', { actions: [] });
+		expect(mockLogger.debug).toHaveBeenLastCalledWith('Returning code actions', { actions: [] });
 		expect(mockLogger.debug).toHaveBeenLastCalledWith('Returning code actions', { actions: [] });
 	});