Skip to content

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

@sebastianbergmann

Description

@sebastianbergmann

#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.

Metadata

Metadata

Labels

feature/assertionIssues related to assertions and expectationstype/backward-compatibilitySomething will be/is intentionally brokentype/enhancementA new idea that should be implementedtype/refactoringA refactoring that should be applied to make the code easier to understand and maintain

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions