fix: prevent error with unclosed tag followed by LF or end of file (#2750)

#2742

The reason that it happens to LF but not CRLF is that CRLF has two characters, so it doesn't touch the range of the start tag name, and therefore won't trigger the magic string error. This should also improve auto-import performance a bit because TypeScript only needs to compute auto-imports for Button and not a global auto-import.

Author: jasonlyu123

packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts

@@ -383,7 +383,7 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
         if (editForThisFile?.edits.length) {
             const [first] = editForThisFile.edits;
             first.newText =
-                getNewScriptStartTag(this.configManager.getConfig()) +
+                getNewScriptStartTag(this.configManager.getConfig(), formatCodeBasis.newLine) +
                 formatCodeBasis.baseIndent +
                 first.newText.trimStart();
 

packages/language-server/src/plugins/typescript/features/CompletionProvider.ts

@@ -595,7 +595,8 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
         return tagNames.map((name) => ({
             label: name,
             kind: CompletionItemKind.Property,
-            textEdit: TextEdit.replace(this.cloneRange(replacementRange), name)
+            textEdit: TextEdit.replace(this.cloneRange(replacementRange), name),
+            commitCharacters: []
         }));
     }
 
@@ -1131,9 +1132,17 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
             }
 
             const config = this.configManager.getConfig();
+            // Remove the empty line after the script tag because getNewScriptStartTag will always add one
+            let newText = change.newText;
+            if (newText[0] === '\r') {
+                newText = newText.substring(1);
+            }
+            if (newText[0] === '\n') {
+                newText = newText.substring(1);
+            }
             return TextEdit.replace(
                 beginOfDocumentRange,
-                `${getNewScriptStartTag(config)}${change.newText}</script>${newLine}`
+                `${getNewScriptStartTag(config, newLine)}${newText}</script>${newLine}`
             );
         }
 

packages/language-server/src/plugins/typescript/features/utils.ts

@@ -429,10 +429,10 @@ export function findChildOfKind(node: ts.Node, kind: ts.SyntaxKind): ts.Node | u
     }
 }
 
-export function getNewScriptStartTag(lsConfig: Readonly<LSConfig>) {
+export function getNewScriptStartTag(lsConfig: Readonly<LSConfig>, newLine: string) {
     const lang = lsConfig.svelte.defaultScriptLanguage;
     const scriptLang = lang === 'none' ? '' : ` lang="${lang}"`;
-    return `<script${scriptLang}>${ts.sys.newLine}`;
+    return `<script${scriptLang}>${newLine}`;
 }
 
 export function checkRangeMappingWithGeneratedSemi(

packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts

@@ -298,6 +298,7 @@ describe('CompletionProviderImpl', function () {
         assert.deepStrictEqual(item, <CompletionItem>{
             label: 'custom-element',
             kind: CompletionItemKind.Property,
+            commitCharacters: [],
             textEdit: {
                 range: { start: { line: 0, character: 1 }, end: { line: 0, character: 2 } },
                 newText: 'custom-element'

packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Element.ts

@@ -43,6 +43,7 @@ export class Element {
     private isSelfclosing: boolean;
     public tagName: string;
     public child?: any;
+    private tagNameEnd: number;
 
     // Add const $$xxx = ... only if the variable name is actually used
     // in order to prevent "$$xxx is defined but never used" TS hints
@@ -74,7 +75,7 @@ export class Element {
         this.startTagStart = this.node.start;
         this.startTagEnd = this.computeStartTagEnd();
 
-        const tagEnd = this.startTagStart + this.node.name.length + 1;
+        const tagEnd = (this.tagNameEnd = this.startTagStart + this.node.name.length + 1);
         // Ensure deleted characters are mapped to the attributes object so we
         // get autocompletion when triggering it on a whitespace.
         if (/\s/.test(str.original.charAt(tagEnd))) {
@@ -205,7 +206,19 @@ export class Element {
         }
 
         if (this.isSelfclosing) {
-            transform(this.str, this.startTagStart, this.startTagEnd, [
+            // The transformation is the whole start tag + <, ex: <span
+            // To avoid the end tag transform being moved to before the tag name,
+            // manually remove the first character and let the `transform` function skip removing unused
+            let transformEnd = this.startTagEnd;
+            if (
+                this.str.original[transformEnd - 1] !== '>' &&
+                (transformEnd === this.tagNameEnd || transformEnd === this.tagNameEnd + 1)
+            ) {
+                transformEnd = this.startTagStart;
+                this.str.remove(this.startTagStart, this.startTagStart + 1);
+            }
+
+            transform(this.str, this.startTagStart, transformEnd, [
                 // Named slot transformations go first inside a outer block scope because
                 // <div let:xx {x} /> means "use the x of let:x", and without a separate
                 // block scope this would give a "used before defined" error
@@ -295,7 +308,7 @@ export class Element {
             default: {
                 createElementStatement = [
                     `${createElement}("`,
-                    [this.node.start + 1, this.node.start + 1 + this.node.name.length],
+                    [this.node.start + 1, this.tagNameEnd],
                     `"${addActions()}, {`
                 ];
                 break;

packages/svelte2tsx/src/htmlxtojsx_v2/nodes/InlineComponent.ts

@@ -38,6 +38,7 @@ export class InlineComponent {
     private startTagStart: number;
     private startTagEnd: number;
     private isSelfclosing: boolean;
+    private tagNameEnd: number;
     public child?: any;
 
     // Add const $$xxx = ... only if the variable name is actually used
@@ -64,7 +65,7 @@ export class InlineComponent {
         this.startTagStart = this.node.start;
         this.startTagEnd = this.computeStartTagEnd();
 
-        const tagEnd = this.startTagStart + this.node.name.length + 1;
+        const tagEnd = (this.tagNameEnd = this.startTagStart + this.node.name.length + 1);
         // Ensure deleted characters are mapped to the attributes object so we
         // get autocompletion when triggering it on a whitespace.
         if (/\s/.test(str.original.charAt(tagEnd))) {
@@ -227,7 +228,7 @@ export class InlineComponent {
             if (endStart === -1) {
                 // Can happen in loose parsing mode when there's no closing tag
                 endStart = this.node.end;
-                this.startTagEnd = this.node.end - 1;
+                this.startTagEnd = Math.max(this.node.end - 1, this.tagNameEnd);
             } else {
                 endStart += this.node.start;
             }
@@ -238,7 +239,17 @@ export class InlineComponent {
             }
             this.endTransformation.push('}');
 
-            transform(this.str, this.startTagStart, this.startTagEnd, [
+            let transformationEnd = this.startTagEnd;
+
+            // The transformation is the whole start tag + <, ex: <Comp
+            // To avoid the "cannot move inside itself" error,
+            // manually remove the first character and let the transform function skip removing unused
+            if (transformationEnd === this.tagNameEnd) {
+                transformationEnd = this.startTagStart;
+                this.str.remove(this.startTagStart, this.startTagStart + 1);
+            }
+
+            transform(this.str, this.startTagStart, transformationEnd, [
                 // See comment above why this goes first
                 ...namedSlotLetTransformation,
                 ...this.startTransformation,

packages/svelte2tsx/test/htmlx2jsx/samples/editing-unclosed-component-no-attr.v5/expected.error.json

@@ -0,0 +1,20 @@
+{
+    "code": "expected_token",
+    "message": "Expected token >\nhttps://svelte.dev/e/expected_token",
+    "filename": "(unknown)",
+    "start": {
+        "line": 7,
+        "column": 2,
+        "character": 138
+    },
+    "end": {
+        "line": 7,
+        "column": 2,
+        "character": 138
+    },
+    "position": [
+        138,
+        138
+    ],
+    "frame": " 5: <div>\n 6:     <Comp\n 7: </div>\n      ^\n 8: \n 9: <Comp"
+}

packages/svelte2tsx/test/htmlx2jsx/samples/editing-unclosed-component-no-attr.v5/expectedv2.js

@@ -0,0 +1,5 @@
+
+ { svelteHTML.createElement("div", {});
+    { const $$_pmoC1C = __sveltets_2_ensureComponent(Comp); new $$_pmoC1C({ target: __sveltets_2_any(), props: {}});} }
+
+{ const $$_pmoC0C = __sveltets_2_ensureComponent(Comp); new $$_pmoC0C({ target: __sveltets_2_any(), props: {}});}

packages/svelte2tsx/test/htmlx2jsx/samples/editing-unclosed-component-no-attr.v5/input.svelte

@@ -0,0 +1,9 @@
+<!-- 
+    Don't add trailing newline to this file.
+    A component ends where the file ends is a part of the test. 
+-->
+<div>
+    <Comp
+</div>
+
+<Comp

packages/svelte2tsx/test/htmlx2jsx/samples/editing-unclosed-element-no-attr.v5/expected.error.json

@@ -0,0 +1,20 @@
+{
+    "code": "expected_token",
+    "message": "Expected token >\nhttps://svelte.dev/e/expected_token",
+    "filename": "(unknown)",
+    "start": {
+        "line": 7,
+        "column": 2,
+        "character": 138
+    },
+    "end": {
+        "line": 7,
+        "column": 2,
+        "character": 138
+    },
+    "position": [
+        138,
+        138
+    ],
+    "frame": " 5: <div>\n 6:     <span\n 7: </div>\n      ^\n 8: \n 9: <span"
+}