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

feat(cs-config, performance): add native_function_invocation rule #22

Merged
merged 1 commit into from
May 25, 2023

Conversation

yannchabed
Copy link
Contributor

Why?

native_function_invocation rule adds leading \ before function invocation to speed up resolving.

refer to the rule documentation

strict parameter is set to false to keep the leading \in case it is not necessary. (Limits the amount of fixes)

⚠️ this rule is risky in case native function rules are overriden (implies a major version)

Example:

--- Original
+++ New
 <?php

 function baz($options)
 {
-    if (!array_key_exists("foo", $options)) {
+    if (!\array_key_exists("foo", $options)) {
         throw new \InvalidArgumentException();
     }

     return json_encode($options);
 }

Why setting strict parameter false ?

true value (default): The unnecessary leading \ are removed

--- Original
+++ New
 <?php

 function baz($options)
 {
-     return \json_encode($options);
+     return json_encode($options);

 }

false value: The unnecessary leading \ are kept. One can chosse to keep it or not.

--- Original
+++ New
 <?php

 function baz($options)
 {
     return \json_encode($options);
 }

TODO

  • code is tested (locally)
  • documentation is updated (if needed)

@yannchabed yannchabed self-assigned this May 3, 2023
@yannchabed yannchabed requested a review from a team as a code owner May 3, 2023 08:30
src/BedrockStreaming.php Outdated Show resolved Hide resolved
Copy link
Member

@Oliboy50 Oliboy50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤞

@yannchabed yannchabed force-pushed the feat/add-native-function-invocation-rule branch from 9c77ac7 to d5a783e Compare May 3, 2023 12:02
@yannchabed yannchabed force-pushed the feat/add-native-function-invocation-rule branch from d5a783e to d63bdcc Compare May 25, 2023 07:48
@yannchabed yannchabed merged commit 72708c4 into master May 25, 2023
@yannchabed yannchabed deleted the feat/add-native-function-invocation-rule branch May 25, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants