From a7319dbbbbd5e495d3cc0a1537b4ffe8d582bf3d Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Mon, 24 Apr 2017 13:58:30 -0700 Subject: [PATCH] Fix optional flow parenthesis (#1357) In #1251, we now have a proper whitelist of all the types that should have parenthesis. Turns out, it included NullableTypeAnnotation which is `?a`. For `?a => void` this wasn't needed but for `(?(a => b)) => c` it was! It's better to always put it anyway if it's not just a simple literal. I've added tests for all the combinations I could think of, so we'll catch regressions if they happen. Fixes #1353 --- src/printer.js | 8 ++-- .../__snapshots__/jsfmt.spec.js.snap | 2 +- .../__snapshots__/jsfmt.spec.js.snap | 44 ++++++++++++++++++- tests/flow_return_arrow/issue-1249.js | 20 +++++++++ 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/printer.js b/src/printer.js index 49d44f39..d16aca3f 100644 --- a/src/printer.js +++ b/src/printer.js @@ -2401,7 +2401,6 @@ function printFunctionParams(path, print, options, expandArg) { const flowTypeAnnotations = [ "AnyTypeAnnotation", "NullLiteralTypeAnnotation", - "NullableTypeAnnotation", "GenericTypeAnnotation", "ThisTypeAnnotation", "NumberTypeAnnotation", @@ -2411,14 +2410,17 @@ function printFunctionParams(path, print, options, expandArg) { "MixedTypeAnnotation", "BooleanTypeAnnotation", "BooleanLiteralTypeAnnotation", - "StringLiteralTypeAnnotation", "StringTypeAnnotation" ]; const isFlowShorthandWithOneArg = (isObjectTypePropertyAFunction(parent) || isTypeAnnotationAFunction(parent) || - parent.type === "TypeAlias") && + parent.type === "TypeAlias" || + parent.type === "UnionTypeAnnotation" || + parent.type === "IntersectionTypeAnnotation" || + (parent.type === "FunctionTypeAnnotation" && + parent.returnType === fun)) && fun[paramsField].length === 1 && fun[paramsField][0].name === null && fun[paramsField][0].typeAnnotation && diff --git a/tests/flow_function_parentheses/__snapshots__/jsfmt.spec.js.snap b/tests/flow_function_parentheses/__snapshots__/jsfmt.spec.js.snap index d95b8e8d..21e8f65d 100644 --- a/tests/flow_function_parentheses/__snapshots__/jsfmt.spec.js.snap +++ b/tests/flow_function_parentheses/__snapshots__/jsfmt.spec.js.snap @@ -75,7 +75,7 @@ type f = /* comment */ arg => void; type f = arg /* comment */ => void; -type f = ?arg => void; +type f = (?arg) => void; class X { constructor( diff --git a/tests/flow_return_arrow/__snapshots__/jsfmt.spec.js.snap b/tests/flow_return_arrow/__snapshots__/jsfmt.spec.js.snap index 91eedad4..bcb14aa0 100644 --- a/tests/flow_return_arrow/__snapshots__/jsfmt.spec.js.snap +++ b/tests/flow_return_arrow/__snapshots__/jsfmt.spec.js.snap @@ -2,8 +2,48 @@ exports[`issue-1249.js 1`] = ` type Bar = ( number | string ) => number; +type X = (?(number, number) => number) => void; +type X = ?((number, number) => number) => void; +type X = ?(number, number) => number => void; +type X = 1234 => void; +type X = 'abc' => void; +type X = true => void; +type X = false => void; +type X = boolean => void; +type X = number => void; +type X = void => void; +type X = null => void; +type X = any => void; +type X = empty => void; +type X = mixed => void; +type X = string => void; +type X = abc => void; +type X = a | b => void; +type X = (a | b) => void; +type X = a & b => void; +type X = (a & b) => void; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ type Bar = (number | string) => number; +type X = (?(number, number) => number) => void; +type X = ?((number, number) => number) => void; +type X = ?(number, number) => number => void; +type X = (1234) => void; +type X = ("abc") => void; +type X = true => void; +type X = false => void; +type X = boolean => void; +type X = number => void; +type X = void => void; +type X = null => void; +type X = any => void; +type X = empty => void; +type X = mixed => void; +type X = string => void; +type X = abc => void; +type X = a | (b => void); +type X = (a | b) => void; +type X = a & (b => void); +type X = (a & b) => void; `; @@ -14,8 +54,8 @@ const f = (): (a & string => string) => {}; function f(): string => string {} ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ const f = (): (string => string) => {}; -const f = (): a | ((string) => string) => {}; -const f = (): a & ((string) => string) => {}; +const f = (): a | (string => string) => {}; +const f = (): a & (string => string) => {}; function f(): string => string {} `; diff --git a/tests/flow_return_arrow/issue-1249.js b/tests/flow_return_arrow/issue-1249.js index c291e3a9..3f2ff1a2 100644 --- a/tests/flow_return_arrow/issue-1249.js +++ b/tests/flow_return_arrow/issue-1249.js @@ -1 +1,21 @@ type Bar = ( number | string ) => number; +type X = (?(number, number) => number) => void; +type X = ?((number, number) => number) => void; +type X = ?(number, number) => number => void; +type X = 1234 => void; +type X = 'abc' => void; +type X = true => void; +type X = false => void; +type X = boolean => void; +type X = number => void; +type X = void => void; +type X = null => void; +type X = any => void; +type X = empty => void; +type X = mixed => void; +type X = string => void; +type X = abc => void; +type X = a | b => void; +type X = (a | b) => void; +type X = a & b => void; +type X = (a & b) => void;