sebastianbergmann / sebastianbergmann/phpunit

Replace `LogicalNot::negate()` with dedicated negation methods on `Constraint`

Open
#6,686 1 comment 0 reactions 1 assignee View on GitHub

@sebastianbergmann is already working on this.

Since May 28, 2026.

feature/assertion type/backward-compatibility type/enhancement type/refactoring
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 (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?

  1. 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.
  2. 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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.