search

JSMonk / hegel

An advanced static type checker
https://hegel.js.org
MIT License
2.1k stars 59 forks source link

Lots of examples broken #169

Open srg-kostyrko opened 4 years ago

srg-kostyrko commented 4 years ago

after https://github.com/JSMonk/hegel/pull/117 was merged lots of examples in docs got broken - they report errors when docs say there should be none like in

https://hegel.js.org/try#PTAEDEFcDsGMBcCWB7aoAiBTWAbAhgE55KoBQAZjAimgM6QC2AjABR4Bco0jARpgQBpQPTtwZ8CASlG9+oAN6lQoApniQCaPKADUwgNykAvqVIgIVEmgCiADwAOq2rRqlYqWvFD0GAJlAAvKCUcFagbDLi-EIiXLJSkRIKSipqGlq6Bsam5gCCBATIAO4WoTSgdo6Yzq7u0J7ejADMgeEccVGCwon80h1JAQB8oNp6PIakPqxMQr6SoOaAvBuAMjsAhJOMviwzoHMLYCvrPk3bs-NLazlg1gXIBJwAKgCe9pigAETkeDi0mO+giFoALgyAY9mIiB4ODeRUQ8AAFqB4C83u8xBJ3htmCwvj9MEJ3u9JBNzDdCvdQM9Xh8AOQ0-6A4HuMEQqEwuGI5HUtHxTE+LaEoQAFmJVwqtwpVNRNKY9IBQMQIJZSDZoFhCKRKI+6P4fOaLHeTHeBN8RMMQA

one more point about this check is that promise chaining will report an error if we make some side effects in then and does not care about the returned promise

https://hegel.js.org/try#IYZwngdgxgBAZgV2gFwJYHsIxACgJQwDeAvgFCm54B0yAFgKYQ74wC8AfEaTDAPS8wAJumzoAtvWypBk+nDj0oyEKWJ4gA

tough this is not a very good approach I would say - so not sure if something needs to be changed here

JSMonk commented 4 years ago

Ahh. Can you try to fix the examples? I will fix the behavior with async functions that return Promise<undefined>.

srg-kostyrko commented 4 years ago

I started working on examples will add here other findings

srg-kostyrko commented 4 years ago

Array.push returns the new length of an array that rarely is being used but is reported as an error now

http://localhost:3000/try/#MYewdgzgLgBAJiArgIwDYFMIC4YEEBO+AhgJ4A8YiAtsuvgHwwC8MA2gLJFQAWAdAAoBJADQxOPXgFEAugG4AUPIQoMEXgAdEEbgAoALACYAlAqA

JSMonk commented 4 years ago

Oh, it's a problem, because everybody uses push without using his return value

srg-kostyrko commented 4 years ago

yeh, and the same is actually true for methods that mutate array in-place like reverse or splice - they have return value but usually used without using it

srg-kostyrko commented 4 years ago

other problem - Date it has lots of set methods and all of them return number I think almost nobody uses it

https://hegel.js.org/try#MYewdgzgLgBAJgQygUxgXhmZB3GARJZACgEoAoRFAOgmSgAkQBXAJwiIEYAmEoA

thecotne commented 4 years ago

i think sooner or later we need to introduce strictness rules and/or warnings

so if someone does not mind prepending void to bunch of statements and they can opt-in (or opt-out whatever) i think opt-out option is better so that it's strict by default and you turn off some checks if it gets annoying ...

also it's good for education purposes to have checker complain about unused values and to discover that some functions actually have return values (and you will be like "So interesting this tool is pretty cool!")

srg-kostyrko commented 4 years ago

I created PR that fixes most of places there are couple point where array.push is used - I have not changed them for now

other point is about type inference https://hegel.js.org/try#GYVwdgxgLglg9mABAZwKZRABwKICZsAUAJgEYCUiA3gFCKKkB0ATqhAmK1AWQNzUC+1IA not sure though that we should change it in this case

JSMonk commented 4 years ago

I've merged. Thank you a lot ^_^.