(fix) store rework (#1531)

Reworks $store handling in the code base. Previously, svelte2tsx would transform $store access to something like (__get(store), $store). This is now no longer the case. This means we need to add some extra code to some of the intellisense features: - if someone renames a $store, store also needs to be renamed and vice versa - if someone wants to find references of a $store, we also need to find store references and vice versa - if someone wants to go to the definition of a $store, we need to reroute that to store - if someone wants to go to the implementation of a $store, we need to reroute that to store diagnostics of wrong $store usages need to be handled This means some more code overall, but I feel it's the better choice since we had so many edge cases with our transformation other time, and #1475 being an upstream bug we cannot solve ourselves but need to solve for the new transformation.
sveltejs · Jun 21, 2022 · e91eeba · e91eeba
1 parent c44706a
commit e91eeba
Show file tree

Hide file tree

Showing 99 changed files with 1,053 additions and 807 deletions.
diff --git a/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts b/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts
@@ -68,7 +68,11 @@ import { SemanticTokensProviderImpl } from './features/SemanticTokensProvider';
 import { SignatureHelpProviderImpl } from './features/SignatureHelpProvider';
 import { TypeDefinitionProviderImpl } from './features/TypeDefinitionProvider';
 import { UpdateImportsProviderImpl } from './features/UpdateImportsProvider';
-import { isNoTextSpanInGeneratedCode, SnapshotMap } from './features/utils';
+import {
+    is$storeVariableIn$storeDeclaration,
+    isTextSpanInGeneratedCode,
+    SnapshotMap
+} from './features/utils';
 import { LSAndTSDocResolver } from './LSAndTSDocResolver';
 import { ignoredBuildDirectories } from './SnapshotManager';
 import { isAttributeName, isAttributeShorthand, isEventHandler } from './svelte-ast-utils';
@@ -344,19 +348,36 @@ export class TypeScriptPlugin
 
         const result = await Promise.all(
             defs.definitions.map(async (def) => {
-                const snapshot = await snapshots.retrieve(def.fileName);
+                if (def.fileName.endsWith('svelte-shims.d.ts')) {
+                    return;
+                }
 
-                if (
-                    !def.fileName.endsWith('svelte-shims.d.ts') &&
-                    isNoTextSpanInGeneratedCode(snapshot.getFullText(), def.textSpan)
-                ) {
-                    return LocationLink.create(
-                        pathToUrl(def.fileName),
-                        convertToLocationRange(snapshot, def.textSpan),
-                        convertToLocationRange(snapshot, def.textSpan),
-                        convertToLocationRange(tsDoc, defs.textSpan)
-                    );
+                let snapshot = await snapshots.retrieve(def.fileName);
+
+                // Go from generated $store to store if user wants to find definition for $store
+                if (isTextSpanInGeneratedCode(snapshot.getFullText(), def.textSpan)) {
+                    if (
+                        !is$storeVariableIn$storeDeclaration(
+                            snapshot.getFullText(),
+                            def.textSpan.start
+                        )
+                    ) {
+                        return;
+                    }
+                    // there will be exactly one definition, the store
+                    def = lang.getDefinitionAndBoundSpan(
+                        tsDoc.filePath,
+                        tsDoc.getFullText().indexOf(');', def.textSpan.start) - 1
+                    )!.definitions![0];
+                    snapshot = await snapshots.retrieve(def.fileName);
                 }
+
+                return LocationLink.create(
+                    pathToUrl(def.fileName),
+                    convertToLocationRange(snapshot, def.textSpan),
+                    convertToLocationRange(snapshot, def.textSpan),
+                    convertToLocationRange(tsDoc, defs.textSpan)
+                );
             })
         );
         return result.filter(isNotNullOrUndefined);

diff --git a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts
@@ -25,7 +25,7 @@ import { DocumentSnapshot, SvelteDocumentSnapshot } from '../DocumentSnapshot';
 import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
 import { changeSvelteComponentName, convertRange } from '../utils';
 import { CompletionsProviderImpl } from './CompletionProvider';
-import { findContainingNode, isNoTextSpanInGeneratedCode, SnapshotMap } from './utils';
+import { findContainingNode, isTextSpanInGeneratedCode, SnapshotMap } from './utils';
 
 /**
  * TODO change this to protocol constant if it's part of the protocol
@@ -298,7 +298,7 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
                                 );
                             }
 
-                            if (!isNoTextSpanInGeneratedCode(snapshot.getFullText(), edit.span)) {
+                            if (isTextSpanInGeneratedCode(snapshot.getFullText(), edit.span)) {
                                 return undefined;
                             }
 

diff --git a/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts b/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts
@@ -17,7 +17,9 @@ import {
     findNodeAtSpan,
     isReactiveStatement,
     isInReactiveStatement,
-    gatherIdentifiers
+    gatherIdentifiers,
+    isStoreVariableIn$storeDeclaration,
+    get$storeOffsetOf$storeDeclaration
 } from './utils';
 import { not, flatten, passMap, regexIndexOf, swapRangeStartEndIfNecessary } from '../../../utils';
 import { LSConfigManager } from '../../../ls-config';
@@ -37,7 +39,8 @@ enum DiagnosticCode {
     MULTIPLE_PROPS_SAME_NAME = 1117, // "An object literal cannot have multiple properties with the same name in strict mode."
     TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y = 2345, // "Argument of type '..' is not assignable to parameter of type '..'."
     MISSING_PROPS = 2739, // "Type '...' is missing the following properties from type '..': ..."
-    MISSING_PROP = 2741 // "Property '..' is missing in type '..' but required in type '..'."
+    MISSING_PROP = 2741, // "Property '..' is missing in type '..' but required in type '..'."
+    NO_OVERLOAD_MATCHES_CALL = 2769 // "No overload matches this call"
 }
 
 export class DiagnosticsProviderImpl implements DiagnosticsProvider {
@@ -80,9 +83,40 @@ export class DiagnosticsProviderImpl implements DiagnosticsProvider {
             ...lang.getSuggestionDiagnostics(tsDoc.filePath),
             ...lang.getSemanticDiagnostics(tsDoc.filePath)
         ];
-        diagnostics = diagnostics
-            .filter(isNotGenerated(tsDoc.getText(0, tsDoc.getLength())))
-            .filter(not(isUnusedReactiveStatementLabel));
+
+        const additionalStoreDiagnostics: ts.Diagnostic[] = [];
+        const notGenerated = isNotGenerated(tsDoc.getFullText());
+        for (const diagnostic of diagnostics) {
+            if (
+                (diagnostic.code === DiagnosticCode.NO_OVERLOAD_MATCHES_CALL ||
+                    diagnostic.code === DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y) &&
+                !notGenerated(diagnostic)
+            ) {
+                if (isStoreVariableIn$storeDeclaration(tsDoc.getFullText(), diagnostic.start!)) {
+                    const storeName = tsDoc
+                        .getFullText()
+                        .substring(diagnostic.start!, diagnostic.start! + diagnostic.length!);
+                    const storeUsages = lang.findReferences(
+                        tsDoc.filePath,
+                        get$storeOffsetOf$storeDeclaration(tsDoc.getFullText(), diagnostic.start!)
+                    )![0].references;
+                    for (const storeUsage of storeUsages) {
+                        additionalStoreDiagnostics.push({
+                            ...diagnostic,
+                            messageText: `Cannot use '${storeName}' as a store. '${storeName}' needs to be an object with a subscribe method on it.\n\n${ts.flattenDiagnosticMessageText(
+                                diagnostic.messageText,
+                                '\n'
+                            )}`,
+                            start: storeUsage.textSpan.start,
+                            length: storeUsage.textSpan.length
+                        });
+                    }
+                }
+            }
+        }
+        diagnostics.push(...additionalStoreDiagnostics);
+
+        diagnostics = diagnostics.filter(notGenerated).filter(not(isUnusedReactiveStatementLabel));
         diagnostics = resolveNoopsInReactiveStatements(lang, diagnostics);
 
         return diagnostics

diff --git a/packages/language-server/src/plugins/typescript/features/FindComponentReferencesProvider.ts b/packages/language-server/src/plugins/typescript/features/FindComponentReferencesProvider.ts
@@ -4,7 +4,7 @@ import { FindComponentReferencesProvider } from '../../interfaces';
 import { DocumentSnapshot, SvelteDocumentSnapshot } from '../DocumentSnapshot';
 import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
 import { convertToLocationRange, hasNonZeroRange } from '../utils';
-import { isNoTextSpanInGeneratedCode, SnapshotMap } from './utils';
+import { isTextSpanInGeneratedCode, SnapshotMap } from './utils';
 
 const COMPONENT_SUFFIX = '__SvelteComponent_';
 
@@ -40,7 +40,7 @@ export class FindComponentReferencesProviderImpl implements FindComponentReferen
 
                 const snapshot = await snapshots.retrieve(ref.fileName);
 
-                if (!isNoTextSpanInGeneratedCode(snapshot.getFullText(), ref.textSpan)) {
+                if (isTextSpanInGeneratedCode(snapshot.getFullText(), ref.textSpan)) {
                     return null;
                 }
 

diff --git a/packages/language-server/src/plugins/typescript/features/FindReferencesProvider.ts b/packages/language-server/src/plugins/typescript/features/FindReferencesProvider.ts
@@ -1,10 +1,19 @@
+import type ts from 'typescript';
 import { Location, Position, ReferenceContext } from 'vscode-languageserver';
 import { Document } from '../../../lib/documents';
 import { flatten, isNotNullOrUndefined, pathToUrl } from '../../../utils';
 import { FindReferencesProvider } from '../../interfaces';
+import { SvelteDocumentSnapshot } from '../DocumentSnapshot';
 import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
 import { convertToLocationRange, hasNonZeroRange } from '../utils';
-import { isNoTextSpanInGeneratedCode, SnapshotMap } from './utils';
+import {
+    get$storeOffsetOf$storeDeclaration,
+    getStoreOffsetOf$storeDeclaration,
+    is$storeVariableIn$storeDeclaration,
+    isStoreVariableIn$storeDeclaration,
+    isTextSpanInGeneratedCode,
+    SnapshotMap
+} from './utils';
 
 export class FindReferencesProviderImpl implements FindReferencesProvider {
     constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) {}
@@ -16,48 +25,129 @@ export class FindReferencesProviderImpl implements FindReferencesProvider {
     ): Promise<Location[] | null> {
         const { lang, tsDoc } = await this.getLSAndTSDoc(document);
 
-        const references = lang.findReferences(
+        const rawReferences = lang.findReferences(
             tsDoc.filePath,
             tsDoc.offsetAt(tsDoc.getGeneratedPosition(position))
         );
-        if (!references) {
+        if (!rawReferences) {
             return null;
         }
 
         const snapshots = new SnapshotMap(this.lsAndTsDocResolver);
         snapshots.set(tsDoc.filePath, tsDoc);
 
+        const references = flatten(rawReferences.map((ref) => ref.references));
+        references.push(...(await this.getStoreReferences(references, tsDoc, snapshots, lang)));
+
         const locations = await Promise.all(
-            flatten(references.map((ref) => ref.references)).map(async (ref) => {
-                if (!context.includeDeclaration && ref.isDefinition) {
-                    return null;
-                }
-
-                const snapshot = await snapshots.retrieve(ref.fileName);
-
-                if (!isNoTextSpanInGeneratedCode(snapshot.getFullText(), ref.textSpan)) {
-                    return null;
-                }
-
-                const location = Location.create(
-                    pathToUrl(ref.fileName),
-                    convertToLocationRange(snapshot, ref.textSpan)
-                );
-
-                // Some references are in generated code but not wrapped with explicit ignore comments.
-                // These show up as zero-length ranges, so filter them out.
-                if (!hasNonZeroRange(location)) {
-                    return null;
-                }
-
-                return location;
-            })
+            references.map(async (ref) => this.mapReference(ref, context, snapshots))
+        );
+
+        return (
+            locations
+                .filter(isNotNullOrUndefined)
+                // Possible $store references are added afterwards, sort for correct order
+                .sort(sortLocationByFileAndRange)
+        );
+    }
+
+    /**
+     * If references of a $store are searched, also find references for the corresponding store
+     * and vice versa.
+     */
+    private async getStoreReferences(
+        references: ts.ReferencedSymbolEntry[],
+        tsDoc: SvelteDocumentSnapshot,
+        snapshots: SnapshotMap,
+        lang: ts.LanguageService
+    ): Promise<ts.ReferencedSymbolEntry[]> {
+        // If user started finding references at $store, find references for store, too
+        let storeReferences: ts.ReferencedSymbolEntry[] = [];
+        const storeReference = references.find(
+            (ref) =>
+                ref.fileName === tsDoc.filePath &&
+                isTextSpanInGeneratedCode(tsDoc.getFullText(), ref.textSpan) &&
+                is$storeVariableIn$storeDeclaration(tsDoc.getFullText(), ref.textSpan.start)
+        );
+        if (storeReference) {
+            const additionalReferences =
+                lang.findReferences(
+                    tsDoc.filePath,
+                    getStoreOffsetOf$storeDeclaration(
+                        tsDoc.getFullText(),
+                        storeReference.textSpan.start
+                    )
+                ) || [];
+            storeReferences = flatten(additionalReferences.map((ref) => ref.references));
+        }
+
+        // If user started finding references at store, find references for $store, too
+        // If user started finding references at $store, find references for $store in other files
+        const $storeReferences: ts.ReferencedSymbolEntry[] = [];
+        for (const ref of [...references, ...storeReferences]) {
+            const snapshot = await snapshots.retrieve(ref.fileName);
+            if (
+                !(
+                    isTextSpanInGeneratedCode(snapshot.getFullText(), ref.textSpan) &&
+                    isStoreVariableIn$storeDeclaration(snapshot.getFullText(), ref.textSpan.start)
+                )
+            ) {
+                continue;
+            }
+            if (storeReference?.fileName === ref.fileName) {
+                // $store in X -> usages of store -> store in X -> we would add duplicate $store references
+                continue;
+            }
+
+            const additionalReferences =
+                lang.findReferences(
+                    snapshot.filePath,
+                    get$storeOffsetOf$storeDeclaration(snapshot.getFullText(), ref.textSpan.start)
+                ) || [];
+            $storeReferences.push(...flatten(additionalReferences.map((ref) => ref.references)));
+        }
+
+        return [...storeReferences, ...$storeReferences];
+    }
+
+    private async mapReference(
+        ref: ts.ReferencedSymbolEntry,
+        context: ReferenceContext,
+        snapshots: SnapshotMap
+    ) {
+        if (!context.includeDeclaration && ref.isDefinition) {
+            return null;
+        }
+
+        const snapshot = await snapshots.retrieve(ref.fileName);
+
+        if (isTextSpanInGeneratedCode(snapshot.getFullText(), ref.textSpan)) {
+            return null;
+        }
+
+        const location = Location.create(
+            pathToUrl(ref.fileName),
+            convertToLocationRange(snapshot, ref.textSpan)
         );
 
-        return locations.filter(isNotNullOrUndefined);
+        // Some references are in generated code but not wrapped with explicit ignore comments.
+        // These show up as zero-length ranges, so filter them out.
+        if (!hasNonZeroRange(location)) {
+            return null;
+        }
+
+        return location;
     }
 
     private async getLSAndTSDoc(document: Document) {
         return this.lsAndTsDocResolver.getLSAndTSDoc(document);
     }
 }
+
+function sortLocationByFileAndRange(l1: Location, l2: Location): number {
+    const localeCompare = l1.uri.localeCompare(l2.uri);
+    return localeCompare === 0
+        ? (l1.range.start.line - l2.range.start.line) * 10000 +
+              (l1.range.start.character - l2.range.start.character)
+        : localeCompare;
+}
diff --git a/packages/language-server/src/plugins/typescript/features/HoverProvider.ts b/packages/language-server/src/plugins/typescript/features/HoverProvider.ts
@@ -20,28 +20,11 @@ export class HoverProviderImpl implements HoverProvider {
         }
 
         const offset = tsDoc.offsetAt(tsDoc.getGeneratedPosition(position));
-        let info = lang.getQuickInfoAtPosition(tsDoc.filePath, offset);
+        const info = lang.getQuickInfoAtPosition(tsDoc.filePath, offset);
         if (!info) {
             return null;
         }
 
-        const textSpan = info.textSpan;
-
-        // show docs of $store instead of store if necessary
-        const is$store = tsDoc
-            .getFullText()
-            .substring(0, info.textSpan.start)
-            .endsWith('(__sveltets_1_store_get(');
-        if (is$store) {
-            const infoFor$store = lang.getQuickInfoAtPosition(
-                tsDoc.filePath,
-                textSpan.start + textSpan.length + 3
-            );
-            if (infoFor$store) {
-                info = infoFor$store;
-            }
-        }
-
         const declaration = ts.displayPartsToString(info.displayParts);
         const documentation = getMarkdownDocumentation(info.documentation, info.tags);
 
@@ -51,7 +34,7 @@ export class HoverProviderImpl implements HoverProvider {
             .join('\n');
 
         return mapObjWithRangeToOriginal(tsDoc, {
-            range: convertRange(tsDoc, textSpan),
+            range: convertRange(tsDoc, info.textSpan),
             contents
         });
     }