Fix cryptic error messages caused by missing aesthetics

[dpseidel]

Fixes #2637 ; Fixes #2706

This is a minor change switching the order of calls in layer$compute_geom_1 to call check_required_aesthetics() before geom*$setup_data() in order to catch missing aesthetics and offer informative error messages.

This fixes cryptic error messages from GeomArea, GeomCol, GeomCrossbar, GeomErrorbar, GeomErrorbarh, GeomLine, GeomRaster, GeomSpoke, and GeomTile.

From my analysis this should have no adverse side effects.

The other Geoms' were appropriately handled for one of two reasons:

1. their setup_data() functions simply return data unchanged and therefore did not call any potentially missing aesthetics which caused the cryptic errors before check_required_aesthetics was originally called (e.g. GeomPoint, GeomText, GeomRibbon, ...).

Or,

2. the missing aesthetics are caught by stat_*$compute_layer() and the appropriate warning returned earlier (e.g. GeomBoxplot, GeomSmooth, GeomHex, GeomBar...).

All tests pass and I have checked the new behaviour of all Geoms, but I am open to considering other ways to test this or potential side effects I have not foreseen. I was unsure if the NEWS bullet was warranted but as it fixes two user facing issues I figured it was potentially useful.

[hadley]

Can you please turn one of the issues into a test? (i.e. test that the error is informative)

@clauswilke do you mind reviewing this? I suspect you are more familiar with this part of the ggplot2 code than me at the moment.

[dpseidel]

✔️ test added!

[clauswilke]

I had looked at this when #2637 was originally filed. My judgment then was that it's probably fine to switch the order but I don't understand the implications well enough to be 100% confident. I think it is reasonable to go ahead with the change and hope for the best. It might break some extension package somewhere out there, but it's probably not technically a breaking change, more of an API clarification.

As an extra check, I just looked at the code I have written in ggridges. It assumes that required aesthetics are checked before setup_data() is called. It's probably what most people would assume intuitively: You want to check your input, not what you do with your input.

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/