Skip to content

Conversation

TimeToogo
Copy link
Contributor

The current behaviour in the name resolver visitor is incorrect as special class names are case-insensitive as demonstrated here:
http://3v4l.org/4g7Hj

The current behaviour is incorrect as special class names are case-insensitive as demonstrated here:
http://3v4l.org/4g7Hj
@Ocramius
Copy link

Ocramius commented Aug 6, 2014

@TimeToogo can you also provide a test case?

@rdlowrey
Copy link

rdlowrey commented Aug 6, 2014

General +1 for test case inclusion in PRs.

@aik099
Copy link

aik099 commented Aug 6, 2014

I suspect that keywords in general are case insensitive.

Could you please add/update corresponding test case?

@GrahamCampbell
Copy link
Contributor

Maybe we should raise this issue with hhvm as a php5 compliance issue?

@TimeToogo
Copy link
Contributor Author

Added the corresponding test :)
Have to go to sleep now, its 2am in melbourne...

$stmt = $stmts[0];
$methodStmt = $stmt->stmts[1]->stmts[1];

$this->assertEquals('SELF', (string)$methodStmt->stmts[0]->class);
Copy link

Choose a reason for hiding this comment

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

The test suggests, that result is always in uppercase. However code change lowercases it.

I think test should check, that regardless of case in PHP file the result is lowercase.

@nikic
Copy link
Owner

nikic commented Aug 6, 2014

Good catch, I'll merge this in about a week, when I'm back home.

Did not look into why this is.
Looks like the original tree already lowercases static::
@TimeToogo
Copy link
Contributor Author

Finally got the test to pass.
Did not realise the parser will automatically lowercase static:: calls.
Is there any reason for this obscurity?

@aik099
Copy link

aik099 commented Aug 7, 2014

I thought, that all 3 are lowercased. I that parser can do some normalization, since writing keywords in uppercase doesn't seem like a good idea to me (even if it works). Same as calling IsSet when function name is isset.

@TimeToogo
Copy link
Contributor Author

@aik099 As far as I can tell only static:: was lowercased, the others are left as user defined as the test illustrates.

@aik099
Copy link

aik099 commented Aug 7, 2014

Maybe you should introduce some random lowercased letter, e.g. SeLF to clearly see if it's really preserved case or just force uppercase.

@TimeToogo
Copy link
Contributor Author

@aik099 I just confirmed locally that for parent::and self:: the casing is fully preserved.

@nikic
Copy link
Owner

nikic commented Aug 11, 2014

Merged via c034005. The case of "static" is now also preserved, see ef121e6.

@nikic nikic closed this Aug 11, 2014
@staabm
Copy link
Contributor

staabm commented Aug 18, 2014

just for reference: since php 5.5. self::,parent:: and static:: are case insensitive
php.net/manual/en/migration55.incompatible.php#migration55.incompatible.self-parent-static

@aik099
Copy link

aik099 commented Aug 18, 2014

Ah, so these ones were case-sensitive before PHP 5.5?

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.

7 participants