Replace `LogicalNot::negate()` with dedicated negation methods on `Constraint`
#6685 restructured how inverse assertions produce failure-message text. Each first-party constraint now authors its own negated description through the existing toStringInContext() / failureDescriptionInContext() hooks, and LogicalNot::negate() is retained only as the documented fallback for third-party constraints.
That work was deliberately backward-compatible: it leaves LogicalNot::negate(), a public method, untouched, and reuses the existing hooks rather than introducing new API. This issue tracks possible clean-up that requires a major version.
The toStringInContext() / failureDescriptionInContext() hooks are generic over operators. Every first-party negation override therefore looks like this:
protected function toStringInContext(Operator $operator, mixed $role): string
{
if (!$operator instanceof LogicalNot) {
return '';
}
return 'is not equal to ' . $this->valueAsString();
}The instanceof LogicalNot guard is mandatory. Without it, BinaryOperator (LogicalAnd / LogicalOr / LogicalXor) would wrongly negate the operands of conjunctions and disjunctions. But it is also the same boilerplate repeated in dozens of constraints, with an unused $role parameter, hiding the actual intent ("this returns the negated description") behind a generic name and an instanceof check.
Meanwhile LogicalNot::negate() is still in the codebase, still part of the public API, and still applies the same value-mangling string surgery to any constraint that does not override the hooks.
We could introduce dedicated negation methods on Constraint:
abstract class Constraint
{
/**
* Returns the description of this constraint when it is wrapped in a
* LogicalNot operator. Override to author the negation directly.
*/
protected function negatedToString(): string
{
// see "Default behaviour" below
}
/**
* Counterpart of failureDescription() for the LogicalNot case.
*/
protected function negatedFailureDescription(mixed $other): string
{
return Exporter::export($other) . ' ' . $this->negatedToString();
}
}LogicalNot(viaUnaryOperator) calls these dedicated methods directly for its own case, instead of routing through the generictoStringInContext()/failureDescriptionInContext()hooks.- First-party constraints are migrated to override
negatedToString()/negatedFailureDescription()instead of the guardedtoStringInContext()/failureDescriptionInContext()overrides. The bodies are nearly identical, minus theinstanceofguard and the unused$roleparameter — migration is mechanical. toStringInContext()/failureDescriptionInContext()themselves stay as they still serveBinaryOperator(LogicalAnd/LogicalOr/LogicalXor).LogicalNot::negate()is removed.
But what should negatedToString() return by default for a third-party constraint that has not been updated to override it?
- A mechanical fallback like
return 'not (' . $this->toString() . ')';would always be correct, never mangle a value, but the wording is rougher than whatnegate()currently produces for the simpler cases. - Keep
negate()as a private helper ofLogicalNot: drop it from the public API, but reuse the implementation internally as the default. Preserves current third-party output verbatim, at the cost of carrying the string-rewriting code internally and continuing to expose third-party constraints to its known fragility.
(1) is simpler and gives third-party authors a clear, robust path forward; (2) is more conservative. My current preference is (1).
Removing LogicalNot::negate() is a change that breaks backward compatibility and needs to wait for a new major version.
Source: sebastianbergmann/phpunit