address CR feedback

microsoft · Apr 18, 2017 · 683a0a0 · 683a0a0
1 parent ba6e22c
commit 683a0a0
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 13 deletions.
diff --git a/src/server/session.ts b/src/server/session.ts
@@ -1447,7 +1447,7 @@ namespace ts.server {
         }
 
         private extractPositionAndRange(args: protocol.FileLocationOrRangeRequestArgs, scriptInfo: ScriptInfo): { position: number, textRange: TextRange } {
-            let position: number;
+            let position: number = undefined;
             let textRange: TextRange;
             if (this.isLocation(args)) {
                 position = getPosition(args);
@@ -1481,7 +1481,7 @@ namespace ts.server {
                 position || textRange,
                 args.refactorName
             );
-            return map(result, action => this.mapCodeAction(action, scriptInfo));
+            return result ? map(result, action => this.mapCodeAction(action, scriptInfo)) : undefined;
         }
 
         private getCodeFixes(args: protocol.CodeFixRequestArgs, simplifiedResult: boolean): protocol.CodeAction[] | CodeAction[] {
@@ -1507,12 +1507,24 @@ namespace ts.server {
         }
 
         private getStartAndEndPosition(args: protocol.FileRangeRequestArgs, scriptInfo: ScriptInfo) {
-            const startPosition = args.startPosition !== undefined
-                ? args.startPosition
-                : args.startPosition = scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset);
-            const endPosition = args.endPosition !== undefined
-                ? args.endPosition
-                : args.startPosition = scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset);
+            let startPosition: number = undefined, endPosition: number = undefined;
+            if (args.startPosition !== undefined ) {
+                startPosition = args.startPosition;
+            }
+            else {
+                startPosition = scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset);
+                // save the result so we don't always recompute
+                args.startPosition = startPosition;
+            }
+
+            if (args.endPosition !== undefined) {
+                endPosition = args.endPosition;
+            }
+            else {
+                endPosition = scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset);
+                args.endPosition = endPosition;
+            }
+
             return { startPosition, endPosition };
         }
 

diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts
@@ -14,6 +14,11 @@ namespace ts {
         isApplicableForPositionOrRange(context: LightRefactorContext, positionOrRange: number | TextRange): boolean;
     }
 
+    /**
+     * The `GetApplicableRefactor` API call is supposed to be fast, therefore only syntactic checks should be conducted
+     * to see if a refactor is applicable. The `LightRefactorContent` limits the context information accesable to the
+     * refactor to enforce such design. Such context should not provide a bound source file with symbols.
+     */
     export interface LightRefactorContext {
         /**
          * The AST that was not bound, so the symbols associated with the nodes are not accessible.
@@ -55,17 +60,17 @@ namespace ts {
         export function getRefactorCodeActions(
             context: RefactorContext,
             positionOrRange: number | TextRange,
-            refactorName: string) {
+            refactorName: string): CodeAction[] | undefined {
 
-            const result: CodeAction[] = [];
+            let result: CodeAction[];
             const refactor = refactors.get(refactorName);
             if (!refactor) {
                 return undefined;
             }
 
             const codeActions = refactor.getCodeActions(context, positionOrRange);
             if (codeActions) {
-                addRange(result, codeActions);
+                addRange((result || (result = [])), codeActions);
             }
             return result;
         }

diff --git a/src/services/services.ts b/src/services/services.ts
@@ -1999,7 +1999,7 @@ namespace ts {
             fileName: string,
             formatOptions: FormatCodeSettings,
             positionOrRange: number | TextRange,
-            refactorName: string): CodeAction[] {
+            refactorName: string): CodeAction[] | undefined {
 
             const context: RefactorContext = {
                 boundSourceFile: getValidSourceFile(fileName),

diff --git a/src/services/types.ts b/src/services/types.ts
@@ -264,7 +264,7 @@ namespace ts {
         getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: FormatCodeSettings): CodeAction[];
 
         getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[];
-        getRefactorCodeActions(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string): CodeAction[];
+        getRefactorCodeActions(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string): CodeAction[] | undefined;
 
         getEmitOutput(fileName: string, emitOnlyDtsFiles?: boolean): EmitOutput;