Skip to content

[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

Closed
wants to merge 3 commits into from

Conversation

GuilhemN
Copy link
Contributor

@GuilhemN GuilhemN commented Aug 6, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? not truly
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.

@GuilhemN GuilhemN changed the base branch from master to 3.4 August 6, 2017 13:49
@GuilhemN
Copy link
Contributor Author

GuilhemN commented Aug 6, 2017

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;
Copy link
Member

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"

Copy link
Contributor Author

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) {
Copy link
Member

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

Copy link
Contributor Author

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.
Copy link
Member

Choose a reason for hiding this comment

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

2.5?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, indeed

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Aug 6, 2017
@nicolas-grekas nicolas-grekas changed the title Trigger a deprecation when using an internal class/trait/interface [Debug] Trigger a deprecation when using an internal class/trait/interface Aug 7, 2017
@nicolas-grekas
Copy link
Member

Thank you @GuilhemN.

nicolas-grekas added a commit that referenced this pull request Aug 7, 2017
…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
@nicolas-grekas
Copy link
Member

I merged quickly so you can continue on #23816 (reviewers, if you had any comment here, please comment on this other PR)

This was referenced Oct 18, 2017
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.

3 participants