search

amphp / process

An async process dispatcher for Amp.
MIT License
235 stars 30 forks source link

RFC Check for constant existence before declaring #38

Closed dantleech closed 5 years ago

dantleech commented 5 years ago

I use an tool which requires the composer autoloader.php in order to use the class map.

When this tool is used with the library it issues a warning as these constants are then redefined.

This PR checks for the existence of the constants before defining them.

kelunik commented 5 years ago

This is probably a bug in that tool then and should be fixed there.

dantleech commented 5 years ago

I'm not sure it can be classed as a bug - the tool needs the ClassLoader from composer for a given project, and this is the only easy way to facilitate that, so it's by design.

An extension for the tool also uses amphp/process, so when including for the project it fails with the constant re-declaration error.

Other tools also include the autoloader additionally (e.g. Phpstan, WorseReflection), but don't use amphp/process so it's not an issue.

For sure, it's not a nice thing to do in general (importing somebody elses autoloader) but I'm not sure there's a better way currently.

kelunik commented 5 years ago

Why are only the constants a problem, but not the function definitions? They'd be declared twice, too.

dantleech commented 5 years ago

hmm, good point, this PR did fix the issue however, so that's confusing.

I can't reproduce now even with the extension, closing. Will re-open if it occurs again and I can further isolate the issue.

dfelton commented 5 years ago

@dantleech The two constants in question are defined within the scope of a namespace. Unless the other application is defining code within that same namespace, there shouldn't be any collision of constants here.

Basic CLI example (with XDebug on) of referencing a constant inside a namespace, as well as failing to specify the namespace of the constant while outside the namespace:

 $ php -r "namespace Foo; const BAR = 'FooBar'; namespace FizzBuzz; var_dump(\Foo\BAR); var_dump(BAR);"
string(6) "FooBar"
PHP Warning:  Use of undefined constant BAR - assumed 'BAR' (this will throw an Error in a future version of PHP) in Command line code on line 1
PHP Stack trace:
PHP   1. {main}() Command line code:0

Warning: Use of undefined constant BAR - assumed 'BAR' (this will throw an Error in a future version of PHP) in Command line code on line 1

Call Stack:
    0.0040     391280   1. {main}() Command line code:0

string(3) "BAR"