search

sirbrillig / phpcs-changed

🐘 Run phpcs on files and only report new warnings/errors compared to the previous version.
MIT License
31 stars 11 forks source link

Fixed typo when checking if the field `diffFile` is empty. #99

Closed dmitr1y closed 5 months ago

dmitr1y commented 5 months ago

After moving the options to CliOptions in commit d7eb1168f5d4054, the field diff was renamed to diffFile, which caused a bug in the handler for the --diff argument.

sirbrillig commented 5 months ago

Wow, nice find. That must have totally broken manual mode. Interesting that static analysis missed that; possibly because empty() works on undefined values? I'll check. There's some static analysis errors but I think they might just be because Psalm has become more strict. I'll make sure that's the case and if so I can fix them in a different PR.

sirbrillig commented 5 months ago

Interesting that static analysis missed that; possibly because empty() works on undefined values? I'll check.

Yup. That's the case. If I remove the empty(), then $this->diff shows an error. I may remove all those empty() checks.

ERROR: UndefinedThisPropertyFetch - PhpcsChanged/CliOptions.php:369:8 - Instance property PhpcsChanged\CliOptions::$diff is not defined (see https://psalm.dev/041)
                        if (!$this->diff || empty($this->phpcsUnmodified) || empty($this->phpcsModified)) {