From 21f192367adf0f8ecaf5dc9c8ee972f1fd292d1d Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 26 Aug 2019 10:42:17 -0700 Subject: [PATCH] Fix infer from usage prop assignment (#33088) * Add test case * Fix infer from usage property assignment Property assignment and shorthand property assignment were incorrectly treated differently; both have ObjectLiteralExpression as a parent, but the code previously assumed that property assignments had ObjectLiteralExpression as parent.parent. Also make fourslash directives case insensitive and less whitespace sensitive. * Add "incorrect 3-slash" error to fourslash parsing. --- src/harness/fourslash.ts | 20 ++++++++++++------- src/services/codefixes/inferFromUsage.ts | 10 +++------- .../codeFixInferFromUsageJSDestructuring.ts | 18 +++++++++++++++++ .../codeFixInferFromUsageJSXElement.ts | 9 +-------- 4 files changed, 35 insertions(+), 22 deletions(-) create mode 100644 tests/cases/fourslash/codeFixInferFromUsageJSDestructuring.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 81c83a07bd7..ae16159bc9d 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -80,11 +80,11 @@ namespace FourSlash { // To add additional option, add property into the testOptMetadataNames, refer the property in either globalMetadataNames or fileMetadataNames // Add cases into convertGlobalOptionsToCompilationsSettings function for the compiler to acknowledge such option from meta data const enum MetadataOptionNames { - baselineFile = "BaselineFile", - emitThisFile = "emitThisFile", // This flag is used for testing getEmitOutput feature. It allows test-cases to indicate what file to be output in multiple files project - fileName = "Filename", - resolveReference = "ResolveReference", // This flag is used to specify entry file for resolve file references. The flag is only allow once per test file - symlink = "Symlink", + baselineFile = "baselinefile", + emitThisFile = "emitthisfile", // This flag is used for testing getEmitOutput feature. It allows test-cases to indicate what file to be output in multiple files project + fileName = "filename", + resolveReference = "resolvereference", // This flag is used to specify entry file for resolve file references. The flag is only allow once per test file + symlink = "symlink", } // List of allowed metadata names @@ -3281,7 +3281,7 @@ ${code} function parseTestData(basePath: string, contents: string, fileName: string): FourSlashData { // Regex for parsing options in the format "@Alpha: Value of any sort" - const optionRegex = /^\s*@(\w+): (.*)\s*/; + const optionRegex = /^\s*@(\w+):\s*(.*)\s*/; // List of all the subfiles we've parsed out const files: FourSlashFile[] = []; @@ -3293,6 +3293,7 @@ ${code} // Note: IE JS engine incorrectly handles consecutive delimiters here when using RegExp split, so // we have to string-based splitting instead and try to figure out the delimiting chars const lines = contents.split("\n"); + let i = 0; const markerPositions = ts.createMap(); const markers: Marker[] = []; @@ -3321,6 +3322,7 @@ ${code} } for (let line of lines) { + i++; if (line.length > 0 && line.charAt(line.length - 1) === "\r") { line = line.substr(0, line.length - 1); } @@ -3329,11 +3331,15 @@ ${code} const text = line.substr(4); currentFileContent = currentFileContent === undefined ? text : currentFileContent + "\n" + text; } + else if (line.substr(0, 3) === "///" && currentFileContent !== undefined) { + throw new Error("Three-slash line in the middle of four-slash region at line " + i); + } else if (line.substr(0, 2) === "//") { // Comment line, check for global/file @options and record them const match = optionRegex.exec(line.substr(2)); if (match) { - const [key, value] = match.slice(1); + const key = match[1].toLowerCase(); + const value = match[2]; if (!ts.contains(fileMetadataNames, key)) { // Check if the match is already existed in the global options if (globalOptions[key] !== undefined) { diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index 82a3a9d229b..396d7f44fa8 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -717,13 +717,9 @@ namespace ts.codefix { } function inferTypeFromPropertyAssignment(assignment: PropertyAssignment | ShorthandPropertyAssignment, checker: TypeChecker, usageContext: UsageContext) { - const objectLiteral = isShorthandPropertyAssignment(assignment) ? - assignment.parent : - assignment.parent.parent; - const nodeWithRealType = isVariableDeclaration(objectLiteral.parent) ? - objectLiteral.parent : - objectLiteral; - + const nodeWithRealType = isVariableDeclaration(assignment.parent.parent) ? + assignment.parent.parent : + assignment.parent; addCandidateThisType(usageContext, checker.getTypeAtLocation(nodeWithRealType)); } diff --git a/tests/cases/fourslash/codeFixInferFromUsageJSDestructuring.ts b/tests/cases/fourslash/codeFixInferFromUsageJSDestructuring.ts new file mode 100644 index 00000000000..fb394c3507a --- /dev/null +++ b/tests/cases/fourslash/codeFixInferFromUsageJSDestructuring.ts @@ -0,0 +1,18 @@ +/// +// @allowJs: true +// @checkJs: true +// @noEmit: true +// @filename:destruct.js +//// function [|formatter|](message) { +//// const { type } = false ? { type: message } : message; +//// } +verify.codeFix({ + description: "Infer parameter types from usage", + index: 0, + newFileContent: `/** + * @param {{ type: any; }} message + */ +function formatter(message) { + const { type } = false ? { type: message } : message; +}` +}); diff --git a/tests/cases/fourslash/codeFixInferFromUsageJSXElement.ts b/tests/cases/fourslash/codeFixInferFromUsageJSXElement.ts index 250673d91fc..3200fffafb8 100644 --- a/tests/cases/fourslash/codeFixInferFromUsageJSXElement.ts +++ b/tests/cases/fourslash/codeFixInferFromUsageJSXElement.ts @@ -5,9 +5,6 @@ // @module: es2015 // @moduleResolution: node -// @Filename: /node_modules/@types/react/index.d.ts -////export = React; -////export as namespace React; ////declare namespace React { //// export class Component { render(): JSX.Element | null; } ////} @@ -16,10 +13,6 @@ //// interface Element {} //// } ////} - - -// @filename: a.tsx -//// import React from 'react'; //// //// export default function Component([|props |]) { //// if (props.isLoading) { @@ -37,4 +30,4 @@ //// } -verify.rangeAfterCodeFix("props: { isLoading: any; update: (arg0: any) => void; }",/*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, 0); \ No newline at end of file +verify.rangeAfterCodeFix("props: { isLoading: any; update: (arg0: any) => void; }",/*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, 0);