(feat) rename of component props in other component (#1043)

dummdidumm · web-flow · commit 67f8a4aa93f4 · 2021-06-03T11:05:20.000+02:00
Allow rename of component's A props in another component B. #110
diff --git a/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts b/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts
@@ -136,13 +136,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
             return null;
         }
 
-        const eventCompletions = await this.getEventCompletions(
-            lang,
-            document,
-            tsDoc,
-            fragment,
-            position
-        );
+        const eventCompletions = await this.getEventCompletions(lang, document, tsDoc, position);
 
         if (isEventTriggerCharacter) {
             return CompletionList.create(eventCompletions, !!tsDoc.parserError);
@@ -214,15 +208,13 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
         lang: ts.LanguageService,
         doc: Document,
         tsDoc: SvelteDocumentSnapshot,
-        fragment: SvelteSnapshotFragment,
         originalPosition: Position
     ): Promise<Array<AppCompletionItem<CompletionEntryWithIdentifer>>> {
         const snapshot = await getComponentAtPosition(
             this.lsAndTsDocResolver,
             lang,
             doc,
             tsDoc,
-            fragment,
             originalPosition
         );
         if (!snapshot) {
diff --git a/packages/language-server/src/plugins/typescript/features/HoverProvider.ts b/packages/language-server/src/plugins/typescript/features/HoverProvider.ts
@@ -2,7 +2,7 @@ import ts from 'typescript';
 import { Hover, Position } from 'vscode-languageserver';
 import { Document, getWordAt, mapObjWithRangeToOriginal } from '../../../lib/documents';
 import { HoverProvider } from '../../interfaces';
-import { SvelteDocumentSnapshot, SvelteSnapshotFragment } from '../DocumentSnapshot';
+import { SvelteDocumentSnapshot } from '../DocumentSnapshot';
 import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
 import { getMarkdownDocumentation } from '../previewer';
 import { convertRange } from '../utils';
@@ -15,13 +15,7 @@ export class HoverProviderImpl implements HoverProvider {
         const { lang, tsDoc } = await this.getLSAndTSDoc(document);
         const fragment = await tsDoc.getFragment();
 
-        const eventHoverInfo = await this.getEventHoverInfo(
-            lang,
-            document,
-            tsDoc,
-            fragment,
-            position
-        );
+        const eventHoverInfo = await this.getEventHoverInfo(lang, document, tsDoc, position);
         if (eventHoverInfo) {
             return eventHoverInfo;
         }
@@ -66,7 +60,6 @@ export class HoverProviderImpl implements HoverProvider {
         lang: ts.LanguageService,
         doc: Document,
         tsDoc: SvelteDocumentSnapshot,
-        fragment: SvelteSnapshotFragment,
         originalPosition: Position
     ): Promise<Hover | null> {
         const possibleEventName = getWordAt(doc.getText(), doc.offsetAt(originalPosition), {
@@ -82,7 +75,6 @@ export class HoverProviderImpl implements HoverProvider {
             lang,
             doc,
             tsDoc,
-            fragment,
             originalPosition
         );
         if (!component) {
diff --git a/packages/language-server/src/plugins/typescript/features/RenameProvider.ts b/packages/language-server/src/plugins/typescript/features/RenameProvider.ts
@@ -1,11 +1,5 @@
 import { Position, WorkspaceEdit, Range } from 'vscode-languageserver';
-import {
-    Document,
-    mapRangeToOriginal,
-    positionAt,
-    offsetAt,
-    getLineAtPosition
-} from '../../../lib/documents';
+import { Document, mapRangeToOriginal, getLineAtPosition } from '../../../lib/documents';
 import { filterAsync, isNotNullOrUndefined, pathToUrl } from '../../../utils';
 import { RenameProvider } from '../../interfaces';
 import {
@@ -17,7 +11,7 @@ import { convertRange } from '../utils';
 import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
 import ts from 'typescript';
 import { uniqWith, isEqual } from 'lodash';
-import { isNoTextSpanInGeneratedCode, SnapshotFragmentMap } from './utils';
+import { isComponentAtPosition, isNoTextSpanInGeneratedCode, SnapshotFragmentMap } from './utils';
 
 export class RenameProviderImpl implements RenameProvider {
     constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) {}
@@ -29,7 +23,7 @@ export class RenameProviderImpl implements RenameProvider {
         const fragment = await tsDoc.getFragment();
 
         const offset = fragment.offsetAt(fragment.getGeneratedPosition(position));
-        const renameInfo = this.getRenameInfo(lang, tsDoc, offset);
+        const renameInfo = this.getRenameInfo(lang, tsDoc, document, position, offset);
         if (!renameInfo) {
             return null;
         }
@@ -47,7 +41,7 @@ export class RenameProviderImpl implements RenameProvider {
 
         const offset = fragment.offsetAt(fragment.getGeneratedPosition(position));
 
-        if (!this.getRenameInfo(lang, tsDoc, offset)) {
+        if (!this.getRenameInfo(lang, tsDoc, document, position, offset)) {
             return null;
         }
 
@@ -118,7 +112,9 @@ export class RenameProviderImpl implements RenameProvider {
     private getRenameInfo(
         lang: ts.LanguageService,
         tsDoc: SvelteDocumentSnapshot,
-        offset: number
+        doc: Document,
+        originalPosition: Position,
+        generatedOffset: number
     ): {
         canRename: true;
         kind: ts.ScriptElementKind;
@@ -131,20 +127,18 @@ export class RenameProviderImpl implements RenameProvider {
         if (tsDoc.parserError) {
             return null;
         }
-        const renameInfo: any = lang.getRenameInfo(tsDoc.filePath, offset, {
+        const renameInfo: any = lang.getRenameInfo(tsDoc.filePath, generatedOffset, {
             allowRenameOfImportPath: false
         });
-        // TODO this will also forbid renames of svelte component properties
-        // in another component because the ScriptElementKind is a JSXAttribute.
-        // To fix this we would need to enhance svelte2tsx with info methods like
-        // "what props does this file have?"
         if (
             !renameInfo.canRename ||
-            renameInfo.kind === ts.ScriptElementKind.jsxAttribute ||
-            renameInfo.fullDisplayName?.includes('JSX.IntrinsicElements')
+            renameInfo.fullDisplayName?.includes('JSX.IntrinsicElements') ||
+            (renameInfo.kind === ts.ScriptElementKind.jsxAttribute &&
+                !isComponentAtPosition(doc, tsDoc, originalPosition))
         ) {
             return null;
         }
+
         return renameInfo;
     }
 
@@ -279,13 +273,9 @@ export class RenameProviderImpl implements RenameProvider {
 
     // --------> svelte2tsx?
     private isInSvelte2TsxPropLine(fragment: SvelteSnapshotFragment, loc: ts.RenameLocation) {
-        const pos = positionAt(loc.textSpan.start, fragment.text);
-        const textInLine = fragment.text.substring(
-            offsetAt({ ...pos, character: 0 }, fragment.text),
-            loc.textSpan.start
-        );
+        const textBeforeProp = fragment.text.substring(0, loc.textSpan.start);
         // This is how svelte2tsx writes out the props
-        if (textInLine.includes('return { props: {')) {
+        if (textBeforeProp.includes('\nreturn { props: {')) {
             return true;
         }
     }
diff --git a/packages/language-server/src/plugins/typescript/features/utils.ts b/packages/language-server/src/plugins/typescript/features/utils.ts
@@ -6,12 +6,7 @@ import {
     getNodeIfIsInComponentStartTag,
     isInTag
 } from '../../../lib/documents';
-import {
-    DocumentSnapshot,
-    SnapshotFragment,
-    SvelteDocumentSnapshot,
-    SvelteSnapshotFragment
-} from '../DocumentSnapshot';
+import { DocumentSnapshot, SnapshotFragment, SvelteDocumentSnapshot } from '../DocumentSnapshot';
 import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
 
 /**
@@ -23,7 +18,6 @@ export async function getComponentAtPosition(
     lang: ts.LanguageService,
     doc: Document,
     tsDoc: SvelteDocumentSnapshot,
-    fragment: SvelteSnapshotFragment,
     originalPosition: Position
 ): Promise<SvelteDocumentSnapshot | null> {
     if (tsDoc.parserError) {
@@ -43,6 +37,7 @@ export async function getComponentAtPosition(
         return null;
     }
 
+    const fragment = await tsDoc.getFragment();
     const generatedPosition = fragment.getGeneratedPosition(doc.positionAt(node.start + 1));
     const def = lang.getDefinitionAtPosition(
         tsDoc.filePath,
@@ -59,6 +54,26 @@ export async function getComponentAtPosition(
     return snapshot;
 }
 
+export function isComponentAtPosition(
+    doc: Document,
+    tsDoc: SvelteDocumentSnapshot,
+    originalPosition: Position
+): boolean {
+    if (tsDoc.parserError) {
+        return false;
+    }
+
+    if (
+        isInTag(originalPosition, doc.scriptInfo) ||
+        isInTag(originalPosition, doc.moduleScriptInfo)
+    ) {
+        // Inside script tags -> not a component
+        return false;
+    }
+
+    return !!getNodeIfIsInComponentStartTag(doc.html, doc.offsetAt(originalPosition));
+}
+
 /**
  * Checks if this a section that should be completely ignored
  * because it's purely generated.
diff --git a/packages/language-server/test/plugins/typescript/features/RenameProvider.test.ts b/packages/language-server/test/plugins/typescript/features/RenameProvider.test.ts
@@ -209,13 +209,21 @@ describe('RenameProvider', () => {
         assert.deepStrictEqual(result, expectedEditsForPropRename);
     });
 
-    // TODO this does not work right now, see `RenameProviderImpl.cannotRename` for more explanation
-    // it('should do rename of prop of component A in component B', async () => {
-    //     const { provider, renameDoc2 } = await setup();
-    //     const result = await provider.rename(renameDoc2, Position.create(4, 10), 'newName');
+    it('should do rename of prop of component A in component B', async () => {
+        const { provider, renameDoc2 } = await setup();
+        const result = await provider.rename(renameDoc2, Position.create(5, 10), 'newName');
 
-    //     assert.deepStrictEqual(result, expectedEditsForPropRename);
-    // });
+        assert.deepStrictEqual(result, expectedEditsForPropRename);
+    });
+
+    it('should not allow rename of intrinsic attribute', async () => {
+        const { provider, renameDoc2 } = await setup();
+        const prepareResult = await provider.prepareRename(renameDoc2, Position.create(7, 7));
+        const renameResult = await provider.rename(renameDoc2, Position.create(7, 7), 'newName');
+
+        assert.deepStrictEqual(prepareResult, null);
+        assert.deepStrictEqual(renameResult, null);
+    });
 
     it('should do rename of prop without type of component A in component A', async () => {
         const { provider, renameDoc3 } = await setup();
diff --git a/packages/language-server/test/plugins/typescript/testfiles/rename/rename2.svelte b/packages/language-server/test/plugins/typescript/testfiles/rename/rename2.svelte
@@ -4,4 +4,5 @@ import Rename3 from './rename3.svelte';
 </script>
 
 <Rename exportedProp={2}></Rename>
-<Rename3 exportedPropFromJs={2}></Rename3>
+<Rename3 exportedPropFromJs={2}></Rename3>
+<div class="foo" />
