-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Debug] Trigger a deprecation when using an internal class/trait/interface #23807
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
Conversation
Fabbot failure unrelated. |
@@ -166,10 +167,14 @@ public function loadClass($class) | |||
} | |||
|
|||
$parent = get_parent_class($class); | |||
$doc = $refl->getDocComment(); | |||
if (preg_match('#\n \* @internal(?: .+?)?\r?\n \*(?: @|/$)#s', $doc, $notice)) { | |||
self::$internal[$name] = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should keep an reuse the notice in the same way as we do for "final"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I updated the code base to have consistent messages.
} | ||
} | ||
|
||
foreach (call_user_func(function () use ($name, $parent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we make this closure into a method? that'd ease perf and reading
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just removed it in favor of an array_merge
call
@@ -30,8 +30,7 @@ | |||
* | |||
* @see ExecutionContextInterface | |||
* | |||
* @internal You should not instantiate or use this class. Code against | |||
* {@link ExecutionContextInterface} instead. | |||
* @internal since version 3.5. Code against ExecutionContextInterface instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2.5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, indeed
Thank you @GuilhemN. |
…lass/trait/interface (GuilhemN) This PR was squashed before being merged into the 3.4 branch (closes #23807). Discussion ---------- [Debug] Trigger a deprecation when using an internal class/trait/interface | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes <!-- don't forget updating src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | not truly <!-- don't forget updating UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | #23800 | License | MIT | Doc PR | Trigger a deprecation when the user uses an internal class/trait/interface. The deprecation is not triggered when an `@internal` is used in the same vendor. Commits ------- b89ba29 [Debug] Trigger a deprecation when using an internal class/trait/interface
I merged quickly so you can continue on #23816 (reviewers, if you had any comment here, please comment on this other PR) |
Trigger a deprecation when the user uses an internal class/trait/interface.
The deprecation is not triggered when an
@internal
is used in the same vendor.