Skip to content
/ rector-src Public

[DeadCode] Skip isset() from property fetch from docblock on RemoveAlwaysTrueIfConditionRector#5754

Merged
samsonasik merged 7 commits intomainfrom
skip-isset-from-doc
Mar 21, 2024
Merged

[DeadCode] Skip isset() from property fetch from docblock on RemoveAlwaysTrueIfConditionRector#5754
samsonasik merged 7 commits intomainfrom
skip-isset-from-doc

Conversation

@samsonasik
Copy link
Copy Markdown
Member

@samsonasik samsonasik commented Mar 21, 2024 •
edited
Loading

The following code should be skipped as rely to docblock

class SkipPropertyFetchDoc
{
    /**
     * @var string
     */
    public $bodyFormat = '';

    public function run()
    {
        if (isset($this->bodyFormat) && $this->bodyFormat !== '') {
        }
    }

    public function reset()
    {
        $this->bodyFormat = null;
    }
}

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 21, 2024

@TomasVotruba using Isset_ instance and ArrayDimFetch check seems the solution.

@samsonasik samsonasik force-pushed the skip-isset-from-doc branch from e0f6c8a to 20393d7 Compare March 21, 2024 17:04
@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 21, 2024

The left side can be also empty() check, so it seems need specific Variable node only check.

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 21, 2024

I tried the BooleanAnd new feature, and it seems cause many changes, that incorrect.

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 21, 2024

I think BooleanAnd should only check 2 things on the left:

  • instanceof from typed param
  • is_* from typed param

like #5748, so correct native type will always guaranted.

I will look into it, also apply on ReduceAlwaysFalseIfOrRector

@samsonasik samsonasik self-assigned this Mar 21, 2024
@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 21, 2024

I added SafeLeftTypeBooleanAndOrAnalyzer so it can be improved over time in single location.

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 21, 2024

All checks have passed 🎉 @TomasVotruba I am merging it to have faster feedback to test ;)

@samsonasik samsonasik merged commit b4eb883 into main Mar 21, 2024
@samsonasik samsonasik deleted the skip-isset-from-doc branch March 21, 2024 18:35
@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Mar 21, 2024

Thanks 👍

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

Reviewers

No reviews

Assignees

@samsonasik samsonasik

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@samsonasik @TomasVotruba