Skip to content

[3.0]Closures support $this in PHP >= 5.4 #14365

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
May 16, 2015
Merged

[3.0]Closures support $this in PHP >= 5.4 #14365

merged 1 commit into from
May 16, 2015

Conversation

dosten
Copy link
Contributor

@dosten dosten commented Apr 15, 2015

Q A
Bug fix? no
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
License MIT

Closures support $this in PHP >= 5.4

ping @aitboudad

@aitboudad
Copy link
Contributor

@dosten you need change dumpCatalogue too, see Translator.php#L392

@dosten
Copy link
Contributor Author

dosten commented Apr 15, 2015

@aitboudad done

@aitboudad
Copy link
Contributor

It would be better to group in one PR all changes for closures support > 5.4, not only Translator ?

@dosten
Copy link
Contributor Author

dosten commented Apr 15, 2015

I will go to make all changes in this PR since this only makes some changes missing in #12841

@dosten dosten changed the title [Translation] Closures support $this in PHP >= 5.4 Closures support $this in PHP >= 5.4 Apr 15, 2015
@@ -354,8 +352,6 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
}

/**
* This method is public because it needs to be callable from a closure in PHP 5.3. It should be converted back to protected in 3.0.
* @internal
* @return GeneratorDumperInterface
*/
public function getGeneratorDumperInstance()
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be converted to protected

Copy link
Member

Choose a reason for hiding this comment

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

to private, not to protected

Copy link
Contributor

Choose a reason for hiding this comment

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

no it was protected before 2.7

Copy link
Member

Choose a reason for hiding this comment

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

@dosten
Copy link
Contributor Author

dosten commented Apr 16, 2015

@aitboudad done, something more?

@aitboudad
Copy link
Contributor

no it's ok for me, 👍

@aitboudad aitboudad changed the title Closures support $this in PHP >= 5.4 [3.0]Closures support $this in PHP >= 5.4 Apr 16, 2015
@xabbuh
Copy link
Member

xabbuh commented Apr 16, 2015

👍

@mpdude
Copy link
Contributor

mpdude commented Apr 16, 2015

As the one who wrote most of this (or that) code, thanks for bringing this up! 👍

@dosten
Copy link
Contributor Author

dosten commented Apr 21, 2015

ping @Tobion @stof

$baseClass = $this->options['matcher_base_class'];
$expressionLanguageProviders = $this->expressionLanguageProviders;
$that = $this; // required for PHP 5.3 where "$this" cannot be use()d in anonymous functions. Change in Symfony 3.0.

$cache = $this->getConfigCacheFactory()->cache($this->options['cache_dir'].'/'.$class.'.php',
Copy link
Contributor

Choose a reason for hiding this comment

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

$class is not defined, please look at tests

Copy link
Contributor

Choose a reason for hiding this comment

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

it should be $this->options['matcher_cache_class']

@dosten
Copy link
Contributor Author

dosten commented May 1, 2015

ping @fabpot

@@ -320,25 +315,22 @@ public function getGenerator()
if (null === $this->options['cache_dir'] || null === $this->options['generator_cache_class']) {
$this->generator = new $this->options['generator_class']($this->getRouteCollection(), $this->context, $this->logger);
} else {
$class = $this->options['generator_cache_class'];
$baseClass = $this->options['generator_base_class'];
$that = $this; // required for PHP 5.3 where "$this" cannot be use()d in anonymous functions. Change in Symfony 3.0.
$cache = $this->getConfigCacheFactory()->cache($this->options['cache_dir'].'/'.$class.'.php',
Copy link
Contributor

Choose a reason for hiding this comment

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

same problem

@Tobion
Copy link
Contributor

Tobion commented May 1, 2015

@dosten the code is still broken. please make sure the tests pass before pinging people.

@dosten
Copy link
Contributor Author

dosten commented May 1, 2015

@Tobion sorry, fixed and rebased but the tests still failing: The service definition "templating.helper.assets" does not exist.

@nicolas-grekas
Copy link
Member

@dosten can you please rebase+squash?

@dosten
Copy link
Contributor Author

dosten commented May 15, 2015

@nicolas-grekas done

@fabpot
Copy link
Member

fabpot commented May 16, 2015

Thank you @dosten.

@fabpot fabpot merged commit 38f32c1 into symfony:master May 16, 2015
fabpot added a commit that referenced this pull request May 16, 2015
This PR was merged into the 3.0-dev branch.

Discussion
----------

[3.0]Closures support $this in PHP >= 5.4

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

Closures support `$this` in PHP >= 5.4

ping @aitboudad

Commits
-------

38f32c1 $this can be used inside a closure in PHP >= 5.4
@dosten dosten deleted the remove-self-var branch May 16, 2015 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants