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

DocBlock/Tags/Source: remove redundant code #288

Merged

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Aug 1, 2021

PHPStan flags the code within the Source::__toString() method:

 ------ -------------------------------------------------------------------------------------------
  Line   DocBlock\Tags\Source.php
 ------ -------------------------------------------------------------------------------------------
  111    Result of || is always true.
  114    Result of || is always true.
  114    Result of || is always true.
 ------ -------------------------------------------------------------------------------------------

I have investigated this and can confirm that these flags are correct.

  1. $this->startingLine is cast to an integer in the __construct() method (line 45) and subsequently cast to a string in __toString() (line 105).
    This means that it can only ever be a non-empty ("truthy") string or the string '0', so the $startingLine || $startingLine === '0' condition used in two places is redundant.
  2. $this->lineCount is either an integer or null after the __construct() method (line 46).
    In the __toString() method, if the lineCount is an integer, it is effectively cast to a string by the concatenation with an empty string on line 107, while if the lineCount was null, it is turned into an empty string.
    By changing the concatenation from concatenating with an empty string to concatenating with a one-space string, we can remove the ternary in the return statement checking for $lineCount being empty.

The existing unit tests already cover this code and still pass after this change.

PHPStan flags the code within the `Source::__toString()` method:
```
 ------ -------------------------------------------------------------------------------------------
  Line   DocBlock\Tags\Source.php
 ------ -------------------------------------------------------------------------------------------
  111    Result of || is always true.
  114    Result of || is always true.
  114    Result of || is always true.
 ------ -------------------------------------------------------------------------------------------
```

I have investigated this and can confirm that these flags are correct.

1. `$this->startingLine` is cast to an integer in the `__construct()` method (line 45) and subsequently cast to a string in `__toString()` (line 105).
    This means that it can only ever be a non-empty ("truthy") string or the string '0', so the `$startingLine || $startingLine === '0'` condition used in two places is redundant.
2. `$this->lineCount` is either an integer or `null` after the `__construct()` method (line 46).
    In the `__toString()` method, if the `lineCount` is an integer, it is effectively cast to a string by the concatenation with an empty string on line 107, while if the `lineCount` was `null`, it is turned into an empty string.
    By changing the concatenation from concatenating with an empty string to concatenating with a one-space string, we can remove the ternary in the `return` statement checking for `$lineCount` being empty.

The existing unit tests already cover this code and still pass after this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants