fix: Do not show duplicate actions. (#2528)

packages/_server/src/codeActions.ts

@@ -23,6 +23,7 @@ import { DiagnosticData } from './models/DiagnosticData';
 import { Suggestion } from './models/Suggestion';
 import { GetSettingsResult, SuggestionGenerator } from './SuggestionsGenerator';
 import { uniqueFilter } from './utils';
+import * as range from './utils/range';
 import * as Validator from './validator';
 
 const createCommand = LangServerCommand.create;
@@ -45,58 +46,104 @@ export function onCodeActionHandler(
     fnSettingsVersion: (doc: TextDocument) => number,
     clientApi: ClientApi
 ): (params: CodeActionParams) => Promise<CodeAction[]> {
-    type SettingsDictPair = GetSettingsResult;
-    interface CacheEntry {
-        docVersion: number;
-        settingsVersion: number;
-        settings: Promise<SettingsDictPair>;
+    const codeActionHandler = new CodeActionHandler(documents, fnSettings, fnSettingsVersion, clientApi);
+
+    return (params) => codeActionHandler.handler(params);
+}
+
+type SettingsDictPair = GetSettingsResult;
+interface CacheEntry {
+    docVersion: number;
+    settingsVersion: number;
+    settings: Promise<SettingsDictPair>;
+}
+
+class CodeActionHandler {
+    private sugGen: SuggestionGenerator<TextDocument>;
+    private settingsCache: Map<string, CacheEntry>;
+
+    constructor(
+        readonly documents: TextDocuments<TextDocument>,
+        readonly fnSettings: (doc: TextDocument) => Promise<CSpellUserSettings>,
+        readonly fnSettingsVersion: (doc: TextDocument) => number,
+        readonly clientApi: ClientApi
+    ) {
+        this.settingsCache = new Map<string, CacheEntry>();
+        this.sugGen = new SuggestionGenerator((doc) => this.getSettings(doc));
     }
-    const sugGen = new SuggestionGenerator(getSettings);
-    const settingsCache = new Map<string, CacheEntry>();
 
-    async function getSettings(doc: TextDocument): Promise<GetSettingsResult> {
-        const cached = settingsCache.get(doc.uri);
-        const settingsVersion = fnSettingsVersion(doc);
+    async getSettings(doc: TextDocument): Promise<GetSettingsResult> {
+        const cached = this.settingsCache.get(doc.uri);
+        const settingsVersion = this.fnSettingsVersion(doc);
         if (!cached || cached.docVersion !== doc.version || cached.settingsVersion !== settingsVersion) {
-            const settings = constructSettings(doc);
-            settingsCache.set(doc.uri, { docVersion: doc.version, settings, settingsVersion });
+            const settings = this.constructSettings(doc);
+            this.settingsCache.set(doc.uri, { docVersion: doc.version, settings, settingsVersion });
         }
-        return settingsCache.get(doc.uri)!.settings;
+        return this.settingsCache.get(doc.uri)!.settings;
     }
 
-    async function constructSettings(doc: TextDocument): Promise<SettingsDictPair> {
-        const settings = constructSettingsForText(await fnSettings(doc), doc.getText(), doc.languageId);
+    private async constructSettings(doc: TextDocument): Promise<SettingsDictPair> {
+        const settings = constructSettingsForText(await this.fnSettings(doc), doc.getText(), doc.languageId);
         const dictionary = await getDictionary(settings);
         return { settings, dictionary };
     }
 
-    const handler = async (params: CodeActionParams) => {
-        const actions: CodeAction[] = [];
+    public async handler(params: CodeActionParams): Promise<CodeAction[]> {
         const {
             context,
             textDocument: { uri },
         } = params;
         const { diagnostics } = context;
         const spellCheckerDiags = diagnostics.filter((diag) => diag.source === Validator.diagSource);
+        const eslintSpellCheckerDiags = diagnostics.filter((diag) => diag.source === 'eslint' && diag.code == '@cspell/spellchecker');
+
+        if (!spellCheckerDiags.length && !eslintSpellCheckerDiags.length) return [];
+
+        const textDocument = this.documents.get(uri);
+        if (!textDocument) return [];
+
+        const rangeIntersectDiags = [...spellCheckerDiags, ...eslintSpellCheckerDiags]
+            .map((diag) => diag.range)
+            .reduce((a: LangServerRange | undefined, b) => a && range.intersect(a, b), params.range);
+
+        // Only provide suggestions if the selection is contained in the diagnostics.
+        if (!rangeIntersectDiags || !(range.equal(params.range, rangeIntersectDiags) || isWordLikeSelection(textDocument, params.range))) {
+            return [];
+        }
+
+        const ctx = {
+            params,
+            textDocument,
+        };
+
+        return eslintSpellCheckerDiags.length
+            ? this.handlerESLint({ ...ctx, diags: eslintSpellCheckerDiags })
+            : this.handlerCSpell({ ...ctx, diags: spellCheckerDiags });
+    }
+
+    private async handlerCSpell(handlerContext: CodeActionHandlerContext) {
+        const { params, textDocument, diags: spellCheckerDiags } = handlerContext;
+        const actions: CodeAction[] = [];
+        const uri = textDocument.uri;
         if (!spellCheckerDiags.length) return [];
-        const optionalTextDocument = documents.get(uri);
-        if (!optionalTextDocument) return [];
-        log(`CodeAction Only: ${context.only} Num: ${diagnostics.length}`, uri);
-        const textDocument = optionalTextDocument;
-        const { settings: docSetting, dictionary } = await getSettings(textDocument);
+
+        // We do not want to clutter the actions when someone is trying to refactor code
+        if (spellCheckerDiags.length > 1) return [];
+
+        const { settings: docSetting, dictionary } = await this.getSettings(textDocument);
         if (!isUriAllowed(uri, docSetting.allowedSchemas)) {
             log(`CodeAction Uri Not allowed: ${uri}`);
             return [];
         }
-        const pWorkspaceConfig = clientApi.sendOnWorkspaceConfigForDocumentRequest({ uri });
+        const pWorkspaceConfig = this.clientApi.sendOnWorkspaceConfigForDocumentRequest({ uri });
 
         function replaceText(range: LangServerRange, text?: string) {
             return TextEdit.replace(range, text || '');
         }
 
-        function getSuggestions(word: string) {
-            return sugGen.genWordSuggestions(textDocument, word);
-        }
+        const getSuggestions = (word: string) => {
+            return this.sugGen.genWordSuggestions(textDocument, word);
+        };
 
         async function genCodeActionsForSuggestions(_dictionary: SpellingDictionary) {
             log('CodeAction generate suggestions');
@@ -135,9 +182,44 @@ export function onCodeActionHandler(
         }
 
         return genCodeActionsForSuggestions(dictionary);
-    };
+    }
 
-    return handler;
+    private async handlerESLint(handlerContext: CodeActionHandlerContext): Promise<CodeAction[]> {
+        const { params, textDocument, diags: eslintSpellCheckerDiags } = handlerContext;
+        const uri = textDocument.uri;
+        const actions: CodeAction[] = [];
+        if (!eslintSpellCheckerDiags.length) return [];
+
+        // We do not want to clutter the actions when someone is trying to refactor code
+        // or if it is already handled by ESLint.
+        if (eslintSpellCheckerDiags.length > 1) return [];
+
+        const { settings: docSetting, dictionary } = await this.getSettings(textDocument);
+        const pWorkspaceConfig = this.clientApi.sendOnWorkspaceConfigForDocumentRequest({ uri });
+
+        async function genCodeActions(_dictionary: SpellingDictionary) {
+            const word = extractText(textDocument, params.range);
+            // Only suggest adding if it is our diagnostic and there is a word.
+            if (word && eslintSpellCheckerDiags.length) {
+                const wConfig = await pWorkspaceConfig;
+                const targets = calculateConfigTargets(docSetting, wConfig);
+                debugTargets && logTargets(targets);
+
+                if (!docSetting.hideAddToDictionaryCodeActions) {
+                    actions.push(...generateTargetActions(textDocument, eslintSpellCheckerDiags, word, targets));
+                }
+            }
+            return actions;
+        }
+
+        return genCodeActions(dictionary);
+    }
+}
+
+interface CodeActionHandlerContext {
+    params: CodeActionParams;
+    diags: Diagnostic[];
+    textDocument: TextDocument;
 }
 
 const directiveToTitle: Record<string, string | undefined> = Object.assign(Object.create(null), {
@@ -237,3 +319,11 @@ function generateTargetActions(doc: TextDocument, spellCheckerDiags: Diagnostic[
     });
     return actions;
 }
+
+function isWordLikeSelection(doc: TextDocument, range: LangServerRange): boolean {
+    if (range.start.line !== range.end.line) return false;
+
+    const text = doc.getText(range);
+    const hasSpace = /\s/.test(text.trim());
+    return !hasSpace;
+}

packages/_server/src/utils/position.ts

@@ -0,0 +1,24 @@
+import { Position } from 'vscode-languageserver';
+
+/**
+ * Compares two positions
+ * @param a
+ * @param b
+ * @returns
+ *   negative - a < b
+ *   zero - a == b
+ *   positive a > b
+ */
+export function compare(a: Position, b: Position): number {
+    return a.line - b.line || a.character - b.character;
+}
+
+/**
+ * Check if two positions are equivalent.
+ * @param a - Position
+ * @param b - Position
+ * @returns true if they represent the same position.
+ */
+export function equal(a: Position, b: Position): boolean {
+    return a === b || compare(a, b) === 0;
+}

packages/_server/src/utils/range.ts

@@ -0,0 +1,33 @@
+import type { Range } from 'vscode-languageserver';
+
+import * as UtilPosition from './position';
+
+export function intersect(a: Range, b: Range): Range | undefined {
+    if (a === b) return a;
+
+    const start = UtilPosition.compare(a.start, b.start) >= 0 ? a.start : b.start;
+    const end = UtilPosition.compare(a.end, b.end) <= 0 ? a.end : b.end;
+
+    return UtilPosition.compare(start, end) > 0 ? undefined : { start, end };
+}
+
+/**
+ * Check if two ranges are equivalent.
+ * @param a - Range
+ * @param b - Range
+ * @returns true if they are equivalent.
+ */
+export function equal(a: Range, b: Range): boolean {
+    return UtilPosition.equal(a.start, b.start) && UtilPosition.equal(a.end, b.end);
+}
+
+/**
+ * Check if `b` is fully contained in `a`.
+ * @param a - Outer Range
+ * @param b - Inner Range
+ * @returns
+ */
+export function contains(a: Range, b: Range): boolean {
+    const intersection = intersect(a, b);
+    return (intersection && equal(intersection, b)) || false;
+}