Coding-standard: FullyQualifiedGlobal{Constants,Functions}Sniff: Configuration options needed

[Majkl578]

It should be possible to:

• specify whether use is allowed for referencing functions/constants,
• specify whether \ is allowed for referencing functions/constants,
• specify how to apply the fix, either by prepending \ (currently forced now) or by adding use.

Original PR: #235

[kukulich]

I think it could be 4 new sniffs.

[janedbal]

Moving from #235:
I was really looking forward for this feature. IMHO it could be solved by better naming of the sniff. The problem is that the original sniff was implemented for too narrow purpose and Majkl's update therefore doesn't completely fit in. What do you think?

[kukulich]

As I said in #235 I prefer more sniffs:

• FullyQualifiedConstantsSniff: all constants have to be FQN, the same as FullyQualifiedGlobalConstants but not only for global constants, the option exlude should be supported too.
• FullyQualifiedFunctionsSniff: all functions have to be FQN, the same as FullyQualifiedGlobalFunctions but not only for global functions, the option exlude should be supported too.

FullyQualifiedGlobalConstants and FullyQualifiedGlobalFunctions could be then marked as deprecated probably and removed in later version.

To force use is little complicated because there's already ReferenceUsedNamesOnlySniff. Not sure how complicated would be to implemented functions and contants in this sniff.

[kukulich]

It may by better to split ReferenceUsedNamesOnlySniff to 3 sniffs: classes, functions and constants.

[kukulich]

It looks that some support in ReferenceUsedNamesOnlySniff was already there, just needed to fix: https://github.com/slevomat/coding-standard/commit/998b5e96ce36a55d7821d17f39d296a17c05b481

[Majkl578]

Tried to configure ReferenceUsedNamesOnlySniff somehow, but haven't found a way to have reports that global functions are used inside namespace without use function. That kind of defeats the idea. :/ Maybe it doesn't take into consideration the fact that non-imported functions/constants automatically fall back to global scope? (Though there could be an issue with functional code and same-namespace functions.)

[kukulich]

First problem is here: https://github.com/slevomat/coding-standard/blob/998b5e96ce36a55d7821d17f39d296a17c05b481/SlevomatCodingStandard/Sniffs/Namespaces/ReferenceUsedNamesOnlySniff.php#L170

Not sure if it's fixable in this sniff. It's too complicated. I'll look at it hopefully in next few days.

[kukulich]

@Majkl578 Could you please try https://github.com/slevomat/coding-standard/tree/global-functions-and-constants ?

[Majkl578]

@kukulich Looks promising! Trying:

<property name="allowFullyQualifiedGlobalClasses" type="boolean" value="true"/>
<property name="allowFullyQualifiedGlobalFunctions" type="boolean" value="false"/>
<property name="allowFullyQualifiedGlobalConstants" type="boolean" value="false"/>
<property name="allowFullyQualifiedNameForCollidingClasses" type="boolean" value="true"/>

It seems to collide with allowFullyQualifiedNameForCollidingClasses - when enabled, global non-imported functions are not reported, only those with \ in front are. When disabled, they're properly reported.
Also I'd suggest different message, current one is a bit confusing: Function array_keys() should not be referenced via a fully qualified name, but via a use statement.

Thanks!

[kukulich]

@Majkl578 Thank you for the test. I think I should add new error code, new message and probably two new options: ignoreGlobalFunctions, ignoreGlobalConstants.

[kukulich]

@Majkl578 Could you try new version please?

[Majkl578]

Tested on my project and on Doctrine ORM with:

<property name="allowFullyQualifiedGlobalClasses" type="boolean" value="true"/>
<property name="allowFullyQualifiedGlobalFunctions" type="boolean" value="false"/>
<property name="allowFullyQualifiedGlobalConstants" type="boolean" value="false"/>
<property name="allowFullyQualifiedNameForCollidingClasses" type="boolean" value="true"/>
<property name="allowFallbackGlobalFunctions" type="boolean" value="false"/>
<property name="allowFallbackGlobalConstants" type="boolean" value="false"/>

LGTM. 💯 One issue only: the uses are not added in alphabetical order.

[kukulich]

Thank you for testing. I will finish the sniff next week and do new release.

Alphabetical order should be fixed by AlphabeticallySortedUsesSniff

[Majkl578]

Alphabetical order should be fixed by AlphabeticallySortedUsesSniff

True, it's minor issue, but phpcbf must be executed again, or is there an option to specify sniff order/dependency?

[kukulich]

phpcbf runs fixer in 50 loops so only if there’s a lot of changes in one place it’s not fixed in the first run.

[kukulich]

Released 4.3.0 with improved support for functions and constants in ReferenceUsedNamesOnlySniff.

[Majkl578]

Let's close this, I think it fulfills my expectations, thanks. 😊 @janedbal have you given it a try?

[janedbal]

Works great! Thank you very much! Found only one small issue: #239

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.