From eb7718d84d30cda9f50458a617cd51894df25914 Mon Sep 17 00:00:00 2001 From: Gabe Johnson Date: Thu, 2 Jul 2020 21:02:14 -0500 Subject: [PATCH 1/3] Add failing tests for arrow functions We add three failing tests: 1. {()=>{};``} 2. {x=>x;``} 3. {f:x=>x,y:2} We also add a fourth test mirroring the third but with a function block body instead of an expression. The hope is that we get no regressions when we change the representation of arrow functions. --- test/Test/Language/Javascript/ExpressionParser.hs | 2 ++ test/Test/Language/Javascript/StatementParser.hs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/test/Test/Language/Javascript/ExpressionParser.hs b/test/Test/Language/Javascript/ExpressionParser.hs index 63e7e22c..4bf6be9d 100644 --- a/test/Test/Language/Javascript/ExpressionParser.hs +++ b/test/Test/Language/Javascript/ExpressionParser.hs @@ -145,6 +145,8 @@ testExpressionParser = describe "Parse expressions:" $ do testExpr "(a, ...b) => b" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSIdentifier 'a',JSSpreadExpression (JSIdentifier 'b'))) => JSIdentifier 'b'))" testExpr "(a,b=1) => a + b" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSIdentifier 'a',JSOpAssign ('=',JSIdentifier 'b',JSDecimal '1'))) => JSExpressionBinary ('+',JSIdentifier 'a',JSIdentifier 'b')))" testExpr "([a,b]) => a + b" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSArrayLiteral [JSIdentifier 'a',JSComma,JSIdentifier 'b'])) => JSExpressionBinary ('+',JSIdentifier 'a',JSIdentifier 'b')))" + testExpr "{f:()=>{},y:2}" `shouldBe` "Right (JSAstExpression (JSObjectLiteral [JSPropertyNameandValue (JSIdentifier 'f') [JSArrowExpression (()) => JSStatementBlock []],JSPropertyNameandValue (JSIdentifier 'y') [JSDecimal '2']]))" + testExpr "{f:x=>x,y:2}" `shouldBe` "Right (JSAstExpression (JSObjectLiteral [JSPropertyNameandValue (JSIdentifier 'f') [JSArrowExpression (JSIdentifier 'x') => JSIdentifier 'x'],JSPropertyNameandValue (JSIdentifier 'y') [JSDecimal '2']]))" it "generator expression" $ do testExpr "function*(){}" `shouldBe` "Right (JSAstExpression (JSGeneratorExpression '' () (JSBlock [])))" diff --git a/test/Test/Language/Javascript/StatementParser.hs b/test/Test/Language/Javascript/StatementParser.hs index 5e0c98c8..7e0f6929 100644 --- a/test/Test/Language/Javascript/StatementParser.hs +++ b/test/Test/Language/Javascript/StatementParser.hs @@ -21,6 +21,8 @@ testStatementParser = describe "Parse statements:" $ do testStmt "{}" `shouldBe` "Right (JSAstStatement (JSStatementBlock []))" testStmt "{x=1}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSOpAssign ('=',JSIdentifier 'x',JSDecimal '1')]))" testStmt "{x=1;y=2}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSOpAssign ('=',JSIdentifier 'x',JSDecimal '1'),JSSemicolon,JSOpAssign ('=',JSIdentifier 'y',JSDecimal '2')]))" + testStmt "{()=>{};``}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSArrowExpression (()) => JSStatementBlock [],JSSemicolon,JSTemplateLiteral ((),'``',[])]))" + testStmt "{x=>x;``}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSArrowExpression (JSIdentifier 'x') => JSIdentifier 'x',JSSemicolon,JSTemplateLiteral ((),'``',[])]))" testStmt "{{}}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSStatementBlock []]))" testStmt "{{{}}}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSStatementBlock [JSStatementBlock []]]))" From 858b214e4c193dd65d2d2fd8f6d27678c1ba7c53 Mon Sep 17 00:00:00 2001 From: Gabe Johnson Date: Thu, 2 Jul 2020 21:09:44 -0500 Subject: [PATCH 2/3] Change the representation of arrow function bodies The issue with the current parsing rules and representation is that arrow function bodies are not statements, they're either function blocks or expressions. Looking at the specification (https://www.ecma-international.org/ecma-262/#prod-ExpressionBody) we see that the expressions are `AssignmentExpression` in particular. Making this change appears to fix the additional tests, but it breaks minification. --- src/Language/JavaScript/Parser/AST.hs | 12 +++++++++++- src/Language/JavaScript/Parser/Grammar7.y | 16 ++++++++++++---- src/Language/JavaScript/Pretty/Printer.hs | 4 ++++ src/Language/JavaScript/Process/Minify.hs | 6 +++++- .../Test/Language/Javascript/ExpressionParser.hs | 10 +++++----- test/Test/Language/Javascript/StatementParser.hs | 2 +- 6 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/Language/JavaScript/Parser/AST.hs b/src/Language/JavaScript/Parser/AST.hs index 32302727..dda17b3e 100644 --- a/src/Language/JavaScript/Parser/AST.hs +++ b/src/Language/JavaScript/Parser/AST.hs @@ -3,6 +3,7 @@ module Language.JavaScript.Parser.AST ( JSExpression (..) , JSAnnot (..) + , JSConciseBody(..) , JSBinOp (..) , JSUnaryOp (..) , JSSemi (..) @@ -188,7 +189,7 @@ data JSExpression | JSExpressionParen !JSAnnot !JSExpression !JSAnnot -- ^lb,expression,rb | JSExpressionPostfix !JSExpression !JSUnaryOp -- ^expression, operator | JSExpressionTernary !JSExpression !JSAnnot !JSExpression !JSAnnot !JSExpression -- ^cond, ?, trueval, :, falseval - | JSArrowExpression !JSArrowParameterList !JSAnnot !JSStatement -- ^parameter list,arrow,block` + | JSArrowExpression !JSArrowParameterList !JSAnnot !JSConciseBody -- ^parameter list,arrow,body` | JSFunctionExpression !JSAnnot !JSIdent !JSAnnot !(JSCommaList JSExpression) !JSAnnot !JSBlock -- ^fn,name,lb, parameter list,rb,block` | JSGeneratorExpression !JSAnnot !JSAnnot !JSIdent !JSAnnot !(JSCommaList JSExpression) !JSAnnot !JSBlock -- ^fn,*,name,lb, parameter list,rb,block` | JSMemberDot !JSExpression !JSAnnot !JSExpression -- ^firstpart, dot, name @@ -205,6 +206,11 @@ data JSExpression | JSYieldFromExpression !JSAnnot !JSAnnot !JSExpression -- ^yield, *, expr deriving (Data, Eq, Show, Typeable) +data JSConciseBody + = JSConciseFunctionBody !JSBlock + | JSConciseExpressionBody !JSExpression + deriving (Data, Eq, Show, Typeable) + data JSArrowParameterList = JSUnparenthesizedArrowParameter !JSIdent | JSParenthesizedArrowParameterList !JSAnnot !(JSCommaList JSExpression) !JSAnnot @@ -455,6 +461,10 @@ instance ShowStripped JSArrowParameterList where ss (JSUnparenthesizedArrowParameter x) = ss x ss (JSParenthesizedArrowParameterList _ xs _) = ss xs +instance ShowStripped JSConciseBody where + ss (JSConciseFunctionBody b) = ss b + ss (JSConciseExpressionBody e) = ss e + instance ShowStripped JSModuleItem where ss (JSModuleExportDeclaration _ x1) = "JSModuleExportDeclaration (" ++ ss x1 ++ ")" ss (JSModuleImportDeclaration _ x1) = "JSModuleImportDeclaration (" ++ ss x1 ++ ")" diff --git a/src/Language/JavaScript/Parser/Grammar7.y b/src/Language/JavaScript/Parser/Grammar7.y index 13a15269..c4209764 100644 --- a/src/Language/JavaScript/Parser/Grammar7.y +++ b/src/Language/JavaScript/Parser/Grammar7.y @@ -1238,8 +1238,10 @@ FunctionExpression : ArrowFunctionExpression { $1 {- 'ArrowFunctionExpressio | LambdaExpression { $1 {- 'FunctionExpression1' -} } | NamedFunctionExpression { $1 {- 'FunctionExpression2' -} } +-- ArrowFunctionExpression : +-- ( ArrowParameterList ) => ConciseBody See clause 14.2 ArrowFunctionExpression :: { AST.JSExpression } -ArrowFunctionExpression : ArrowParameterList Arrow StatementOrBlock +ArrowFunctionExpression : ArrowParameterList Arrow ConciseBody { AST.JSArrowExpression $1 $2 $3 } ArrowParameterList :: { AST.JSArrowParameterList } @@ -1247,9 +1249,15 @@ ArrowParameterList : PrimaryExpression {%^ toArrowParameterList $1 } | LParen RParen { AST.JSParenthesizedArrowParameterList $1 AST.JSLNil $2 } -StatementOrBlock :: { AST.JSStatement } -StatementOrBlock : Block MaybeSemi { blockToStatement $1 $2 } - | Expression MaybeSemi { expressionToStatement $1 $2 } +-- ConciseBody : +-- { FunctionBody } +-- ExpressionBody +ConciseBody :: { AST.JSConciseBody } +ConciseBody : FunctionBody { AST.JSConciseFunctionBody $1 } + | ExpressionBody { AST.JSConciseExpressionBody $1 } + +ExpressionBody :: { AST.JSExpression } +ExpressionBody : AssignmentExpression { $1 } -- StatementListItem : -- Statement diff --git a/src/Language/JavaScript/Pretty/Printer.hs b/src/Language/JavaScript/Pretty/Printer.hs index 6ef75b0b..179e3dcd 100644 --- a/src/Language/JavaScript/Pretty/Printer.hs +++ b/src/Language/JavaScript/Pretty/Printer.hs @@ -104,6 +104,10 @@ instance RenderJS JSExpression where instance RenderJS JSArrowParameterList where (|>) pacc (JSUnparenthesizedArrowParameter p) = pacc |> p (|>) pacc (JSParenthesizedArrowParameterList lb ps rb) = pacc |> lb |> "(" |> ps |> ")" |> rb + +instance RenderJS JSConciseBody where + (|>) pacc (JSConciseExpressionBody e) = pacc |> e + (|>) pacc (JSConciseFunctionBody b) = pacc |> b -- ----------------------------------------------------------------------------- -- Need an instance of RenderJS for every component of every JSExpression or JSAnnot -- constuctor. diff --git a/src/Language/JavaScript/Process/Minify.hs b/src/Language/JavaScript/Process/Minify.hs index b6121ea8..094c24c2 100644 --- a/src/Language/JavaScript/Process/Minify.hs +++ b/src/Language/JavaScript/Process/Minify.hs @@ -156,7 +156,7 @@ instance MinifyJS JSExpression where -- Non-Terminals fix _ (JSArrayLiteral _ xs _) = JSArrayLiteral emptyAnnot (map fixEmpty xs) emptyAnnot - fix a (JSArrowExpression ps _ ss) = JSArrowExpression (fix a ps) emptyAnnot (fixStmt emptyAnnot noSemi ss) + fix a (JSArrowExpression ps _ fb) = JSArrowExpression (fix a ps) emptyAnnot (fixEmpty fb) fix a (JSAssignExpression lhs op rhs) = JSAssignExpression (fix a lhs) (fixEmpty op) (fixEmpty rhs) fix a (JSAwaitExpression _ ex) = JSAwaitExpression a (fixSpace ex) fix a (JSCallExpression ex _ xs _) = JSCallExpression (fix a ex) emptyAnnot (fixEmpty xs) emptyAnnot @@ -187,6 +187,10 @@ instance MinifyJS JSArrowParameterList where fix _ (JSUnparenthesizedArrowParameter p) = JSUnparenthesizedArrowParameter (fixEmpty p) fix _ (JSParenthesizedArrowParameterList _ ps _) = JSParenthesizedArrowParameterList emptyAnnot (fixEmpty ps) emptyAnnot +instance MinifyJS JSConciseBody where + fix _ (JSConciseExpressionBody e) = JSConciseExpressionBody (fixEmpty e) + fix _ (JSConciseFunctionBody b) = JSConciseFunctionBody (fixEmpty b) + fixVarList :: JSCommaList JSExpression -> JSCommaList JSExpression fixVarList (JSLCons h _ v) = JSLCons (fixVarList h) emptyAnnot (fixEmpty v) fixVarList (JSLOne a) = JSLOne (fixSpace a) diff --git a/test/Test/Language/Javascript/ExpressionParser.hs b/test/Test/Language/Javascript/ExpressionParser.hs index 4bf6be9d..14ad3fba 100644 --- a/test/Test/Language/Javascript/ExpressionParser.hs +++ b/test/Test/Language/Javascript/ExpressionParser.hs @@ -137,15 +137,15 @@ testExpressionParser = describe "Parse expressions:" $ do testExpr "function([a,b]){}" `shouldBe` "Right (JSAstExpression (JSFunctionExpression '' (JSArrayLiteral [JSIdentifier 'a',JSComma,JSIdentifier 'b']) (JSBlock [])))" testExpr "function([a,...b]){}" `shouldBe` "Right (JSAstExpression (JSFunctionExpression '' (JSArrayLiteral [JSIdentifier 'a',JSComma,JSSpreadExpression (JSIdentifier 'b')]) (JSBlock [])))" testExpr "function({a,b}){}" `shouldBe` "Right (JSAstExpression (JSFunctionExpression '' (JSObjectLiteral [JSPropertyIdentRef 'a',JSPropertyIdentRef 'b']) (JSBlock [])))" - testExpr "a => {}" `shouldBe` "Right (JSAstExpression (JSArrowExpression (JSIdentifier 'a') => JSStatementBlock []))" - testExpr "(a) => { a + 2 }" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSIdentifier 'a')) => JSStatementBlock [JSExpressionBinary ('+',JSIdentifier 'a',JSDecimal '2')]))" - testExpr "(a, b) => {}" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSIdentifier 'a',JSIdentifier 'b')) => JSStatementBlock []))" + testExpr "a => {}" `shouldBe` "Right (JSAstExpression (JSArrowExpression (JSIdentifier 'a') => JSBlock []))" + testExpr "(a) => { a + 2 }" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSIdentifier 'a')) => JSBlock [JSExpressionBinary ('+',JSIdentifier 'a',JSDecimal '2')]))" + testExpr "(a, b) => {}" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSIdentifier 'a',JSIdentifier 'b')) => JSBlock []))" testExpr "(a, b) => a + b" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSIdentifier 'a',JSIdentifier 'b')) => JSExpressionBinary ('+',JSIdentifier 'a',JSIdentifier 'b')))" - testExpr "() => { 42 }" `shouldBe` "Right (JSAstExpression (JSArrowExpression (()) => JSStatementBlock [JSDecimal '42']))" + testExpr "() => { 42 }" `shouldBe` "Right (JSAstExpression (JSArrowExpression (()) => JSBlock [JSDecimal '42']))" testExpr "(a, ...b) => b" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSIdentifier 'a',JSSpreadExpression (JSIdentifier 'b'))) => JSIdentifier 'b'))" testExpr "(a,b=1) => a + b" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSIdentifier 'a',JSOpAssign ('=',JSIdentifier 'b',JSDecimal '1'))) => JSExpressionBinary ('+',JSIdentifier 'a',JSIdentifier 'b')))" testExpr "([a,b]) => a + b" `shouldBe` "Right (JSAstExpression (JSArrowExpression ((JSArrayLiteral [JSIdentifier 'a',JSComma,JSIdentifier 'b'])) => JSExpressionBinary ('+',JSIdentifier 'a',JSIdentifier 'b')))" - testExpr "{f:()=>{},y:2}" `shouldBe` "Right (JSAstExpression (JSObjectLiteral [JSPropertyNameandValue (JSIdentifier 'f') [JSArrowExpression (()) => JSStatementBlock []],JSPropertyNameandValue (JSIdentifier 'y') [JSDecimal '2']]))" + testExpr "{f:()=>{},y:2}" `shouldBe` "Right (JSAstExpression (JSObjectLiteral [JSPropertyNameandValue (JSIdentifier 'f') [JSArrowExpression (()) => JSBlock []],JSPropertyNameandValue (JSIdentifier 'y') [JSDecimal '2']]))" testExpr "{f:x=>x,y:2}" `shouldBe` "Right (JSAstExpression (JSObjectLiteral [JSPropertyNameandValue (JSIdentifier 'f') [JSArrowExpression (JSIdentifier 'x') => JSIdentifier 'x'],JSPropertyNameandValue (JSIdentifier 'y') [JSDecimal '2']]))" it "generator expression" $ do diff --git a/test/Test/Language/Javascript/StatementParser.hs b/test/Test/Language/Javascript/StatementParser.hs index 7e0f6929..bce3ff64 100644 --- a/test/Test/Language/Javascript/StatementParser.hs +++ b/test/Test/Language/Javascript/StatementParser.hs @@ -21,7 +21,7 @@ testStatementParser = describe "Parse statements:" $ do testStmt "{}" `shouldBe` "Right (JSAstStatement (JSStatementBlock []))" testStmt "{x=1}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSOpAssign ('=',JSIdentifier 'x',JSDecimal '1')]))" testStmt "{x=1;y=2}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSOpAssign ('=',JSIdentifier 'x',JSDecimal '1'),JSSemicolon,JSOpAssign ('=',JSIdentifier 'y',JSDecimal '2')]))" - testStmt "{()=>{};``}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSArrowExpression (()) => JSStatementBlock [],JSSemicolon,JSTemplateLiteral ((),'``',[])]))" + testStmt "{()=>{};``}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSArrowExpression (()) => JSBlock [],JSSemicolon,JSTemplateLiteral ((),'``',[])]))" testStmt "{x=>x;``}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSArrowExpression (JSIdentifier 'x') => JSIdentifier 'x',JSSemicolon,JSTemplateLiteral ((),'``',[])]))" testStmt "{{}}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSStatementBlock []]))" testStmt "{{{}}}" `shouldBe` "Right (JSAstStatement (JSStatementBlock [JSStatementBlock [JSStatementBlock []]]))" From f32038cba71bb2c04581b0c6aa0eb15087822e3f Mon Sep 17 00:00:00 2001 From: Gabe Johnson Date: Thu, 2 Jul 2020 21:21:02 -0500 Subject: [PATCH 3/3] Fix minifier tests The minifier was treating arrow functions with block bodies incorrectly if they contained a single expression statement with no return statement. It was transforming them into arrow functions with expression bodies. This means for instance that in ```js let x = (() => { 42 })(); ``` `x` would equal `undefined` before minification, but would equal `42` after minification. The fix is to change the tests to match the correct behavior. --- test/Test/Language/Javascript/Minify.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Test/Language/Javascript/Minify.hs b/test/Test/Language/Javascript/Minify.hs index 233ac5d3..7327e008 100644 --- a/test/Test/Language/Javascript/Minify.hs +++ b/test/Test/Language/Javascript/Minify.hs @@ -117,9 +117,9 @@ testMinifyExpr = describe "Minify expressions:" $ do minifyExpr "a => {}" `shouldBe` "a=>{}" minifyExpr "(a) => {}" `shouldBe` "(a)=>{}" - minifyExpr "( a ) => { a + 2 }" `shouldBe` "(a)=>a+2" + minifyExpr "( a ) => { a + 2 }" `shouldBe` "(a)=>{a+2}" minifyExpr "(a, b) => a + b" `shouldBe` "(a,b)=>a+b" - minifyExpr "() => { 42 }" `shouldBe` "()=>42" + minifyExpr "() => { 42 }" `shouldBe` "()=>{42}" minifyExpr "(a, ...b) => b" `shouldBe` "(a,...b)=>b" minifyExpr "(a = 1, b = 2) => a + b" `shouldBe` "(a=1,b=2)=>a+b" minifyExpr "( [ a , b ] ) => a + b" `shouldBe` "([a,b])=>a+b"