Codecov Report

Merging#538 intomaster willincrease coverage by0.03%.
The diff coverage is100%.

@@            Coverage Diff             @@##           master     #538      +/-   ##==========================================+ Coverage   94.23%   94.27%   +0.03%==========================================  Files         104      104                Lines        4234     4246      +12       Branches     1152     1160       +8     ==========================================+ Hits         3990     4003      +13  Misses        142      142+ Partials      102      101       -1

Good point. Do you have some examples of what else should pass/fail with this new option?

The only case you've handled is an arrow function which returns an arrow function and has no body

constx=()=>()=>1

Should probably support all of these cases, as no doubt people will ask for them down the line.

functionFunctionDeclaration(){returnfunctionFunctionExpression_Within_FunctionDeclaration(){returnfunctionFunctionExpression_Within_FunctionExpression(){return()=>{// ArrowFunctionExpression_Within_FunctionExpressionreturn()=>// ArrowFunctionExpression_Within_ArrowFunctionExpression()=>1// ArrowFunctionExpression_Within_ArrowFunctionExpression_WithNoBody}}}}functionFunctionDeclaration(){return()=>{// ArrowFunctionExpression_Within_FunctionDeclaration// etc}}

I.e. every single combination ofFunctionExpression /FunctionDeclaration /ArrowFunctionExpression

@JamesHenry Thanks to the examples provided by@bradzacher and some time looking at them throughhttps://astexplorer.net, I think I figured out how to implement it for all(?) cases now (i.e. not just forArrowFunctionExpression). The tests seem to run fine at least. Branch updated. 🙂👍

@bradzacher I see you already added an approval, but just so it's said: Docs have been updated with function example, and I added two tests to cover a missing branch in my changes.

I also looked into adding a test to coverthe very last uncovered branch in the file, but I'm pretty sure it's actually impossible to hit that branch?

We only get to theif whennode is eitherFunctionExpression orArrowFunctionExpression, which seems to always have a parent ofExpressionStatement? And even if that were to not be the case, there should still be a parent ofProgram?

So, to get the coverage up to 100% (and for others not to waste time trying to cover that branch too) I added anistanbul ignore else there.

@JamesHenry Could you explain what the difference is? From what I can see, currying looks like what this option allows. I'm reading about higher order functions, but they seem a lot more broader, like taking functions as arguments as well, which has nothing to do with this. There seems to be another concept called partial application, which to me looks like a slightly broader currying, but yeah... not really knowledgeable in this area...

I don't mind changing the name though. What do you think@bradzacher?

IIRC technically currying is more the concept of functions which automatically return functions if you don't pass all of the required arguments, instead of explicitly defined functions returning functions.

In that regard, calling these higher order functions is technically more correct.

I think that either will work - people will probably understand either way.
But in case they don't, probs best to change it to something likeallowHigherOrderFunctions.

@JamesHenry@bradzacher Alrighty, the option has now been renamed to technically more correctallowHigherOrderFunctions, and documentation and code adjusted accordingly. 🙂

@JamesHenry Is it good now? Anything else preventing a ✔ from you as well? Would really like to get this PR merged in, closed and released so I can make use of this new option in our project. 🙂

I know I'm being the naming police on this, but this still isn't really currying thatallowCurrying is allowing.

The implementation is more broad than that, it is allowing any higher order function pattern.

I think that is probably what is most valuable anyway, so it's just the name that needs to change, so that users aren't confused and think it checks explicitly for currying patterns.

I'm not married to it, butallowHigherOrderFunctions ?

@JamesHenry Your requested change was fixed over a week ago now. Is there still more you'd like to be changed, or is it good now?