add dict interface splitargdef and combineargdef

[willow-ahrens]

This PR adds new methods splitargdef and combineargdef (in addition to existing splitarg and combinearg) that use dictionaries as an interface. The dict interface more completely expresses corner cases, as described in https://github.com/FluxML/MacroTools.jl/pull/178. In particular, it provides robust solutions for literal nothing argument defaults and avoids typeasserts when no type is given for an argument.

[codecov-commenter]

Codecov Report

Merging #179 (341668a) into master (5e0fe08) will increase coverage by 1.13%. The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master     #179      +/-   ##
==========================================
+ Coverage   74.68%   75.82%   +1.13%     
==========================================
  Files           9        9              
  Lines         403      426      +23     
==========================================
+ Hits          301      323      +22     
- Misses        102      103       +1     

Impacted Files	Coverage Δ
src/utils.jl	70.15% <95.65%> (+3.49%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5e0fe08...341668a. Read the comment docs.

[cstjean]

Hi Peter, thank you for the PR. I agree that the current situation is not ideal, but between the three options:

1. Support two interfaces to splitarg
2. Make a breaking release to replace splitarg with its Dict equivalent
3. Leave splitarg as it is

I'm going to favour option 3, at least until there's another, more compelling reason to make a MacroTools 0.6 release. Having two interfaces for the same thing is more maintenance burden, and confusing for new users of the package. Furthermore, splitarg is only broken in a very marginal use case. It's unlikely that anyone will bump into it, and the work-around (use quote) sounds acceptable to me. It's a judgement call, obviously...

When 0.6 will be on the horizon, I would consider merging this PR with a deprecation from splitarg to splitargdef. Another name I find appealing is splitarg(Dict, ...) instead of splitargdef(...), but that's a minor detail I guess.

[willow-ahrens]

Sure thing! I think this would be great to include with the next version, and I wanted to file the PR before I forgot the details.

I chose splitargdef because it's clearer to deprecate a function for a differently named function (rather than a method for a method), and to match splitdef and splitstructdef, both of which simply use Dict.

[willow-ahrens]

I will clarify that there is no current work-around for literal nothing input to splitarg, and no workaround to differentiate an Any typeassert from no typeassert based on the output of splitarg. These are still rare cases.

[cstjean]

I will clarify that there is no current work-around for literal nothing input to splitarg

There's a user-side work-around, of replacing f(arg=$x) with f(arg=$(QuoteNode(x))). Given that we're erroring in splitarg, it seems quite reasonable to me...

no workaround to differentiate an Any typeassert from no typeassert based on the output of splitarg

True.