Sniff idea: Detect when constant is being used in constant()

[rebeccahum]

trafficstars

Describe the solution you'd like

Not sure if VIPCS is the best way forward, but I think this would be a worthy sniff.

When using constant(), we should throw a warning if a string is not being inputted. Of course it is valid PHP to do something like:

constant( FOO_BAR );

But I think most likely, what the user wants to do is:

constant( 'FOO_BAR' );

What code should be reported as a violation?

constant( FOO_BAR );

What code should not be reported as a violation?

constant( 'FOO_BAR' );
constant( "FOO_BAR" );
constant( $foo_bar );

[jrfnl]

Hmm... while I understand where you are coming from, I don't think this is a good idea. The constant() function should only be used when the name of the constant isn't known or is variable, so if it is known - aka is a text string -, devs shouldn't be using the constant() function, but should be using the constant directly.

When I - as a dev - look at the above code samples, I would say that:

• constant( FOO_BAR ); and constant( $foo_bar ); should NOT be a violation (though the first should be very rare anyway and mostly in the form of constant( static::FOO_BAR ); for a (child) class to be allowed to overrule the constant name being used.
• while constant( 'FOO_BAR' ); and constant( "FOO_BAR" ); should be a violation as the dev should just use the constant directly.

Ref: https://www.php.net/manual/en/function.constant.php

[rebeccahum]

while constant( 'FOO_BAR' ); and constant( "FOO_BAR" ); should be a violation as the dev should just use the constant directly.

Agreed. But in instances like our vip-go-mu-plugins, sometimes we have to call constant( 'FOO_BAR' ) in our code to make our unit tests happy: https://github.com/Automattic/vip-go-mu-plugins/blob/fdd99cf535e55eeda46d7a11a4cbb1603d8bc666/tests/mock-constants.php. So, doing that can result in us having to go against best practices in how constant() is used :/.

[jrfnl]

But in instances like our vip-go-mu-plugins, sometimes we have to call constant( 'FOO_BAR' ) in our code to make our unit tests happy: https://github.com/Automattic/vip-go-mu-plugins/blob/fdd99cf535e55eeda46d7a11a4cbb1603d8bc666/tests/mock-constants.php. So, doing that can result in us having to go against best practices in how constant() is used :/.

@rebeccahum Just had a look, but in those cases, you are not calling the PHP native global constant() function, but a namespaced function called constant().