Skip to content

Strange string concatenation issue after upgrading to PHP 8.3 #13229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
janforth opened this issue Jan 23, 2024 · 9 comments
Closed

Strange string concatenation issue after upgrading to PHP 8.3 #13229

janforth opened this issue Jan 23, 2024 · 9 comments
Labels

Comments

@janforth
Copy link

Description

In PHP 8.2 this line worked perfectly well:

$togetherWithName = "gemeinsam mit " . (($receiver[0] === ($article?->getCoAuthor() ?? "none") || $receiver[0] === ($article?->getCoAuthor()?->getEmail() ?? "none" ) || $coautor) || $emailContext->isForCoAuthorOnly()) ? ($article?->getAuthor()?->getFullName() ?? "N.N.") : ($article?->getCoAuthor()?->getFullName() ?? "N.N.");

After upgrading to PHP 8.3 the string concatenation did not work anymore. The first part of string "gemeinsam mit " (engl.: 'together with') was always left out for whatever reason. The string $togetherWithName always only contained the name of the respective author from the right party of the string concatenation.

This could fix the issue:

            $togetherWithName = (($receiver[0] === ($article?->getCoAuthor() ?? "none") || $receiver[0] === ($article?->getCoAuthor()?->getEmail() ?? "none" ) || $coautor) || $emailContext->isForCoAuthorOnly()) ? ($article?->getAuthor()?->getFullName() ?? "N.N.") : ($article?->getCoAuthor()?->getFullName() ?? "N.N.");
            $togetherWithName = "gemeinsam mit " . $togetherWithName;

But I think the one-liner should work.

Thanks!

PHP Version

PHP 8.3

Operating System

Debian 12

@heiglandreas
Copy link
Contributor

heiglandreas commented Jan 23, 2024

$togetherWithName = "gemeinsam mit " . (($receiver[0] === ($article?->getCoAuthor() ?? "none") || $receiver[0] === ($article?->getCoAuthor()?->getEmail() ?? "none" ) || $coautor) || $emailContext->isForCoAuthorOnly()) ? ($article?->getAuthor()?->getFullName() ?? "N.N.") : ($article?->getCoAuthor()?->getFullName() ?? "N.N.");

The trouble is probably that the whole string gets separatet into the following construct:

if ("gemeinsam mit " . (($receiver[0] === ($article?->getCoAuthor() ?? "none") || $receiver[0] === ($article?->getCoAuthor()?->getEmail() ?? "none" ) || $coautor) || $emailContext->isForCoAuthorOnly())) {
    $togetherWithName = ($article?->getAuthor()?->getFullName() ?? "N.N.");
} else {
    $togetherWithName = ($article?->getCoAuthor()?->getFullName() ?? "N.N.");
}

Having everything in one line might be amazing. But definitely hinders readability. And by that also figuring our what actually happens where.

@janforth
Copy link
Author

Point taken! ;)

@SVGAnimate
Copy link
Contributor

@janforth are you using the mbstring extension?

@heiglandreas
Copy link
Contributor

heiglandreas commented Jan 23, 2024

An option to still use this in one line would be to add some more braces:

 $togetherWithName = "gemeinsam mit " . ((($receiver[0] === ($article?->getCoAuthor() ?? "none") || $receiver[0] === ($article?->getCoAuthor()?->getEmail() ?? "none" ) || $coautor) || $emailContext->isForCoAuthorOnly()) ? ($article?->getAuthor()?->getFullName() ?? "N.N.") : ($article?->getCoAuthor()?->getFullName() ?? "N.N."));

Note the added opening brace after the first dot and the added closing brace before the semicolon. That makes sure that the string concatenation happens after the big chunk has been treated as one trinary operator.

Otherwise the string concatenation is seen as part of the test of the trinary operation.

And according to https://3v4l.org/Sg1rI I'd question that it actually worked in PHP8.2 the way you expected it to work... 🙈

@janforth
Copy link
Author

@janforth are you using the mbstring extension?
@SVGAnimate I do.

@janforth
Copy link
Author

@heiglandreas Thanks for shedding some light on this. It clears things up. Obviously I misjudged the binding effect of the dot in the order of operations. But I can tell you: It definitely worked in PHP 8.2.

@heiglandreas
Copy link
Contributor

@janforth: Can you create a case on 3v4l.org (or a similar tool) that reproduces what you see in PHP8.2 as well as in PHP8.3? That would be really helpful for this case to understand where the problem arises.

@SVGAnimate
Copy link
Contributor

SVGAnimate commented Jan 24, 2024

image

Without using mbstring it seems to work on PHP-8.3.1.

can you show how you use the mb_* functions
class Context {
    function isForCoAuthorOnly() {
        return false;
    }
}
class Author {
    protected $name;
    protected $email;
    function __construct($name, $email) {
        $this->name = $name;
        $this->email = $email;
    }
    function getEmail() {
        return $this->email;
    }
    function getFullName() {
        return $this->name;
    }
}

class Article{
    protected $coAuthor;
    protected $author;
    function __construct($author, $coAuthor) {
        $this->coAuthor = $author;
        $this->author = $coAuthor;
    }
    function getCoAuthor() {
        return $this->coAuthor;
    }
    function getAuthor() {
        return $this->author;
    }
}

$article = new Article(new Author("John", "[email protected]"), new Author("Doe", "[email protected]"));
$emailContext = new Context();

$receiver=["foo"];
$coautor = "John Doe";

// 
$togetherWithName = "gemeinsam mit " . (($receiver[0] === ($article?->getCoAuthor() ?? "none") || $receiver[0] === ($article?->getCoAuthor()?->getEmail() ?? "none" ) || $coautor) || $emailContext->isForCoAuthorOnly()) ? ($article?->getAuthor()?->getFullName() ?? "N.N.") : ($article?->getCoAuthor()?->getFullName() ?? "N.N.");

var_dump($togetherWithName);

@janforth Can you provide a simplified version of your code and the problematic datasets?
And also, can you indicate the encoding of your php file

Copy link

github-actions bot commented Feb 8, 2024

No feedback was provided. The issue is being suspended because we assume that you are no longer experiencing the problem. If this is not the case and you are able to provide the information that was requested earlier, please do so. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants