faylang / fay

A proper subset of Haskell that compiles to JavaScript
https://github.com/faylang/fay/wiki
BSD 3-Clause "New" or "Revised" License
1.29k stars 86 forks source link

Add compiler warning/error for easy newbie mistake: duplicate quotes around string value argument to FFI #388

Closed bitemyapp closed 10 years ago

bitemyapp commented 10 years ago
setBGColour = ffi "%2['css']('backgroundColor', %1)"

makeSquare :: JQuery -> Fay JQuery
makeSquare = addClass "square" >=>
             setWidth 400 >=>
             setHeight 400 >=>
             setBGColour "red"

I had '%1' before and it wasn't working...quite mysteriously, even when I used strict it wasn't clear what was going on :(

bergmark commented 10 years ago

The FFI expects a JS expression, and '%1' is valid. If we disallowed this you would no longer be able to pass the string literal '%1'.

What do you mean about using strict? --strict? It does something different, from --help: "Generate strict and uncurried exports for the supplied modules. Simplifies calling Fay from JS"

bitemyapp commented 10 years ago

@bergmark is there some way to make the FFI templating safer or at least have some linty warnings? Is this something that should exist outside of Fay? (rather not)

'%1' and ::String should have a lint warning, IMO.

Edit: --lint or -Wall or something? This seriously stymied me for over an hour and I suspect others might get caught by something similar.

Thanks for responding to both issues so quickly btw!

bergmark commented 10 years ago

I forgot to reply to this, sorry!

I would love some linting mode to spot common errors, we already have --closure to help avoid common problems when using google closure. But like i said, this isn't obviously an error since '%1' is a valid js expression. I don't think there's any way to tell whether it's more common to make this error or to pass strings with percentages on purpose. I don't think we should give warnings about things that may be correct. For example, I find it annoying that fay can't be hlint warning free because it gives suggestions that are incorrect or are just stylistic.