Skip to content

Conversation

@benesch
Copy link
Contributor

@benesch benesch commented Dec 30, 2015

Given a namespaced file which uses a top-level name like

namespace mindplay;

use PDO;

the parser will incorrectly generate the following uses information:

array('DO' => 'PDO')

This commit adjusts the parser to handle use statements which import a
name without a namespace separator.

@benesch
Copy link
Contributor Author

benesch commented Dec 30, 2015

It's worth noting that the "HandlesMethodAliasingWithTraits" test case in #102 should be failing without this fix—but it's not due to #104!

@aik099
Copy link
Member

aik099 commented Dec 30, 2015

Where DO word would come from during use statement parsing, that you've mentioned in issue description?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's better to be written as if/else without ternary operator to be more readable. Now the 1 + -1 construct that is evaluated at the end is a bit confusing.

@aik099
Copy link
Member

aik099 commented Dec 30, 2015

Today’s review completed.

@benesch
Copy link
Contributor Author

benesch commented Dec 30, 2015

DO is the result of an off-by-one error on PDO. I've updated the code to use explode and array_pop, which is the most readable method I could come up with.

@benesch benesch mentioned this pull request Dec 30, 2015
Copy link
Member

Choose a reason for hiding this comment

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

This line uses the explode function side effect that if char for explode wasn't found in a string the whole string is returned as 1st array element.

It's better to be more specific (this code will replace this and following line):

if (strpos($use, '\\') !== false) {
    $uses[substr($use, 1 + strrpos($use, '\\'))] = $use;
}
else {
    $uses[$use] = $use;
}

@aik099
Copy link
Member

aik099 commented Dec 31, 2015

Today’s review completed.

Given a namespaced file which uses a top-level name like

    namespace mindplay;

    use PDO;

the parser will incorrectly generate the following uses information due
to an off-by-one error:

    array('DO' => 'PDO')

This commit adjusts the parser to handle use statements which import a
name without a namespace separator.
@benesch
Copy link
Contributor Author

benesch commented Jan 9, 2016

Adjusted as requested!

aik099 pushed a commit that referenced this pull request Jan 9, 2016
Fix top-level use statements
@aik099 aik099 merged commit 5446fb5 into php-annotations:master Jan 9, 2016
@aik099
Copy link
Member

aik099 commented Jan 9, 2016

Looks good. Merging. Thanks, @benesch .

@benesch benesch deleted the top-level-uses branch January 14, 2016 22:38
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.

2 participants