sebastianbergmann / sebastianbergmann/phpunit
Replace `LogicalNot::negate()` with dedicated negation methods on `Constraint`
@sebastianbergmann is already working on this.
Since May 28, 2026.
- Dominant language
- PHP
- Stars
- 20.1k
- Forks
- 2.2k
- Avg merge
- 7h
- Merged PRs (30d)
- 61
Description
#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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.