Skip to content

Added class existence check if is_subclass_of() fails in compiler passes #19342

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

Merged
merged 1 commit into from
Jul 18, 2016

Conversation

SCIF
Copy link
Contributor

@SCIF SCIF commented Jul 12, 2016

Q A
Branch? 2.8-3.1
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

If you create an event subscriber and make typo in file name it will cause next error:

  [InvalidArgumentException]                                                                                                      
  Service "event.notification_subscriber" must implement interface "Symfony\Component\EventDispatcher\EventSubscriberInterface".

That's because of is_subclass_of() fails on class absentee. I made error message more clear.

if (!is_subclass_of($class, $interface)) {
if (!class_exists($class, true)) {
Copy link
Member

Choose a reason for hiding this comment

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

true should be false: autoloading already happened the line before

@SCIF
Copy link
Contributor Author

SCIF commented Jul 17, 2016

@nicolas-grekas , i updated a code according your notes.

@nicolas-grekas
Copy link
Member

👍

if (!is_subclass_of($class, $interface)) {
if (!class_exists($class, false)) {
throw new \InvalidArgumentException(sprintf('Class "%s" used for service "%s" can not be found.', $class, $id));
Copy link
Member

Choose a reason for hiding this comment

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

"cannot" instead of "can not" (same below)

@SCIF
Copy link
Contributor Author

SCIF commented Jul 18, 2016

@xabbuh , updated

@xabbuh
Copy link
Member

xabbuh commented Jul 18, 2016

👍

@fabpot
Copy link
Member

fabpot commented Jul 18, 2016

Thank you @SCIF.

@fabpot fabpot merged commit 72db6e7 into symfony:2.8 Jul 18, 2016
fabpot added a commit that referenced this pull request Jul 18, 2016
…ompiler passes (SCIF)

This PR was merged into the 2.8 branch.

Discussion
----------

Added class existence check if is_subclass_of() fails in compiler passes

| Q             | A
| ------------- | ---
| Branch?       | 2.8-3.1
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

If you create an event subscriber and make typo in file name it will cause next error:

```
  [InvalidArgumentException]
  Service "event.notification_subscriber" must implement interface "Symfony\Component\EventDispatcher\EventSubscriberInterface".
```

That's because of `is_subclass_of()` fails on class absentee. I made error message more clear.

Commits
-------

72db6e7 Added class existence check if is_subclass_of() fails in compiler passes
@SCIF SCIF deleted the 2.8 branch July 18, 2016 19:18
@chalasr
Copy link
Member

chalasr commented Jul 27, 2016

Should not this be back ported on the 2.7 branch?

@SCIF
Copy link
Contributor Author

SCIF commented Jul 27, 2016

@chalasr, it's my fault :( Just missed that 2.7 is lts and still maintainable. @fabpot can you merge it to 2.7? I tried and it merged ok.

This was referenced Jul 30, 2016
@SCIF
Copy link
Contributor Author

SCIF commented Aug 3, 2016

@fabpot , is there any chance to merge that PR to 2.7?

@nicolas-grekas
Copy link
Member

@SCIF please openq new PR

@SCIF
Copy link
Contributor Author

SCIF commented Aug 4, 2016

@nicolas-grekas , will do, thanks!

nicolas-grekas added a commit that referenced this pull request Aug 9, 2016
…ompiler passes (SCIF)

This PR was merged into the 2.7 branch.

Discussion
----------

Added class existence check if is_subclass_of() fails in compiler passes

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any
| License       | MIT
| Doc PR        | no

Backport of #19342 to 2.7 branch

Commits
-------

77adea7 Added class existence check if is_subclass_of() fails in compiler passes
nicolas-grekas added a commit that referenced this pull request Sep 6, 2016
…lass_of (chalasr)

This PR was merged into the 2.7 branch.

Discussion
----------

[FrameworkBundle] Check for class existence before is_subclass_of

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Same as #19342

Commits
-------

8a9e0f5 [FrameworkBundle] Check for class existence before is_subclass_of
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.

7 participants