Script side implementation for Brace Completion. (#7587)

* Script side implementation for Brace Completion. This needs updated Visual Studio components to work. * Changed CharacterCodes to number, to keep the API simple * CR feedback * CR feedback and more JSX tests * Swapped 2 comments * typo
microsoft · Apr 15, 2016 · 96deb55 · 96deb55
1 parent 19a9f7f
commit 96deb55
Show file tree

Hide file tree

Showing 12 changed files with 267 additions and 11 deletions.
diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts
@@ -655,7 +655,7 @@ namespace FourSlash {
                 this.assertItemInCompletionList(completions.entries, symbol, text, documentation, kind);
             }
             else {
-                this.raiseError(`No completions at position '${ this.currentCaretPosition }' when looking for '${ symbol }'.`);
+                this.raiseError(`No completions at position '${this.currentCaretPosition}' when looking for '${symbol}'.`);
             }
         }
 
@@ -1758,13 +1758,13 @@ namespace FourSlash {
                 const actual = (<ts.server.SessionClient>this.languageService).getProjectInfo(
                     this.activeFile.fileName,
                     /* needFileNameList */ true
-                    );
+                );
                 assert.equal(
                     expected.join(","),
-                    actual.fileNames.map( file => {
+                    actual.fileNames.map(file => {
                         return file.replace(this.basePath + "/", "");
-                        }).join(",")
-                    );
+                    }).join(",")
+                );
             }
         }
 
@@ -1850,6 +1850,37 @@ namespace FourSlash {
             });
         }
 
+        public verifyBraceCompletionAtPostion(negative: boolean, openingBrace: string) {
+
+            const openBraceMap: ts.Map<ts.CharacterCodes> = {
+                "(": ts.CharacterCodes.openParen,
+                "{": ts.CharacterCodes.openBrace,
+                "[": ts.CharacterCodes.openBracket,
+                "'": ts.CharacterCodes.singleQuote,
+                '"': ts.CharacterCodes.doubleQuote,
+                "`": ts.CharacterCodes.backtick,
+                "<": ts.CharacterCodes.lessThan
+            };
+
+            const charCode = openBraceMap[openingBrace];
+
+            if (!charCode) {
+                this.raiseError(`Invalid openingBrace '${openingBrace}' specified.`);
+            }
+
+            const position = this.currentCaretPosition;
+
+            const validBraceCompletion = this.languageService.isValidBraceCompletionAtPostion(this.activeFile.fileName, position, charCode);
+
+            if (!negative && !validBraceCompletion) {
+                this.raiseError(`${position} is not a valid brace completion position for ${openingBrace}`);
+            }
+
+            if (negative && validBraceCompletion) {
+                this.raiseError(`${position} is a valid brace completion position for ${openingBrace}`);
+            }
+        }
+
         public verifyMatchingBracePosition(bracePosition: number, expectedMatchPosition: number) {
             const actual = this.languageService.getBraceMatchingAtPosition(this.activeFile.fileName, bracePosition);
 
@@ -2239,7 +2270,7 @@ namespace FourSlash {
         };
 
         const host = Harness.Compiler.createCompilerHost(
-            [ fourslashFile, testFile ],
+            [fourslashFile, testFile],
             (fn, contents) => result = contents,
             ts.ScriptTarget.Latest,
             Harness.IO.useCaseSensitiveFileNames(),
@@ -2264,7 +2295,7 @@ namespace FourSlash {
     function runCode(code: string, state: TestState): void {
         // Compile and execute the test
         const wrappedCode =
-`(function(test, goTo, verify, edit, debug, format, cancellation, classification, verifyOperationIsCancelled) {
+            `(function(test, goTo, verify, edit, debug, format, cancellation, classification, verifyOperationIsCancelled) {
 ${code}
 })`;
         try {
@@ -2378,7 +2409,7 @@ ${code}
                         }
                     }
                 }
-            // TODO: should be '==='?
+                // TODO: should be '==='?
             }
             else if (line == "" || lineLength === 0) {
                 // Previously blank lines between fourslash content caused it to be considered as 2 files,
@@ -2870,6 +2901,10 @@ namespace FourSlashInterface {
         public verifyDefinitionsName(name: string, containerName: string) {
             this.state.verifyDefinitionsName(this.negative, name, containerName);
         }
+
+        public isValidBraceCompletionAtPostion(openingBrace: string) {
+            this.state.verifyBraceCompletionAtPostion(this.negative, openingBrace);
+        }
     }
 
     export class Verify extends VerifyNegatable {
@@ -3088,7 +3123,7 @@ namespace FourSlashInterface {
             this.state.getSemanticDiagnostics(expected);
         }
 
-        public ProjectInfo(expected: string []) {
+        public ProjectInfo(expected: string[]) {
             this.state.verifyProjectInfo(expected);
         }
     }

diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts
@@ -437,6 +437,9 @@ namespace Harness.LanguageService {
         getDocCommentTemplateAtPosition(fileName: string, position: number): ts.TextInsertion {
             return unwrapJSONCallResult(this.shim.getDocCommentTemplateAtPosition(fileName, position));
         }
+        isValidBraceCompletionAtPostion(fileName: string, position: number, openingBrace: number): boolean {
+            return unwrapJSONCallResult(this.shim.isValidBraceCompletionAtPostion(fileName, position, openingBrace));
+        }
         getEmitOutput(fileName: string): ts.EmitOutput {
             return unwrapJSONCallResult(this.shim.getEmitOutput(fileName));
         }

diff --git a/src/server/client.ts b/src/server/client.ts
@@ -568,6 +568,10 @@ namespace ts.server {
             throw new Error("Not Implemented Yet."); 
         }
 
+        isValidBraceCompletionAtPostion(fileName: string, position: number, openingBrace: number): boolean {
+            throw new Error("Not Implemented Yet."); 
+        }
+
         getBraceMatchingAtPosition(fileName: string, position: number): TextSpan[] {
             var lineOffset = this.positionToOneBasedLineOffset(fileName, position);
             var args: protocol.FileLocationRequestArgs = {

diff --git a/src/services/services.ts b/src/services/services.ts
@@ -1113,6 +1113,8 @@ namespace ts {
 
         getDocCommentTemplateAtPosition(fileName: string, position: number): TextInsertion;
 
+        isValidBraceCompletionAtPostion(fileName: string, position: number, openingBrace: number): boolean;
+
         getEmitOutput(fileName: string): EmitOutput;
 
         getProgram(): Program;
@@ -7446,6 +7448,36 @@ namespace ts {
             return { newText: result, caretOffset: preamble.length };
         }
 
+        function isValidBraceCompletionAtPostion(fileName: string, position: number, openingBrace: number): boolean {
+
+            // '<' is currently not supported, figuring out if we're in a Generic Type vs. a comparison is too 
+            // expensive to do during typing scenarios
+            // i.e. whether we're dealing with:
+            //      var x = new foo<| ( with class foo<T>{} )
+            // or 
+            //      var y = 3 <|
+            if (openingBrace === CharacterCodes.lessThan) {
+                return false;
+            }
+
+            const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName);
+
+            // Check if in a context where we don't want to perform any insertion
+            if (isInString(sourceFile, position) || isInComment(sourceFile, position)) {
+                return false;
+            }
+
+            if (isInsideJsxElementOrAttribute(sourceFile, position)) {
+                return openingBrace === CharacterCodes.openBrace;
+            }
+
+            if (isInTemplateString(sourceFile, position)) {
+                return false;
+            }
+
+            return true;
+        }
+
         function getParametersForJsDocOwningNode(commentOwner: Node): ParameterDeclaration[] {
             if (isFunctionLike(commentOwner)) {
                 return commentOwner.parameters;
@@ -7740,6 +7772,7 @@ namespace ts {
             getFormattingEditsForDocument,
             getFormattingEditsAfterKeystroke,
             getDocCommentTemplateAtPosition,
+            isValidBraceCompletionAtPostion,
             getEmitOutput,
             getNonBoundSourceFile,
             getProgram

diff --git a/src/services/shims.ts b/src/services/shims.ts
@@ -221,6 +221,13 @@ namespace ts {
          */
         getDocCommentTemplateAtPosition(fileName: string, position: number): string;
 
+        /**
+         * Returns JSON-encoded boolean to indicate whether we should support brace location
+         * at the current position.
+         * E.g. we don't want brace completion inside string-literals, comments, etc.
+         */
+        isValidBraceCompletionAtPostion(fileName: string, position: number, openingBrace: number): string;
+
         getEmitOutput(fileName: string): string;
     }
 
@@ -733,6 +740,13 @@ namespace ts {
             );
         }
 
+        public isValidBraceCompletionAtPostion(fileName: string, position: number, openingBrace: number): string {
+            return this.forwardJSONCall(
+                `isValidBraceCompletionAtPostion('${fileName}', ${position}, ${openingBrace})`,
+                () => this.languageService.isValidBraceCompletionAtPostion(fileName, position, openingBrace)
+            );
+        }
+
         /// GET SMART INDENT
         public getIndentationAtPosition(fileName: string, position: number, options: string /*Services.EditorOptions*/): string {
             return this.forwardJSONCall(

diff --git a/src/services/utilities.ts b/src/services/utilities.ts
@@ -403,21 +403,61 @@ namespace ts {
 
     export function isInString(sourceFile: SourceFile, position: number) {
         let token = getTokenAtPosition(sourceFile, position);
-        return token && (token.kind === SyntaxKind.StringLiteral || token.kind === SyntaxKind.StringLiteralType) && position > token.getStart();
+        return token && (token.kind === SyntaxKind.StringLiteral || token.kind === SyntaxKind.StringLiteralType) && position > token.getStart(sourceFile);
     }
 
     export function isInComment(sourceFile: SourceFile, position: number) {
         return isInCommentHelper(sourceFile, position, /*predicate*/ undefined);
     }
 
+    /**
+     * returns true if the position is in between the open and close elements of an JSX expression.
+     */
+    export function isInsideJsxElementOrAttribute(sourceFile: SourceFile, position: number) {
+        let token = getTokenAtPosition(sourceFile, position);
+
+        if (!token) {
+            return false;
+        }
+
+        // <div>Hello |</div>
+        if (token.kind === SyntaxKind.LessThanToken && token.parent.kind === SyntaxKind.JsxText) {
+            return true;
+        }
+
+        // <div> { | </div> or <div a={| </div>
+        if (token.kind === SyntaxKind.LessThanToken && token.parent.kind === SyntaxKind.JsxExpression) {
+            return true;
+        }
+
+        // <div> { 
+        // |
+        // } < /div>
+        if (token && token.kind === SyntaxKind.CloseBraceToken && token.parent.kind === SyntaxKind.JsxExpression) {
+            return true;
+        }
+
+        // <div>|</div>
+        if (token.kind === SyntaxKind.LessThanToken && token.parent.kind === SyntaxKind.JsxClosingElement) {
+            return true;
+        }
+
+        return false;
+    }
+
+    export function isInTemplateString(sourceFile: SourceFile, position: number) {
+        let token = getTokenAtPosition(sourceFile, position);
+        return isTemplateLiteralKind(token.kind) && position > token.getStart(sourceFile);
+    }
+
     /**
      * Returns true if the cursor at position in sourceFile is within a comment that additionally
      * satisfies predicate, and false otherwise.
      */
     export function isInCommentHelper(sourceFile: SourceFile, position: number, predicate?: (c: CommentRange) => boolean): boolean {
         let token = getTokenAtPosition(sourceFile, position);
 
-        if (token && position <= token.getStart()) {
+        if (token && position <= token.getStart(sourceFile)) {
             let commentRanges = getLeadingCommentRanges(sourceFile.text, token.pos);
 
             // The end marker of a single-line comment does not include the newline character.

diff --git a/tests/cases/fourslash/commentBraceCompletionPosition.ts b/tests/cases/fourslash/commentBraceCompletionPosition.ts
@@ -0,0 +1,23 @@
+/// <reference path="fourslash.ts" />
+
+//// /**
+////  *  inside jsdoc /*1*/
+////  */
+//// function f() {
+////     // inside regular comment /*2*/
+////     var c = "";
+////
+////    /* inside multi-
+////       line comment /*3*/ 
+////    */
+////    var y =12;
+//// }
+
+goTo.marker('1');
+verify.not.isValidBraceCompletionAtPostion('(');
+
+goTo.marker('2');
+verify.not.isValidBraceCompletionAtPostion('(');
+
+goTo.marker('3');
+verify.not.isValidBraceCompletionAtPostion('(');
diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts
@@ -136,6 +136,7 @@ declare namespace FourSlashInterface {
         typeDefinitionCountIs(expectedCount: number): void;
         definitionLocationExists(): void;
         verifyDefinitionsName(name: string, containerName: string): void;
+        isValidBraceCompletionAtPostion(openingBrace?: string): void;
     }
     class verify extends verifyNegatable {
         assertHasRanges(ranges: FourSlash.Range[]): void;
@@ -173,6 +174,7 @@ declare namespace FourSlashInterface {
         noMatchingBracePositionInCurrentFile(bracePosition: number): void;
         DocCommentTemplate(expectedText: string, expectedOffset: number, empty?: boolean): void;
         noDocCommentTemplate(): void;
+
         getScriptLexicalStructureListCount(count: number): void;
         getScriptLexicalStructureListContains(name: string, kind: string, fileName?: string, parentName?: string, isAdditionalSpan?: boolean, markerPosition?: number): void;
         navigationItemsListCount(count: number, searchValue: string, matchKind?: string): void;

diff --git a/tests/cases/fourslash/jsxBraceCompletionPosition.ts b/tests/cases/fourslash/jsxBraceCompletionPosition.ts
@@ -0,0 +1,47 @@
+/// <reference path="fourslash.ts" />
+
+//@Filename: file.tsx
+//// declare var React: any;
+////
+//// var x = <div a="/*2*/" b={/*3*/}>
+//// /*1*/
+//// </div>;
+//// var y = <div>/*4*/</div>
+//// var z = <div>
+////     hello /*5*/
+//// </div>
+//// var z2 = <div> { /*6*/
+//// </div>
+//// var z3 = <div>
+////     {
+//// /*7*/
+//// }
+//// </div>
+
+goTo.marker('1');
+verify.not.isValidBraceCompletionAtPostion('(');
+verify.isValidBraceCompletionAtPostion('{');
+
+goTo.marker('2');
+verify.not.isValidBraceCompletionAtPostion('(');
+verify.not.isValidBraceCompletionAtPostion('{');
+
+goTo.marker('3');
+verify.not.isValidBraceCompletionAtPostion('(');
+verify.isValidBraceCompletionAtPostion('{');
+
+goTo.marker('4');
+verify.not.isValidBraceCompletionAtPostion('(');
+verify.isValidBraceCompletionAtPostion('{');
+
+goTo.marker('5');
+verify.not.isValidBraceCompletionAtPostion('(');
+verify.isValidBraceCompletionAtPostion('{');
+
+goTo.marker('6');
+verify.not.isValidBraceCompletionAtPostion('(');
+verify.isValidBraceCompletionAtPostion('{');
+
+goTo.marker('7');
+verify.not.isValidBraceCompletionAtPostion('(');
+verify.isValidBraceCompletionAtPostion('{');
diff --git a/tests/cases/fourslash/stringBraceCompletionPosition.ts b/tests/cases/fourslash/stringBraceCompletionPosition.ts
@@ -0,0 +1,16 @@
+/// <reference path="fourslash.ts" />
+
+//// var x = "/*1*/";
+//// var x = '/*2*/';
+//// var x = "hello \
+////     /*3*/";
+
+goTo.marker('1');
+verify.not.isValidBraceCompletionAtPostion('(');
+
+goTo.marker('2');
+verify.not.isValidBraceCompletionAtPostion('(');
+
+goTo.marker('3');
+verify.not.isValidBraceCompletionAtPostion('(');
+