-
-
Notifications
You must be signed in to change notification settings - Fork 490
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
Escape output sniff fix for static method calls #2370
base: develop
Are you sure you want to change the base?
Conversation
Is that true though ? Shouldn't the sniff skip over the method call, but then continue ? Test case: echo My::method($param), $somethingElse; // I'd expect two errors here, one for the method call, one for $somethingElse. |
Knew there must be edge cases I missed 😅 I'll work on this some more in the following days, I need to get some sleep 🙈 |
@dingo-d Should this PR be moved back to draft for the time being ? |
5f01f59
to
2fd4721
Compare
@jrfnl The tests seem to be passing now, only the 2 seem to be stalling (or just taking an unusually long time). |
Need to write a recursive method that will check the fully qualified class names and if they have a static method call in them. We should also be careful not to catch the throw Exception cases, as for those we do want to check the parameters of the static method if they are escaped or not.
Add check for static public properties, enums and constants.
Add a way to identify static methods in the is_escaping_function, used for setting the customEscapingFunctions.
Split the merge function to merge lowercased method/function names, because PHP is case insensitive for all class, namespace and method names.
8f336b7
to
4a48492
Compare
Any chance we can move this PR along? |
As it currently is the following code
Would trigger the EscapeOutput sniff for the class name, but also for any attributes of the class method, except the first one.
The sniff should just trigger the class name, and bow out after that (what is inside the method is of no concern, as the output is all that matters).
I've added a test case and found the part that caused the issue (the
*::class
part didn't take into account static methods, only the*::class
case).This should fix it.
Adresses part of #1176