[12.x] Use property promotion in MessageLogged and narrow $level

[cosmastech]

$level here is a closed set of strings, so they can be typehinted specifically. Constructor property promotion simply because I didn't want to duplicate that closed set in the constructor and on the parameter itself 🤠

[browner12]

man, this just feels weird to me. does it give us some benefit over a plain string? feels like we're locking ourselves in unnecessarily.

[cosmastech]

man, this just feels weird to me. does it give us some benefit over a plain string? feels like we're locking ourselves in unnecessarily.

@browner12 yes, it does provide a benefit. Take for instance this function:

    protected function writeLogsToConsole(): void
    {
        Log::listen(function (MessageLogged $messageLogged) {
            $method = match ($messageLogged->level) {
                'emergency', 'alert', 'critical', 'error' => 'error',
                'warning' => 'warn',
                // 'debug', 'info', 'notice' => 'info',
            };
            $this->{$method}($messageLogged->message);
        });
    }

PHPStan can know that all paths are not covered and report a warning for me.

  15     Match expression does not handle remaining values: 'debug'|'info'|'notice'                                                                                          
         🪪  match.unhandled        

I don't feel this is locking us into anything. To me, being locked into something would be like the constructor guarding against invariants by throwing an exception when the $level param doesn't match one of those values.

In this case, we get benefits when using static analysis (for those who choose to use it), but causes no real harm to people who don't have static analysis as part of their development flow.

I also don't think there's anywhere else that's emitting this event besides inside of the Logger 🤷

[browner12]

yah, I would agree no "harm", but there is the maintenance burden to consider. granted it is very small, probably almost non-existent for this case, because log levels are pretty set nowadays.

[cosmastech]

yah, I would agree no "harm", but there is the maintenance burden to consider. granted it is very small, probably almost non-existent for this case, because log levels are pretty set nowadays.

Yes, I agree that it's probably non-existent maintenance. The functions for the LoggerInterface were defined over 10 years ago, I believe.