Skip to content
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

Errors regarding usage of deprecated core functions are too strict in a context of a namespace #1244

Open
markkap opened this issue Nov 28, 2017 · 9 comments

Comments

@markkap
Copy link

markkap commented Nov 28, 2017

In the following code

namespace dummy;

function get_settings() {
   return 'settings';
}

echo get_settings();

You are going to get a notice about the usage of the deprecated function get_settings() although the one being called in this code is the "local" one.

Suggested solution, when in the context of a namespace look for the existence of a "\" before the deprecated function name.

@jrfnl
Copy link
Member

jrfnl commented Nov 28, 2017

@markkap Thanks for reporting this. A lot of sniffs have been written with PHP 5.2 support in mind and they don't all handle more modern code 100% correctly, so this will be a good example to use to fix this.

Related to #764

@jrfnl
Copy link
Member

jrfnl commented Mar 1, 2018

@markkap I've just had a quick look at this and I can't think of a simple solution.

The problem is that - unless, like in your example, the namespaced function definition is in the same file as the function call -, there is no way to reliably know whether that function is defined within the namespace or not.

If not, PHP will fall back on the global function which would mean, it would use the deprecated function after all.

For functions and constants, PHP will fall back to global functions or constants if a namespaced function or constant does not exist.

Ref: http://php.net/manual/en/language.namespaces.fallback.php

@markkap
Copy link
Author

markkap commented Mar 1, 2018

@jrfnl, yes :(

OTOH if it is impossible to know which function is being referred to, it is hard to say that you are calling a deprecated one.

Maybe what can be done is to turn off this check when there is a namespace "use". get_settings is just an obvious generic name that should not generate errors, maybe for other deprecated functions it is less of an issue.

@jrfnl
Copy link
Member

jrfnl commented Mar 1, 2018

@markkap The error will not be turned off in WPCS itself, though I will look into checking use statements at some point.

In the mean time, you can turn it off for your own projects by adding <exclude name="WordPress.WP.DeprecatedFunctions.get_settingsFound"/> to your custom ruleset.

@markkap
Copy link
Author

markkap commented Mar 1, 2018 via email

@jrfnl
Copy link
Member

jrfnl commented Mar 1, 2018

@markkap Methods will not generate false positives, it's just namespaced functions without the namespace prefix, so I wouldn't expect that to have much of an impact.

@markkap
Copy link
Author

markkap commented Mar 1, 2018 via email

@jrfnl
Copy link
Member

jrfnl commented Mar 1, 2018

@markkap I already said, I will look into checking use statements at some point.

Other than that, you're welcome to try and find a workable solution yourself. I'd be happy to look a PR over.

@jrfnl
Copy link
Member

jrfnl commented Mar 14, 2020

Tentatively marking this for WPCS 3.0. PHPCSUtils will contain an Abstract sniff for function call detection which is better than the one we are currently using and would, at least partially solve this.

@jrfnl jrfnl added this to the 3.0.0 milestone Mar 14, 2020
@jrfnl jrfnl modified the milestones: 3.0.0, 3.x Next Apr 15, 2022
@jrfnl jrfnl removed this from the 3.x Next milestone Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants