make a decision on field_attach_validate()

    // @todo: Discuss: Should we call field_attach_validate()? None of the
    // entities in core does this (fields entered through forms are already
    // validated).

This comment was probably written by one of the authors of the original patch that made Flaggings into fieldable entities.

I've since encountered field_attach_validate() myself in other contrib projects, and I know this about it:

As the comment says, core entities' API functions such as node_save() don't call it. This is because the entity forms take care of calling field_attach_form_validate(), which does the same job, but on form values (and which takes care of setting form errors). There's also the fact that if you are coming from the form, there's no point in validating again during the API call.

The philosophy in core therefore appears to be that if you are using the API to save entities, it's your responsibility to call field_attach_validate() yourself, and deal with errors in a way that makes sense to you. The docs at https://api.drupal.org/api/drupal/modules!field!field.attach.inc/functio... confirm this, though modules such as Services Entity (#2213503: call field_attach_validate() prior to saving entities) and Migrate (#2225551: entity saves in migration destinations should call field_attach_validate()) and probably others have not cottoned on to this.

So for Flag my thinking is this: flag() is an API function. Flag module calls it in several places where there aren't any fields involved anyway (eg the JS link). Where it does call it with a form, it doesn't call field_attach_form_validate(). That's a bug (which I've only just spotted!!!!). Fixing that will put us in the same position as the core entities: our forms validate Field API with field_attach_form_validate(), and so get errors during form validation. Callers of our API are expected to use either field_attach_form_validate() in their forms, or if they're not using a form, call field_attach_validate() themselves.

I think core is somewhat remiss in not pointing this out in functions like node_save(), so this issue should add that documentation as well as remove the comment quoted above.