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

NativeFunctionInvocationFixer - namespaced strict to remove backslash #4220

Merged
merged 1 commit into from Jan 4, 2019
Merged

NativeFunctionInvocationFixer - namespaced strict to remove backslash #4220

merged 1 commit into from Jan 4, 2019

Conversation

kubawerlos
Copy link
Contributor

When we want only namespaced calls to have \ in strict mode we should remove \ for global namespace call.

@SpacePossum SpacePossum added this to the 2.12.6 milestone Jan 2, 2019
@keradus
Copy link
Member

keradus commented Jan 4, 2019

Thank you @kubawerlos.

@keradus keradus merged commit 1241bda into PHP-CS-Fixer:master Jan 4, 2019
keradus added a commit that referenced this pull request Jan 4, 2019
… backslash (kubawerlos)

This PR was merged into the 2.14-dev branch.

Discussion
----------

NativeFunctionInvocationFixer - namespaced strict to remove backslash

When we want only namespaced calls to have `\` in strict mode we should remove `\` for global namespace call.

Commits
-------

1241bda NativeFunctionInvocationFixer - namespaced strict to remove backslash
@keradus keradus modified the milestones: 2.12.6, 2.14.0 Jan 4, 2019
@keradus
Copy link
Member

keradus commented Jan 4, 2019

fixing milestone... strict was added in 2.14

@kubawerlos kubawerlos deleted the fix-native-function-invokation-namespaced-strict branch January 4, 2019 14:48
@voskobovich
Copy link

Can I have more information about the bug? I don't understand why we remove the slash in the function call.

@kubawerlos
Copy link
Contributor Author

@voskobovich take a look at the test - we remove backslash only for function in global namespace when fixer configured scope is namespaced and we are in strict mode.

@voskobovich
Copy link

@kubawerlos Ok. I understand what you are doing. But I do not understand why this is necessary? Hard specification of the global namespace for a function should speed up its search when executing code.

@kubawerlos
Copy link
Contributor Author

@voskobovich this is not necessary, you can use scope all, yet it was bugged when scope namespaced was used.

Hard specification of the global namespace for a function should speed up its search when executing code.

That is not true - opcode is the same, not matter if backaslash is used: https://3v4l.org/aK4ZR/vld#output and https://3v4l.org/NCSFH/vld#output.

@voskobovich
Copy link

@kubawerlos Thank! This was new information for me))

@keradus
Copy link
Member

keradus commented Jan 10, 2019

PHP CS Fixer sharing the knowledge and good practices ;)

@mfn
Copy link

mfn commented Aug 17, 2020

Can I use this rule for the opposite, ensuring that all \ are in fact removed (please don't discuss pro/cons => different topic).

I read the current readme for "native_function_invocation" but I'm unsure.

  • exclude (array): list of functions to ignore; defaults to []
  • include (array): list of function names or sets to fix. Defined sets are @internal (all native functions), @ALL (all global functions) and @compiler_optimized (functions that are specially optimized by Zend); defaults to ['@internal']
  • scope ('all', 'namespaced'): only fix function calls that are made within a namespace or fix all; defaults to 'all'
  • strict (bool): whether leading \ of function call not meant to have it should be removed; defaults to false

I figured maybe with this I can achieve this, but it doesn't work:

    'native_function_invocation' => [
      'exclude' => ['@all'],
      'strict' => true,
    ],

Still adds \ everywhere and haven't noticed removing it where it's present.

Thanks :)

@mfn
Copy link

mfn commented Aug 17, 2020

ensuring that all \ are in fact removed

Apologies, I re-read my comment and I think it's not really clear:
the goal would be not to have function imports but just remove the \ on global functions (and not exchanging the \ for an import)

@kubawerlos
Copy link
Contributor Author

@mfn try:

  'native_function_invocation' => [
      'include' => [],
      'strict' => true,
    ],

@mfn
Copy link

mfn commented Aug 18, 2020

How on eart…

Re-reading the readme I guess it makes sense but I still fail to distill this from the text 🤷‍♀️

Anyway, the one combination I did not try 😅, it works, so thank you 👍

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

Successfully merging this pull request may close these issues.

None yet

5 participants