can you please move it one line up in the yaml group?

Are you sure about this? The "good way" would be to move up the Expression line but as a CS fix it shouldn't be done in master, right?

let's group yaml lines now that we can (but we don't move existing lines to reduce merge conflicts)

this means that the DI component now can't work with Yaml < 3.2, isn't it? So the composer.json file should get a conflict rule. Or: would it make sense to enable const parsing by default (and thus not require any change here?)

Fixed

@nicolas-grekas see https://github.com/symfony/symfony/pull/18626/files#diff-85ef95bd09276e850971f82e010b284dR32

The Symfony\Component\Yaml\Exception\RuntimeException might not be caught here, @xabbuh any ideas on how we should handle it?

@Haehnchen see https://github.com/symfony/symfony/pull/18626/files#r67163458

@nicolas-grekas

this means that the DI component not can't work with Yaml < 3.2, isn't it?

Isn't this line enough?

this line does apply only when testing the component, it has no effect when used as a dep of something else

Are we sure we need a flag here? Couldn't we unconditionally enable this?

@xabbuh what do you think?

Depends on whether we want to throw exceptions when a constant is not defined. Imo the flag makes sense if we then throw an exception when a constant does not exist (which would make debugging a lot easier than when we just returned null). Though if we enabled this feature unconditionally, we would not be able to throw exceptions when the constant was not defined.

Throwing the exception already depends on the exceptionOnInvalidType flag currently

Ok then, they are many options.

1. Parse constants by defaults:

   case 0 === strpos($scalar, '!php/const:'):
   if (defined($const = substr($scalar, 11))) {
       return constant($const);
   }
   if (self::$exceptionOnInvalidType) {
       throw new ParseException(sprintf('The PHP constant "%s" is not defined.', $const));
   }
   
   return;

2. Or using a flag as is:

   // use Symfony\Component\Yaml\Exception\RuntimeException;
   
   case 0 === strpos($scalar, '!php/const:'):
   if (self::$constantSupport) {
       $const = substr($scalar, 11);
       if (defined($const)) {
           return constant($const);
       }
   
       throw new RuntimeException(sprintf('The constant "%s" is not defined.', $const));
   }
   if (self::$exceptionOnInvalidType) {
       throw new ParseException(sprintf('The string "%s" could not be parsed as constant. Have you forgotten to use "Yaml::PARSE_CONSTANT" flag?', $scalar));
   }
   
   return;

3. Suggestions?

For now I vote for option 2, since php objects are not parsed by default.

My vote is for a flag too because this component should be as close to the Yaml spec as possible. !php/const: is a Symfony thing, not a Yaml thing.

I agree with that. It will consistent with how we already treat PHP object support.

Fine by me, then my comment above applies.

What about:

                    case 0 === strpos($scalar, '!php/const:'):
                        if (self::$constantSupport && defined($const = substr($scalar, 11))) {
                            return constant($const);
                        }
                        if (self::$exceptionOnInvalidType) {
                            throw new ParseException(sprintf('The "%s" YAML string could not be parsed because the %s.', $scalar, self::$constantSupport ? ' "Yaml::PARSE_CONSTANT" flag was not set' : 'corresponding PHP constant was not defined'));
                        }

                        return; 

@nicolas-grekas 👍

@nicolas-grekas Working on it I think we should throw an exception when the constant is not defined even if self::$exceptionOnInvalidType is false, what do you think @symfony/deciders?

In that case maybe I should only change the YAML RuntimeException to a ParseException so it's caught in the DI loader.

I think throwing an exception when a constant is not defined is a good idea.

This must throw an exception when constant support is disabled and $exceptionOnInvalidType is set (like we do for object support).

Alright!