Skip to content
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

"Use type casting instead" inspection should probably be disabled by default #1464

Closed
Ocramius opened this issue Dec 12, 2019 · 2 comments
Closed
Assignees
Milestone

Comments

@Ocramius
Copy link

Ocramius commented Dec 12, 2019

Consider following code snippet:

class Something
{
    public function __toString() : string {
        return 'ohai!';
    }
}

function gimmeSomething() : Something
{
    // ... 
}

echo gimmeSomething()->__toString();

This reports "(string) gimmeSomething() would express the intent better".

I've put a lot of thought in this last year, and I think an explicit ->__toString() call is preferable. Specifically, if gimmeSomething() changes to nullable:

function gimmeSomething() : ?Something
{
    // ... 
}

Then the expression (string) echo gimmeSomething() keeps working (bug), while echo gimmeSomething()->__toString(); crashes (null pointer exception, expected).

I've documented this in more detail at ShittySoft/symfony-live-berlin-2018-doctrine-tutorial#3 (comment), but using a (string) cast is most probably not to be endorsed.

I'm not sure if this is related to the discussion above (or if it is a core IDE bug), but the following doesn't seem to report a NPE either:

<?php

class Something
{
    public function __toString() : string {
        return 'ohai!';
    }
}

function gimmeSomething() : ?Something
{
    return new Something();
}

// NPE here - not being reported
echo gimmeSomething()->__toString();
@kalessil kalessil self-assigned this Jan 5, 2020
@kalessil kalessil added this to the C-4.0.2 milestone Jan 5, 2020
@kalessil
Copy link
Owner

kalessil commented Jan 5, 2020

Done: made this configurable, opted-out by default.

@kalessil kalessil closed this as completed Jan 5, 2020
kalessil added a commit that referenced this issue Jan 5, 2020
@Ocramius
Copy link
Author

Ocramius commented Jan 5, 2020

Thank you 💃

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

No branches or pull requests

2 participants