Allow catching as ParseError "Non-abstract method C::foo() must contain a body"

[thg2k]

Description

Since PHP 7 it is possible to catch syntax errors in PHP files during inclusion, which is a very nice feature, such as:

try {
  eval("class C { function foo() }");
}
catch (\Throwable $e) {
  print "CAUGHT [" . get_class($e) . "]: " . $e->getMessage() . "\n";
}
// prints: CAUGHT [ParseError]: syntax error, unexpected token "}", expecting ";" or "{"
// execution continues

Unfortunately, the same is not possible in case the error is a non-abstract method without body:

try {
  eval("class C { function foo(); }");
}
catch (\Throwable $e) {
  print "CAUGHT [" . get_class($e) . "]: " . $e->getMessage() . "\n";
}
// prints: Fatal error: Non-abstract method C::foo() must contain body in /in/YY0Wu(13) : eval()'d code on line 1
// execution terminates

Would it be possible to implement a throw also for this case?

https://3v4l.org/YY0Wu

[thg2k]

Other examples:

// (1) CATCHABLE - ParseError - syntax error, unexpected token ";"
// no classes defined
eval("class D {}; err(;) class C {};");

// (2) CATCHABLE - Error - Class "Z" not found
// class D is defined, class C not defined
eval("class D {}; class C extends Z {};");

// (3) UNCATCHABLE - Cannot declare class C, because the name is already in use
eval("class C {}; class C {};");

// (4) UNCATCHABLE - Non-abstract method C::m() must contain body
eval("class C { function m(); }");

// (5) UNCATCHABLE - Declaration of D::m(string $a) must be compatible with C::m(int $a)
eval('class C { function m(int $a) {} }; class D extends C { function m(string $a) {} };');

[iluuu1994]

This is not implemented as a parse error (it's valid according to the grammar) but as a compiler error. The problem is that, the way PHP is designed, by the time this error is triggered various things have already been declared, such as the class and even the method itself.

With a naive implementation (just changing the fatal error to a parse error):

<?php

try {
    eval("class C { function foo(); }");
} catch (\ParseError $e) {}

echo (string) new ReflectionClass(C::class);

var_dump((new C())->foo());

Class [ <user> class C ] {
  @@ /home/ilutov/Developer/php-src/Zend/tests/gh9333.php(4) : eval()'d code 1-1

  - Constants [0] {
  }

  - Static properties [0] {
  }

  - Static methods [0] {
  }

  - Properties [0] {
  }

  - Methods [1] {
    Method [ <user> public method foo ] {
      @@ /home/ilutov/Developer/php-src/Zend/tests/gh9333.php(4) : eval()'d code 1 - 1
    }
  }
}
NULL

I'm not sure this can't trivially be solved without some kind of pre-pass that validates classes/methods before declaring them. At this point you also could not re-run the eval with valid code as the class has already been declared.

[mvorisek]

This is not implemented as a parse error (it's valid according to the grammar) but as a compiler error. The problem is that, the way PHP is designed, by the time this error is triggered various things have already been declared, such as the class and even the method itself.

I'm not sure this can't trivially be solved without some kind of pre-pass that validates classes/methods before declaring them.

for the start, it would be fine if some things like a persistent string would remain allocated, of course, things like function or class must be freed completely (or never registered)

At this point you also could not re-run the eval with valid code as the class has already been declared.

this would require native unregister/replace class support, I would limit this issue for "catching parse errors only"

[iluuu1994]

this would require native unregister/replace class support, I would limit this issue for "catching parse errors only"

This is already the case. All parse errors (as in errors triggered in Bison) are parse errors. The errors mentioned in this issue are all semantic errors.

[iluuu1994]

I had another look and created this PoC: #10997 It does come with a caveat, so I'm not sure if it's a good idea.