Skip to content
This repository has been archived by the owner on Mar 30, 2018. It is now read-only.

Mutate only methods, no functions #201

Merged
merged 6 commits into from
May 13, 2017
Merged

Mutate only methods, no functions #201

merged 6 commits into from
May 13, 2017

Conversation

kelunik
Copy link
Contributor

@kelunik kelunik commented Jan 8, 2017

According to @MarkRedeman, only methods should be mutated right now and
function support is pending. $inMethod was set to true for functions
as well, because it didn't check whether it's inside a class.

According to @MarkRedeman, only methods should be mutated right now and
function support is pending. `$inMethod` was set to `true` for functions
as well, because it didn't check whether it's inside a class.
@kelunik kelunik mentioned this pull request Jan 8, 2017
@kelunik
Copy link
Contributor Author

kelunik commented Jan 8, 2017

This fixes tests on PHP 5.5, 5.6 and 7.

Copy link
Member

@theofidry theofidry left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @kelunik and sorry for the late answer. Would you mind to add a test fore it? (that a function is not mutated)

@@ -23,7 +23,7 @@ Feature: Use Humbug
$foo = new Foo;
$r = $foo->add(2, 1);
// emulate error
if ($r !== 3) $foo->bar();
if ($r !== 3) require __DIR__ . "/this.does.not.exist.xyz";
Copy link
Member

Choose a reason for hiding this comment

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

let's go with single quotes

src/Mutable.php Outdated
@@ -146,7 +146,7 @@ public function generate()
}

//TODO: handle whitespace!
if (is_array($token) && $token[0] == T_FUNCTION) {
if (is_array($token) && $token[0] == T_FUNCTION && $className !== '???') {
Copy link
Member

Choose a reason for hiding this comment

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

is_array($token) && $token[0] === T_FUNCTION && '???' !== $className

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why Yoda comparison? It's not consistently used and decreases readability IMO.

Copy link
Member

Choose a reason for hiding this comment

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

To avoid things like $token[0] = T_FUNCTION. We could argue about having assignments in a function call or if condition, but it's sometimes used. Yoda condition in those cases prevent those mistakes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a potential mistake I probably never made. It's not worth the readability tradeoff IMO.

Copy link
Member

@theofidry theofidry Apr 20, 2017

Choose a reason for hiding this comment

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

I thought the same before making it once :) Also I don't think the tradeoff is that bad.

Copy link
Contributor

Choose a reason for hiding this comment

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

We wouldn't need yoda if languages implemented assignment as :=. :trollface:

@theofidry theofidry added this to the 1.0.0 milestone Apr 19, 2017
@padraic padraic merged commit 8ee0e7e into humbug:master May 13, 2017
@kelunik kelunik deleted the only-methods branch May 13, 2017 16:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants