#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 (via UnaryOperator) calls these dedicated methods directly for its own case, instead of routing through the generic toStringInContext() / failureDescriptionInContext() hooks.
- First-party constraints are migrated to override
negatedToString() / negatedFailureDescription() instead of the guarded toStringInContext() / failureDescriptionInContext() overrides. The bodies are nearly identical, minus the instanceof guard and the unused $role parameter — migration is mechanical.
toStringInContext() / failureDescriptionInContext() themselves stay as they still serve BinaryOperator (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 what negate() currently produces for the simpler cases.
- Keep
negate() as a private helper of LogicalNot: 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.
#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, andLogicalNot::negate()is retained only as the documented fallback for third-party constraints.That work was deliberately backward-compatible: it leaves
LogicalNot::negate(), apublicmethod, 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:The
instanceof LogicalNotguard 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$roleparameter, hiding the actual intent ("this returns the negated description") behind a generic name and aninstanceofcheck.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:LogicalNot(viaUnaryOperator) calls these dedicated methods directly for its own case, instead of routing through the generictoStringInContext()/failureDescriptionInContext()hooks.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?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.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.