no need to make this nullable:

is it the point that it will be eventually initialized?

right now this is only initialized as $this->message ??= $this->translateMessage()

any access to this property prior to that place will throw an exception

Making this property nullable doesn't look good to me as you would now have to be aware of the inner state of the ConstraintViolationBuilder class to decide whether or not you can call addViolation().

@xabbuh , what do you propose instead?

to decide whether or not you can call

how often is it necessary to "decide" in the way you say it?

afaik, addViolation is only called in custom validators that create violation builder using execution context, and it itself provides ConstraintViolationList

So what is the point in "deciding"?

Take a look at this arbitrary example taken from the Symfony code base (from the ChoiceValidator):

$this->context->buildViolation($constraint->maxMessage)
    ->setParameter('{{ limit }}', $constraint->max)
    ->setPlural((int) $constraint->max)
    ->setCode(Choice::TOO_MANY_ERROR)
    ->addViolation();

You wouldn't know here if the ConstraintViolationBuilderInterface implementation returned by buildViolation() would throw or not.

Sorry, but what do you mean that we wouldn't know if it will throw or not?
It will never throw

That's a core design principle: not allowing clumsy behavior by making the design forbid them. Weakening means increasing the risk for others, while the benefit we'd get is to cover a niche use case that can be implemented another way.

Actually I do not like the idea of using constraint violation builder both for building and adding violations to the list. Though, in current circumstances when there's exactly one builder that is used both to build and to add violations, what's the best way? A separate class that entirely duplicates the current builder but addViolation() method?

I agree that building and adding violations are two different purposes, but isn't it what already happens in the class?

@nicolas-grekas , @xabbuh , what do you think about the fact that this is already so?

Maybe we would do things differently when we were to build the component again and split responsibility here. But given the impact that would have if we split the current implementation this is not going to happen.

let's keep the previous name

I was motivated by using the same naming as in ConstraintViolation, as these properties are named there as $message and $messageTemplate.

Naming them as $message and $translatedMessage here would confuse the picture, since $message means different things. Though, if you'd like them to be named this way, I will make a change.

Frankly speaking I myself thought about adding $translatedMessage, and letting the original $message alone, because the rename might introduce a BC break, as changing parameter name from message to messageTemplate would break the invocation code if it passes it as a named parameter.

Though, I've checked BC promise, and it says:

[10] Parameter names are only covered by the compatibility promise for constructors of Attribute classes. Using PHP named arguments might break your code when upgrading to newer Symfony versions.

[11] Only optional argument(s) of a constructor at last position may be added.

So, keeping it unified looks better to me, but in any case, feel free to ask what you will

having this method is probably confusing for users of this class given that we already have an atPath() method and by looking at the names it's not obvious how they differ.

In most programming languages, setter methods (which modify private variables) are conventionally named by starting with "set" followed by the variable name, with the first letter of the variable name capitalized. For example, if a variable is named firstName, the setter would be setFirstName()

see my concern about this change in the interface

changed the interface

btw, from my point question arises: if at any point we'd like to extend interface, how is it handled?
is this change released on a new major version as bc break? is there any deprecation of "not using new type"?

Adding this method adds a complete new meaning to the interface. I don't feel that it's a good idea having the same interface serve two different purposes.

this is a BC break
looking at the decoration test case, this might not be needed - instead, don't call setPlural if getPlural returns null

you're absolutely right, thank you for pointing this out
for some reason I didn't think about decoration 🫤

I've changed the interface

What about ConstarintViolationBuilder itself?