Skip to content
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

Use a visitor to wrap non-namespaced code in an empty namespace. #13

Closed
wants to merge 3 commits into from
Closed

Use a visitor to wrap non-namespaced code in an empty namespace. #13

wants to merge 3 commits into from

Conversation

rjkip
Copy link

@rjkip rjkip commented Jun 10, 2013

I found that some of the classes were outputted without a namespace, causing a PHP Fatal error: No code may exist outside of namespace {}. A quick fix was implemented in #7, but this ignores files with the character sequence namespace, regardless of its position. It may appear in a comment, for example.

I've implemented a node visitor that ensures all code is wrapped in an empty/root namespace. I didn't feel it was necessary to make this configurable, so I did not implement an additional command line option.

@hinikato
Copy link

The bug still exist, @mtdowling do you have any comments about it? I mean bug in the previous version without this pull request applied. I have not checked the pull request yet. So it would be good to fix it.

@hinikato
Copy link

For tests you can use the https://github.com/doctrine/doctrine1/blob/master/lib/Doctrine/Configurable.php class - it has the '$namespace ' string.

@hinikato
Copy link

The pull request works partially, try run it for Zend Framework 1, it creates separated blocks for each require_once, it is not a big problem but it becomes a problem when there are use statements in the file (to find a use case use other than ZF 1 files).

@rjkip
Copy link
Author

rjkip commented Jun 20, 2013

Hmm, I'll look into those separate blocks.

@hinikato
Copy link

I would like to clarify a situation. It seems like the error occurs for the following case: a file defines one or more classes without namespace (Iike in ZF 1) but uses 'use' statements, for example:

<?php
use My\Other\Class;

class Foo {}

@GrahamCampbell
Copy link
Member

Is this still an issue?

@GrahamCampbell
Copy link
Member

Ping @rjkip and @hinikato.

@rjkip
Copy link
Author

rjkip commented Nov 13, 2014

Woo, that's a long time ago. Will see if I have time in a couple of hours. I'll get back to you.

@GrahamCampbell
Copy link
Member

Ping @rjkip.

@GrahamCampbell
Copy link
Member

Fixed by #49.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants