issue #1729 - Binding query/path params and form fields to struct only works for explicit tags

[aldas]

Binding query/path params and form fields to struct only works for fields that have explicit TAG defined on struct

This is for issue #1729 also read #1681 (comment) last part of binding serie (without docs pr)

Dunno if this should go to 4.x version.

[codecov]

Codecov Report

Merging #1734 (254d08c) into master (b065180) will increase coverage by 0.87%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1734      +/-   ##
==========================================
+ Coverage   85.87%   86.75%   +0.87%     
==========================================
  Files          29       29              
  Lines        2018     2068      +50     
==========================================
+ Hits         1733     1794      +61     
+ Misses        180      175       -5     
+ Partials      105       99       -6     

Impacted Files	Coverage Δ
bind.go	91.25% <100.00%> (-0.06%)	⬇️
middleware/static.go	67.16% <0.00%> (-1.50%)	⬇️
router.go	97.05% <0.00%> (ø)
middleware/csrf.go	80.28% <0.00%> (+4.16%)	⬆️
echo.go	91.53% <0.00%> (+5.13%)	⬆️

---

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 b065180...254d08c. Read the comment docs.

[lammel]

As the semantics of binding already changed with v4 and we are about to restore what the users expect and will required documentation anyway we could also merge this. IMO it makes the usage very explicit and avoids unexpected pitfalls, but it diverges from the pre-4.11 behaviour.

Some feedback would be greatly appreciated.

[aldas]

This will certainly hit some people that have 'forgotten' to add tags and are using 'undocumented feature'.

https://echo.labstack.com/guide/request only has examples with tags so we are changing undocumented 'feature' here - probably ok, no guarantees for undocumented features. This would avoid some errors also - binding to non tagged fields and failing with conversions

This info could be added as a note to next 4.x release. github release page knows how to show markdown

[lammel]

I'm still in favor of merging this and adjusting the documentation for the binder. It will prevent the autobinder from accidentally overwriting values from unexpected sources (like query params adding to the json body). It is still possible, just needs tagging in the structs used.

Maybe @pafuent or @vishr have some more comments on that.
Otherwise I'd be willing to review and adjust some wordings in your PR labstack/echox#172 and maybe improve the docs a little more.

The release notes for v4.2 will definitely include a section for the binder!

[pafuent]

LGTM!
Thanks @aldas

Sorry to notice this later, but the benchmarks are broken after this change. BenchmarkBindbindData is not longer valid.

[lammel]

Thanks @aldas, great work.

Docs will be updated soon to be ready for the next release.