search

Automattic / vip-scanner

Deprecated: Scan all sorts of themes and files and things! Use PHPCS and the VIP coding standards instead
https://automattic.com
140 stars 51 forks source link

Fatal errors on Checks that don't over-ride `CodeCheck::check()` #275

Closed nickdaugherty closed 9 years ago

nickdaugherty commented 9 years ago

https://github.com/Automattic/vip-scanner/blob/f9da641a12fd8704f20be9c9e3fcfba5f0b35ad4/vip-scanner/class-code-check.php#L53

Here, the $php_file in the loop is an array of filename => (string) contents...which causes a fatal error when we try to call $php_file->get_node_tree() on the non-object. I think we're missing some glue somewhere...

Should we really be treating all code as PHP code here? Many files go through this function, including CSS, JS, .DS_Store, etc.

This happens in at least the following classes:

DeprecatedConstantsCheck DeprecatedFunctionsCheck DeprecatedParametersCheck VIPRestrictedClassesCheck VIPRestrictedCommandsCheck VIPParametersCheck

cc @ockham

ockham commented 9 years ago

CodeCheck is only meant as a base class for checks that are run on PHP code; BaseScanner has a conditional to make sure that classes derived from CodeCheck are given pre-parsed ('analyzed') PHP files only, while any other files are passed to classes derived from BaseCheck.

Did this error occur when checking any particular theme? (Just trying to reproduce...)

ockham commented 9 years ago

I'm suspecting the actual problem is this line, where pre-parsed CSS files are also added to $this->analyzed_files. (Both analyzed PHP and CSS files are currently dumped into the same array so they're all passed later to the respective analyzers.) I'll try to come up with a proper fix.