isRequired() isn't overridden anywhere, so it's always been false.

The reason this is weird is that if you submit the form but it has errors, we remove the "required" markers from fields with no errors to emphasize fields with actual problems which need to be addressed. If we don't do this, you get an error and a bunch of "required" text, which could be misleading if you don't read anything (a common affliction among users of software).

That is, required fields actually have three states:

Name: [Thing____] Required (grey)
Name: [Thing____] Required (red)
Name: [Thing____]

State (1) is shown when you load the form for the first time.
State (2) is shown if you submit the form and your input is invalid.
State (3) is shown if you submit the form and your input is good, but some other input on the form is bad. We show state (3) instead of state (1) to avoid potential confusion with marking the field as "Required".

This change isn't inconsistent with that, but I suspect it will make building ad-hoc forms which follow these rules more complicated in practice, even though the API is much more clear. I think you'll have to do something like this:

$name_required = true;
$name_error = null;

if ($request->isFormPost()) {
  $name_required = false;
  $name_error = ...;

  // ...
}

$control
  ->setError($name_error)
  ->setRequired($name_required);

Basically, two state variables instead of one and a somewhat-confusing $name_required = false; (meaning "don't show the grey-text required hint") when the field is actually required.

Do you have another way to write forms which uses this API and isn't more complicated than the current weird/magic/overloaded API?

I do think the current API is pretty bad, but it's not completely just bad for no reason.

We have many fewer ad-hoc forms now than we did in the past, so I think there's also a reasonable argument for "make the API sensible, users can deal with typing slightly more stuff since ad-hoc forms are often the wrong approach anyway".

I think there's a reasonable argument for "tell AphrontForm about request state, let it deal with hiding 'required'". EditEngine deals with this problem in this way (setIsSubmittedForm(), etc), but it would be somewhat easy to forget to provide it. A reasonable fix might be to provide newForm() on PhabricatorController, similar to newDialog(), that populates viewer + request state.

I didn't intentionally think about it, but I got this:

• Add a showHints property, to both form and control

Then the API would be something like:

$form = id(new Form())
  ->setShowHints(empty($errors))
  ->appendControl(id(new Control()))
    ->setRequired(true))
    ...

The form's showHints will trickle down to the controls, and the gray "Required" will only be shown if it's enabled.

still very low priority...

Yeah, something like that is what I meant by "tell AphrontForm about request state, let it deal with hiding 'required'".

The EditEngine version of that is roughly PhabricatorEditField->setIsSubmittedForm(), which has approximately the same effect:

if ($this->getIsSubmittedForm()) {
  $error = $this->getControlError();
  if ($error !== null) {
    $control->setError($error);
  }
} else if ($this->getIsRequired()) {
  $control->setError(true);
}

I think it's reasonable to extend that logic to AphrontForm.