40 check sanitize post|req|file|get

[frantorres]

Includes code for a PHP Parser that could be useful for following tests. Includes sanitize and escape checks.

[felixarntz]

@frantorres I'm curious to understand why this PR implements a PHP parser. Couldn't this use existing WPCS sniffs from PHPCodeSniffer instead?

[frantorres]

Hi @felixarntz I’m happy to see you over here 😁

I started with this approach while wasn’t conscious about using PHPCS for this specific checks, now I know that using PHPCS would have been pretty straightforward but this was the track I took at that point, anyway I have some other checks in mind like prefixing that will need this PHP Parser.

Good news is that doing checks this way instead of using PHPCS allows some additional control on the output, showing specific parts problems are and, something still in mind, suggesting possible solutions.

Code is still a bit dirty and needs polishing (I’m in a rush still actively testing it with different code from different developers) but functional for most of the cases, specially most common cases.

[BrianHenryIE]

I have some other checks in mind like prefixing that will need this PHP Parser.

Can you elaborate on this? I maintain BrianHenryIE/strauss which prefixes (although with regex, not PhpParser). I agree WordPress plugins' libraries should be prefixed, with few exceptions (which I haven't really settled on myself).

I agree with using PHPCS as much as possible too. Maybe you could get what is needed by extending the existing sniff to improve the output. I'd worry if the source of truth becomes plugin-check, then WPCS would be a lot of duplicate work, and the PHP and WordPress worlds would diverge a little as PHPCS is used less.

[frantorres]

Hi @BrianHenryIE , that would be a check to detect which names that can collide is the plugin using. If it finds a namespace, will just show the namespace, if there is no a namespace and a class, will show the name of the class (ignoring function names inside it), if there is a bunch of functions (without namespace, class wrapping it, etc), the name of all those functions. Also, will show other things that also needs prefixing, like names when you save something to the options table.

I hope this never turns into the source of truth as I think WPCS is far more polished than this. There is no intention to be perfect here (that's not technically possible anyway), more like a simple way for developers to spot possible problems by themselves.

I'd be glad if anyone can work on a PHPCS-WPCS version, that's a great idea but I won't have the time to work on that, at least for now, as I'm focusing on having more checks even though those aren't perfect. My mood right now is "Done is better than perfect", will have time to improve and I'm sure some people can join to help.

[felixarntz]

@frantorres

I started with this approach while wasn’t conscious about using PHPCS for this specific checks, now I know that using PHPCS would have been pretty straightforward but this was the track I took at that point, anyway I have some other checks in mind like prefixing that will need this PHP Parser.

Would those other checks not be possible to achieve with PHPCodeSniffer? We need to consider the maintenance aspect here as well - PHPCS is a widely used and established tool, so relying on it is probably a lot more stable and less effort than to rely on another complex parser. You've already done the work of course, but we also need to consider the future cost of using a custom very complex piece of code vs PHPCS that is widely used for a similar purpose.

[frantorres]

@felixarntz I haven't had the chance to go through PHPCS and see the possibilities we can have there, I think you know more about it and maybe you can help at least assessing the possibilities we have there.

As for example, for prefixing I'm thinking about going thought all the plugin files reading all this structures and strings (namespaces, classes, functions, name of save option), get all the names used there and show up them together (and, as improvement to make it cristal clear, we can analyze those names to deduce if it's using prefixing and which is using).

While I use PHPCS with my editor, I think it works reading each file independently and showing 1 error for each coincidence with a specific format. But maybe I'm totally wrong about it, maybe you can clarify or check that.

And to be clear, I'm not sticking with this parser and I love PHPCS, I just have no clue about how it internally works, so maybe I'm not the guy to do those checks that way until I learn about it.