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

Doesn't support comments outside of namespaces declared with bracketed syntax #412

Closed
cjohansson opened this issue Aug 29, 2017 · 11 comments

Comments

@cjohansson
Copy link

cjohansson commented Aug 29, 2017

Comments outside of namespaces defined with brackets seems to crash the parser.

Steps to reproduce:

  1. Create PHPParser project like this
composer require nikic/php-parser
  1. Create file testClass.php like this
<?php

namespace A {
	class MyClassA {
		function myFunctionA() {
			echo "myEchoA";
		}
	}
}

namespace B {
	class MyClassB {
		function myFunctionB() {
			echo "myEchoB";
		}
	}
}

namespace {
	$a = new \A\MyClassA();
	$a->myFunctionA();
	$b = new \B\MyClassB();
	$b->myFunctionB();
	
}

// Test comment
  1. Create file test.php
<?php
require 'vendor/autoload.php';
use PhpParser\ParserFactory;
$parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7);
$code = file_get_contents(__DIR__ . '/testClass.php');
try {
    $stmts = $parser->parse($code);
    // $stmts is an array of statement nodes
} catch (Error $e) {
    echo 'Parse Error: ', $e->getMessage();
}

Expected result:
It works

Actual result:
It fails with error PHP Fatal error: Uncaught PhpParser\Error: No code may exist outside of namespace {} on line 19

@cjohansson
Copy link
Author

It seems I had a syntax error :(

@cjohansson cjohansson changed the title Doesn't support declaring multiple namespaces, bracketed syntax Doesn't support comments outside of namespaces declared with bracketed syntax Aug 29, 2017
@cjohansson cjohansson reopened this Aug 29, 2017
@cjohansson
Copy link
Author

Ok I was able to reproduce it now

@TomasVotruba
Copy link
Contributor

Hi, could you send failing test to be sure?

@cjohansson
Copy link
Author

cjohansson commented Aug 29, 2017

Sure here it is

Just PHP without PhpParser works:

php -f testClass.php 
myEchoAmyEchoB

With PhpParser, Fatal error:

php -f test.php 
PHP Fatal error:  Uncaught PhpParser\Error: No code may exist outside of namespace {} on line 28 in /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/ParserAbstract.php:449
Stack trace:
#0 /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/Parser/Php7.php(817): PhpParser\ParserAbstract->handleNamespaces(Array)
#1 /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/ParserAbstract.php(262): PhpParser\Parser\Php7->reduceRule1()
#2 /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/Parser/Multiple.php(50): PhpParser\ParserAbstract->parse('<?php\n\nnamespac...', Object(PhpParser\ErrorHandler\Throwing))
#3 /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/Parser/Multiple.php(31): PhpParser\Parser\Multiple->tryParse(Object(PhpParser\Parser\Php7), Object(PhpParser\ErrorHandler\Throwing), '<?php\n\nnamespac...')
#4 /Users/christianjohansson/Web/PHPParser/test.php(7): PhpParser\Parser\Multiple->parse('< in /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/ParserAbstract.php on line 449

Fatal error: Uncaught PhpParser\Error: No code may exist outside of namespace {} on line 28 in /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/ParserAbstract.php:449
Stack trace:
#0 /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/Parser/Php7.php(817): PhpParser\ParserAbstract->handleNamespaces(Array)
#1 /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/ParserAbstract.php(262): PhpParser\Parser\Php7->reduceRule1()
#2 /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/Parser/Multiple.php(50): PhpParser\ParserAbstract->parse('<?php\n\nnamespac...', Object(PhpParser\ErrorHandler\Throwing))
#3 /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/Parser/Multiple.php(31): PhpParser\Parser\Multiple->tryParse(Object(PhpParser\Parser\Php7), Object(PhpParser\ErrorHandler\Throwing), '<?php\n\nnamespac...')
#4 /Users/christianjohansson/Web/PHPParser/test.php(7): PhpParser\Parser\Multiple->parse('< in /Users/christianjohansson/Web/PHPParser/vendor/nikic/php-parser/lib/PhpParser/ParserAbstract.php on line 449

@TomasVotruba
Copy link
Contributor

I meant send PR with test to this project, so we can see build fail. It would be easier to fix then.

Something like this: Roave/BetterReflection#274

@cjohansson
Copy link
Author

Ok you mean create a unit test for it and do a pull request? All source code is in the first comment, it's not much

@cjohansson
Copy link
Author

Sure, I can do that later this week if you want

@TomasVotruba
Copy link
Contributor

It's better to have (edge) cases covered for future references of the same issues.
Sometimes there is regression bugs and these issues are resend, if not covered with tests.

Sure, I can do that later this week if you want

That would be great. Just wait for @nikic to confirm, if he needs that.

@nikic nikic closed this as completed in a10780c Aug 29, 2017
@TomasVotruba
Copy link
Contributor

Thanks @nikic

@nikic
Copy link
Owner

nikic commented Aug 29, 2017

That would be great. Just wait for @nikic to confirm, if he needs that.

What I mainly care about is having some reproduce code -- it's not so important whether it comes in test form or standalone :)

@TomasVotruba
Copy link
Contributor

@nikic I really appreciate your activity around this repo. It's very rare for me to see such active maintainer 🙇‍♂️

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

No branches or pull requests

3 participants