importFixes: Support missing "React" at a JSXOpeningElement (#16066)

* importFixes: Support missing "React" at a JSXOpeningElement * Fix nextLineRule linting * Rename to resolveJsxNamespaceAtLocation * Expose getJsxNamespace and resolveNameAtLocation separately
microsoft · Jun 5, 2017 · 8ace7b8 · 8ace7b8
1 parent 7056411
commit 8ace7b8
Show file tree

Hide file tree

Showing 12 changed files with 130 additions and 21 deletions.
diff --git a/scripts/tslint/nextLineRule.ts b/scripts/tslint/nextLineRule.ts
@@ -18,7 +18,7 @@ export class Rule extends Lint.Rules.AbstractRule {
 
 function walk(ctx: Lint.WalkContext<void>, checkCatch: boolean, checkElse: boolean): void {
     const { sourceFile } = ctx;
-    function recur(node: ts.Node): void {
+    ts.forEachChild(sourceFile, function recur(node) {
         switch (node.kind) {
             case ts.SyntaxKind.IfStatement:
                 checkIf(node as ts.IfStatement);
@@ -28,7 +28,7 @@ function walk(ctx: Lint.WalkContext<void>, checkCatch: boolean, checkElse: boole
                 break;
         }
         ts.forEachChild(node, recur);
-    }
+    });
 
     function checkIf(node: ts.IfStatement): void {
         const { thenStatement, elseStatement } = node;

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -210,6 +210,11 @@ namespace ts {
             getSuggestionForNonexistentProperty,
             getSuggestionForNonexistentSymbol,
             getBaseConstraintOfType,
+            getJsxNamespace,
+            resolveNameAtLocation(location: Node, name: string, meaning: SymbolFlags): Symbol | undefined {
+                location = getParseTreeNode(location);
+                return resolveName(location, name, meaning, /*nameNotFoundMessage*/ undefined, name);
+            },
         };
 
         const tupleTypes: GenericType[] = [];
@@ -847,7 +852,7 @@ namespace ts {
             location: Node | undefined,
             name: string,
             meaning: SymbolFlags,
-            nameNotFoundMessage: DiagnosticMessage,
+            nameNotFoundMessage: DiagnosticMessage | undefined,
             nameArg: string | Identifier,
             suggestedNameNotFoundMessage?: DiagnosticMessage): Symbol {
             return resolveNameHelper(location, name, meaning, nameNotFoundMessage, nameArg, getSymbol, suggestedNameNotFoundMessage);
@@ -5841,7 +5846,8 @@ namespace ts {
                     }
                 }
                 return arrayFrom(props.values());
-            } else {
+            }
+            else {
                 return getPropertiesOfType(type);
             }
         }
@@ -14151,7 +14157,7 @@ namespace ts {
             checkJsxPreconditions(node);
             // The reactNamespace/jsxFactory's root symbol should be marked as 'used' so we don't incorrectly elide its import.
             // And if there is no reactNamespace/jsxFactory's symbol in scope when targeting React emit, we should issue an error.
-            const reactRefErr = compilerOptions.jsx === JsxEmit.React ? Diagnostics.Cannot_find_name_0 : undefined;
+            const reactRefErr = diagnostics && compilerOptions.jsx === JsxEmit.React ? Diagnostics.Cannot_find_name_0 : undefined;
             const reactNamespace = getJsxNamespace();
             const reactSym = resolveName(node.tagName, reactNamespace, SymbolFlags.Value, reactRefErr, reactNamespace);
             if (reactSym) {

diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -1479,7 +1479,8 @@ namespace ts {
                     }
                 }
                 return sourceFile;
-            } else {
+            }
+            else {
                 const sourceFileNoExtension = options.allowNonTsExtensions && getSourceFile(fileName);
                 if (sourceFileNoExtension) return sourceFileNoExtension;
 

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -2605,6 +2605,9 @@ namespace ts {
          * Does not include properties of primitive types.
          */
         /* @internal */ getAllPossiblePropertiesOfType(type: Type): Symbol[];
+
+        /* @internal */ getJsxNamespace(): string;
+        /* @internal */ resolveNameAtLocation(location: Node, name: string, meaning: SymbolFlags): Symbol | undefined;
     }
 
     export enum NodeBuilderFlags {

diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts
@@ -2372,7 +2372,10 @@ namespace FourSlash {
             const codeFixes = this.getCodeFixActions(this.activeFile.fileName, errorCode);
 
             if (!codeFixes || codeFixes.length === 0) {
-                this.raiseError("No codefixes returned.");
+                if (expectedTextArray.length !== 0) {
+                    this.raiseError("No codefixes returned.");
+                }
+                return;
             }
 
             const actualTextArray: string[] = [];

diff --git a/src/harness/unittests/telemetry.ts b/src/harness/unittests/telemetry.ts
@@ -252,7 +252,8 @@ namespace ts.projectSystem {
         }
 
         getEvent<T extends server.ProjectServiceEvent>(eventName: T["eventName"], mayBeMore = false): T["data"] {
-            if (mayBeMore) assert(this.events.length !== 0); else assert.equal(this.events.length, 1);
+            if (mayBeMore) { assert(this.events.length !== 0); }
+            else { assert.equal(this.events.length, 1); }
             const event = this.events.shift();
             assert.equal(event.eventName, eventName);
             return event.data;

diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts
@@ -138,8 +138,23 @@ namespace ts.codefix {
 
         const currentTokenMeaning = getMeaningFromLocation(token);
         if (context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code) {
-            const symbol = checker.getAliasedSymbol(checker.getSymbolAtLocation(token));
-            return getCodeActionForImport(symbol, /*isDefault*/ false, /*isNamespaceImport*/ true);
+            const umdSymbol = checker.getSymbolAtLocation(token);
+            let symbol: ts.Symbol;
+            let symbolName: string;
+            if (umdSymbol.flags & ts.SymbolFlags.Alias) {
+                symbol = checker.getAliasedSymbol(umdSymbol);
+                symbolName = name;
+            }
+            else if (isJsxOpeningLikeElement(token.parent) && token.parent.tagName === token) {
+                // The error wasn't for the symbolAtLocation, it was for the JSX tag itself, which needs access to e.g. `React`.
+                symbol = checker.getAliasedSymbol(checker.resolveNameAtLocation(token, checker.getJsxNamespace(), SymbolFlags.Value));
+                symbolName = symbol.name;
+            }
+            else {
+                Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here");
+            }
+
+            return getCodeActionForImport(symbol, symbolName, /*isDefault*/ false, /*isNamespaceImport*/ true);
         }
 
         const candidateModules = checker.getAmbientModules();
@@ -159,15 +174,15 @@ namespace ts.codefix {
                 if (localSymbol && localSymbol.name === name && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) {
                     // check if this symbol is already used
                     const symbolId = getUniqueSymbolId(localSymbol);
-                    symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, /*isDefault*/ true));
+                    symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, name, /*isDefault*/ true));
                 }
             }
 
             // check exports with the same name
             const exportSymbolWithIdenticalName = checker.tryGetMemberInModuleExports(name, moduleSymbol);
             if (exportSymbolWithIdenticalName && checkSymbolHasMeaning(exportSymbolWithIdenticalName, currentTokenMeaning)) {
                 const symbolId = getUniqueSymbolId(exportSymbolWithIdenticalName);
-                symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol));
+                symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, name));
             }
         }
 
@@ -218,7 +233,7 @@ namespace ts.codefix {
             return declarations ? some(symbol.declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)) : false;
         }
 
-        function getCodeActionForImport(moduleSymbol: Symbol, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] {
+        function getCodeActionForImport(moduleSymbol: Symbol, symbolName: string, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] {
             const existingDeclarations = getImportDeclarations(moduleSymbol);
             if (existingDeclarations.length > 0) {
                 // With an existing import statement, there are more than one actions the user can do.
@@ -375,10 +390,10 @@ namespace ts.codefix {
                 const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier || getModuleSpecifierForNewImport());
                 const changeTracker = createChangeTracker();
                 const importClause = isDefault
-                    ? createImportClause(createIdentifier(name), /*namedBindings*/ undefined)
+                    ? createImportClause(createIdentifier(symbolName), /*namedBindings*/ undefined)
                     : isNamespaceImport
-                        ? createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(name)))
-                        : createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(name))]));
+                        ? createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(symbolName)))
+                        : createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName))]));
                 const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, createLiteral(moduleSpecifierWithoutQuotes));
                 if (!lastImportDeclaration) {
                     changeTracker.insertNodeAt(sourceFile, sourceFile.getStart(), importDecl, { suffix: `${context.newLineCharacter}${context.newLineCharacter}` });
@@ -392,7 +407,7 @@ namespace ts.codefix {
                 // are there are already a new line seperating code and import statements.
                 return createCodeAction(
                     Diagnostics.Import_0_from_1,
-                    [name, `"${moduleSpecifierWithoutQuotes}"`],
+                    [symbolName, `"${moduleSpecifierWithoutQuotes}"`],
                     changeTracker.getChanges(),
                     "NewImport",
                     moduleSpecifierWithoutQuotes
@@ -412,8 +427,9 @@ namespace ts.codefix {
                         removeFileExtension(getRelativePath(moduleFileName, sourceDirectory));
 
                     function tryGetModuleNameFromAmbientModule(): string {
-                        if (moduleSymbol.valueDeclaration.kind !== SyntaxKind.SourceFile) {
-                            return moduleSymbol.name;
+                        const decl = moduleSymbol.valueDeclaration;
+                        if (isModuleDeclaration(decl) && isStringLiteral(decl.name)) {
+                            return decl.name.text;
                         }
                     }
 

diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts
@@ -185,7 +185,8 @@ namespace ts.FindAllReferences {
         if (entry.type === "node") {
             const { node } = entry;
             return { textSpan: getTextSpan(node), fileName: node.getSourceFile().fileName, ...implementationKindDisplayParts(node, checker) };
-        } else {
+        }
+        else {
             const { textSpan, fileName } = entry;
             return { textSpan, fileName, kind: ScriptElementKind.unknown, displayParts: [] };
         }

diff --git a/src/services/importTracker.ts b/src/services/importTracker.ts
@@ -530,7 +530,8 @@ namespace ts.FindAllReferences {
         if (parent.kind === SyntaxKind.VariableDeclaration) {
             const p = parent as ts.VariableDeclaration;
             return p.parent.kind === ts.SyntaxKind.CatchClause ? undefined : p.parent.parent.kind === SyntaxKind.VariableStatement ? p.parent.parent : undefined;
-        } else {
+        }
+        else {
             return parent;
         }
     }

diff --git a/tests/cases/fourslash/importNameCodeFixUMDGlobalReact0.ts b/tests/cases/fourslash/importNameCodeFixUMDGlobalReact0.ts
@@ -0,0 +1,28 @@
+/// <reference path="fourslash.ts" />
+
+// @jsx: react
+
+// @Filename: /node_modules/@types/react/index.d.ts
+////export = React;
+////export as namespace React;
+////declare namespace React {
+////    export class Component { render(): JSX.Element | null; }
+////}
+////declare global {
+////    namespace JSX {
+////        interface Element {}
+////    }
+////}
+
+// @Filename: /a.tsx
+////[|import { Component } from "react";
+////export class MyMap extends Component { }
+////<MyMap/>;|]
+
+goTo.file("/a.tsx");
+
+verify.importFixAtPosition([
+`import { Component } from "react";
+import * as React from "react";
+export class MyMap extends Component { }
+<MyMap/>;`]);
diff --git a/tests/cases/fourslash/importNameCodeFixUMDGlobalReact1.ts b/tests/cases/fourslash/importNameCodeFixUMDGlobalReact1.ts
@@ -0,0 +1,28 @@
+/// <reference path="fourslash.ts" />
+
+// @jsx: react
+
+// @Filename: /node_modules/@types/react/index.d.ts
+////export = React;
+////export as namespace React;
+////declare namespace React {
+////    export class Component { render(): JSX.Element | null; }
+////}
+////declare global {
+////    namespace JSX {
+////        interface Element {}
+////    }
+////}
+
+// @Filename: /a.tsx
+////[|import { Component } from "react";
+////export class MyMap extends Component { }
+////<MyMap></MyMap>;|]
+
+goTo.file("/a.tsx");
+
+verify.importFixAtPosition([
+`import { Component } from "react";
+import * as React from "react";
+export class MyMap extends Component { }
+<MyMap></MyMap>;`]);
diff --git a/tests/cases/fourslash/importNameCodeFixUMDGlobalReact2.ts b/tests/cases/fourslash/importNameCodeFixUMDGlobalReact2.ts
@@ -0,0 +1,21 @@
+/// <reference path="fourslash.ts" />
+
+// https://github.com/Microsoft/TypeScript/issues/16065
+
+// @jsx: react
+// @jsxFactory: factory
+
+// @Filename: /factory.ts
+////export function factory() { return {}; }
+////declare global {
+////    namespace JSX {
+////        interface Element {}
+////    }
+////}
+
+// @Filename: /a.tsx
+////[|<div/>|]
+
+goTo.file("/a.tsx");
+verify.not
+verify.importFixAtPosition([]);