Proposal to rename "quickfix.biome" to "source.fixAll.biome"

[predragnikolic]

Description

Hello,

Currently biome exposes a "quickfix.biome" code action on save. (see the docs)
The proposal is to rename quickfix.biome to source.fixAll.biome because it adheres to the LSP spec more:

microsoft/language-server-protocol#1629 (comment)

For code actions on save: the client will ask with a specific kind for code actions on save. The kind is CodeActionKind.SourceFixAll

export namespace CodeActionKind {
	export const Empty: CodeActionKind = '';

	/**
	 * Base kind for quickfix actions: 'quickfix'.
	 */
	export const QuickFix: CodeActionKind = 'quickfix';

	// ...
	/**
	 * Base kind for a 'fix all' source action: `source.fixAll`.
	 *
	 * 'Fix all' actions automatically fix errors that have a clear fix that
	 * do not require user input. They should not suppress errors or perform
	 * unsafe fixes such as generating new types or classes.
	 *
	 * @since 3.17.0
	 */
	export const SourceFixAll: CodeActionKind = 'source.fixAll';
}

Related issues:
sublimelsp/LSP#2495

[ematipico]

That's definitely a sound proposal. I wouldn't change it right away because it would break people's code.

We can add it, deprecate the current when, and remove it in V2. How does that sound?

[predragnikolic]

@ematipico yes, sounds good. Not breaking anyone workflow is a good option.

[suxin2017]

I noticed this code action already exist

biome/crates/biome_analyze/src/categories.rs

Lines 95 to 97 in 8ed14cd

	ActionCategory::Source(SourceActionKind::FixAll) => {
	Cow::Borrowed("source.fixAll.biome")
	}

[ematipico]

@suxin2017 It does but it's not currently used in the LSP

biome/crates/biome_lsp/src/handlers/analysis.rs

Lines 163 to 164 in fe09cab

	if action.category.matches("quickfix.biome") && !file_features.supports_lint() {
	return None;

[suxin2017]

biome/crates/biome_lsp/src/handlers/analysis.rs

Lines 24 to 25 in fe09cab

	const FIX_ALL_CATEGORY: ActionCategory = ActionCategory::Source(SourceActionKind::FixAll);

biome/crates/biome_lsp/src/handlers/analysis.rs

Lines 63 to 74 in fe09cab

	if let Some(filter) = &params.context.only {
	for kind in filter {
	let kind = kind.as_str();
	if FIX_ALL_CATEGORY.matches(kind) {
	has_fix_all = true;
	} else if ActionCategory::QuickFix.to_str() == kind {
	// The action is a on-save quick-fixes
	has_quick_fix = true;
	}
	filters.push(kind);
	}
	}

biome/crates/biome_lsp/src/handlers/analysis.rs

Line 199 in fe09cab

.chain(fix_all)

{
  "editor.codeActionsOnSave": {
    "source.fixAll.biome": "explicit"
  }
}

I see that the code here has fix-all, and my vscode can also work properly, is I looking for the wrong location？

[ematipico]

Nice analysis! I believe it's an issue in our documentation then

[rchl]

Note that biome must also report that code action through codeActionProvider.codeActionKinds server capability. See microsoft/language-server-protocol#1629 (comment)

[suxin2017]

Note that biome must also report that code action through codeActionProvider.codeActionKinds server capability. See microsoft/language-server-protocol#1629 (comment)

I think it's set to true, but I'm not sure if true will meet your needs

[rchl]

I'm talking about codeActionProvider.codeActionKinds capability which accepts an array of kinds.

[suxin2017]

I see what you mean, I mean so far 'codeActionProvider: true' is set, do we have to set codeActionKinds to declare source.fixAll.biome as an array? I see the lsp protocol supports the bool definition

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#:~:text=honorsChangeAnnotations%3F%3A%20boolean%3B%0A%7D-,Server%20Capability%3A,for%20a%20code%20action.%0A%09%20*%0A%09%20*%20%40since%203.16.0%0A%09%20*/%0A%09resolveProvider%3F%3A%20boolean%3B%0A%7D,-Registration%20Options%3A%20CodeActionRegistrationOptions%20defined

[predragnikolic]

@suxin2017 this comment should answer your question ->
microsoft/language-server-protocol#1629 (comment)

[rchl]

The LSP spec author admitted in microsoft/language-server-protocol#1629 (comment) that if the code action is not registered in codeActionProvider.codeActionKinds then the editor should not utilize it for the "code actions on save" functionality.

That said, it was also said that VSCode doesn't follow that now.
And also, not sure how the codeActionProvider: true response should be treated in this case...

I would say that specifying the code action should be ideal and most compatible with different LSP clients.

[suxin2017]

Ok, I got it. Can you check this pr

[predragnikolic]

reopening this as part of #3897