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

Class definitions should not be part of DiscouragedPHPFunctions warnings #898

Closed
frankiejarrett opened this issue Mar 30, 2017 · 2 comments · Fixed by #1317
Closed

Class definitions should not be part of DiscouragedPHPFunctions warnings #898

frankiejarrett opened this issue Mar 30, 2017 · 2 comments · Fixed by #1317
Milestone

Comments

@frankiejarrett
Copy link

This seems like a bug to me. You should be able to name your classes anything you want, especially within a namespace.

If you were going to run sniffs on class names (which I can't think of any use cases for) they'd be better off in their own DiscouragedPHPClasses sniff.

Consider the following test.php:

<?php

namespace Foo;

class System {
	// Things
}

class Serialize {
	// Things
}

And WordPress_Sniffs_PHP_DiscouragedPHPFunctionsSniff will throw:

FILE: test.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
  5 | WARNING | system() found. PHP system calls are often disabled
    |         | by server admins.
  9 | WARNING | serialize() found. Serialized data has known
    |         | vulnerability problems with Object Injection. JSON is
    |         | generally a better approach for serializing data. See
    |         | https://www.owasp.org/index.php/PHP_Object_Injection
----------------------------------------------------------------------
@JDGrimes
Copy link
Contributor

Yes, this is definitely a bug, the sniff needs to only flag function calls and not just any T_STRING. Adding T_CLASS to this list should fix the issue, I think.

@ocean90
Copy link
Member

ocean90 commented Apr 17, 2017

Related: #719

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants