nitpick a few PHP Inspections issues #103
Conversation
Codecov Report
@@ Coverage Diff @@
## master #103 +/- ##
=========================================
Coverage 98.47% 98.47%
Complexity 124 124
=========================================
Files 6 6
Lines 523 523
=========================================
Hits 515 515
Misses 8 8
Continue to review full report at Codecov.
|
| @@ -312,7 +312,7 @@ private function findNewDestination( | |||
| } | |||
|
|
|||
| $prevLine = $tokens[$prevPtr]['line']; | |||
| $prevImportArr = $this->getUseImport($phpcsFile, (int) $prevPtr); | |||
| $prevImportArr = (array) $this->getUseImport($phpcsFile, (int) $prevPtr); | |||
There was a problem hiding this comment.
Which quality tool was picky about that?
There was a problem hiding this comment.
PHP Inspections and there would still be a few other issues that got detected by it, but it would be IMHO too tedious to satisfy all the other tools we're using when "fixing" these.
There was a problem hiding this comment.
I don't think that this is overly helpful here, false conversed with (array) becomes an empty array. In the while condition we refer to to the array key content which would raise an error in case false is returned, so this doesn't prevent a bug, it simply makes the inspection tools happy.
So, getUseImport only returns false if use is the last token in the file which would be syntactically incorrect. I'm currently not sure how PHPCS behaves in case of incorrect files, but I'd prefer to remove the conversion here (I'm also wondering why the if ($start === false) condition was introduced in getUseImport if this simply covers a syntactically incorrect case.
There was a problem hiding this comment.
@mmoll, what was a warning related to the introduced array casting?
There was a problem hiding this comment.
@kalessil ohai :)
"'$prevImportArr' may not support offset operations (or its type not annotated properly: [bool, array])." on Line 317
There was a problem hiding this comment.
@mmoll hi :) Checked, the issue is correct, but even adding if ($prevImportArr === false) { break; } will not help. The getUseImport refactoring would be needed, but might be declined by maintainers.
No description provided.