You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
By default, global symbols are marked as exposed. The reason being this allows to easily make any code shipping with polyfills work out of the box. For example:
<?phpnamespaceRandomPrefix {
str_starts_with(...); // correctly resolves to `\RandomPrefix\str_starts_with`
}
namespaceRandomPrefix\N1 {
// Problematic case!str_starts_with(...); // resolves to `\str_starts_with` since `\RandomPrefix\N1\str_starts_with` does not exist
}
namespaceRandomPrefix\N2 {
usefunctionRandomPrefix\str_starts_with;
str_starts_with(...);
}
Currently PHP-Scoper records anytime it comes across a global symbol to expose it. But as demonstrated above, it is only necessary in some scenarios.
Solution
It would be better for PHP-Scoper to track more specific cases that it comes across. For example here, we only need to expose str_starts_with if an ambiguous call is found, it is otherwise unnecessary.
Further considerations:
recordedFunctions is currently a bit misleading and with this feature should be renamed. Indeed we want to record the function declarations.
for each recorded function declaration, if the function is exposed, an alias should be created (this is what is currently done but requires some changes as the existing recordedFunctions is re-purposed.
for each ambiguous function called found:
if no global declaration is found, there is nothing to do *1
if a global declaration is found:
if the symbol is exposed, an alias is created for the global symbol (currently the case) *2 (exposed case)
if the symbol is not exposed, only the necessary aliases should be made *2 (not exposed case)
*1 ambiguous calls found but no global declaration
<?phpnamespace {
foo();
}
namespaceApp {
foo();
bar();
}
// scoped version:namespacePrefix {
foo();
}
namespacePrefix\App {
foo();
bar();
}
// no `\foo()` or `\bar()` function declaration found// during the scoping, either those are internal symbols// or there is a namespaced declaration.// No PHP-Scoper alias needed.
Recorded function declarations: []
Ambiguous function calls:
foo: can be either \foo or App\foo
bar: can be either \bar or App\bar
Calculating aliases: no function declaration => no alias.
*2 ambiguous calls found with function declarations
<?phpnamespace {
functionfoo() {};
foo();
}
namespaceApp {
functionbar() {};
foo();
bar();
}
// scoped version:namespacePrefix {
functionfoo() {};
foo();
}
namespacePrefix\App {
functionbar() {};
foo();
bar();
}
// scoper-autoload.php// if exposed:function_alias('foo', 'Prefix\foo');
function_alias('App\bar', 'App\Prefix\foo'); // <- not necessary for it to work, purely because it is exposed// if not exposed:function_alias('Prefix\App\foo', 'Prefix\foo');
Recorded function declarations:
\foo
\App\bar
Ambiguous function calls:
foo: can be either \foo or App\foo
bar: can be either \bar or App\bar
(another case): bar: can be either \bar or Ppa\bar
Calculating aliases:
for foo, \foo was declared, so if foo is exposed, alias [foo, Prefix\foo].
Otherwise, if needs to be aliases for each case:
[Prefix\foo, Prefix\App\foo]
for bar, App\bar was declared, so if bar is exposed, alias [App\bar, Prefix\App\bar]
Otherwise, if needs to be aliases for each case:
[Prefix\App\bar, Prefix\Ppa\bar]
*3 reviewing the case of polyfills (no usage found)
In this example str_starts_with is marked as internal.
<?phpnamespace {
if (!function_exists('str_starts_with')) {
functionstr_starts_with() {}
}
}
// scoped version:namespacePrefix {
if (!function_exists('str_starts_with') && !function_exists('Prefix\str_starts_with')) {
functionstr_starts_with() {}
}
}
// scoper-autoload.php// if exposed:function_alias('str_starts_with', 'Prefix\str_starts_with');
// if not exposed:// no alias
In this case where no usage is found and the str_starts_with and if not exposed no additional noise is added.
*3 reviewing the case of polyfills (usages found)
In this example str_starts_with is marked as internal.
<?phpnamespace {
if (!function_exists('str_starts_with')) {
functionstr_starts_with() {}
}
}
namespaceAmb {
str_starts_with();
}
namespaceFQ {
\str_starts_with();
}
// scoped version:namespacePrefix {
if (!function_exists('str_starts_with') && !function_exists('Prefix\str_starts_with')) {
functionstr_starts_with() {}
}
}
namespacePrefix\Amb {
str_starts_with();
}
namespacePrefix\FQ {
\str_starts_with();
}
// scoper-autoload.php// if exposed:function_alias('str_starts_with', 'Prefix\str_starts_with');
// if not exposed:function_alias('Prefix\Amb\str_starts_with', 'Prefix\str_starts_with');
// breaks the case of Prefix\FQ.
To implement the solution
This requires the following information:
recording the function declarations and if that symbol is exposed
recording the ambiguous function calls with what they can resolve too
there is two passes to know the aliases that need to be created:
checking the function declarations, to expose the functions that are configured as exposed
checking the ambiguous function calls, which define the necessary aliases
As shown in the last case with the non-ambiguous call to a polyfilled symbol inPrefix\FQ, not exposing the global constants would break. Unfortunately the only viable solution would be to require a second pass knowing in advance the polyfilled symbols. This is not possible as this requires not a second traverse of a file, but a second traverse of the codebase. Maybe possible but much slower and probably the API will be more awkward especially for Box.
Maybe another solution is to treat expose global functions differently, and expose them only if a usage is found. Either that or add a separate setting for it.
The text was updated successfully, but these errors were encountered:
Checking this issue again, it may not be as big of a deal as I make it out to be. Currently there is a lot of exposed symbols due to polyfills shipped, but in a way it should be fine as a polyfill is there to provide a very specific, specified PHP symbol. As such they should all behave the same so it should not be a problem.
Description
By default, global symbols are marked as exposed. The reason being this allows to easily make any code shipping with polyfills work out of the box. For example:
Will be scoped into:
Notice however, that having
str_starts_with
exposed, would only be necessary if there is a call that is ambiguous:Will be scoped into:
Currently PHP-Scoper records anytime it comes across a global symbol to expose it. But as demonstrated above, it is only necessary in some scenarios.
Solution
It would be better for PHP-Scoper to track more specific cases that it comes across. For example here, we only need to expose
str_starts_with
if an ambiguous call is found, it is otherwise unnecessary.Further considerations:
recordedFunctions
is currently a bit misleading and with this feature should be renamed. Indeed we want to record the function declarations.recordedFunctions
is re-purposed.*1 ambiguous calls found but no global declaration
Recorded function declarations: []
Ambiguous function calls:
foo
: can be either\foo
orApp\foo
bar
: can be either\bar
orApp\bar
Calculating aliases: no function declaration => no alias.
*2 ambiguous calls found with function declarations
Recorded function declarations:
\foo
\App\bar
Ambiguous function calls:
foo
: can be either\foo
orApp\foo
bar
: can be either\bar
orApp\bar
bar
: can be either\bar
orPpa\bar
Calculating aliases:
foo
,\foo
was declared, so iffoo
is exposed, alias[foo, Prefix\foo]
.Otherwise, if needs to be aliases for each case:
[Prefix\foo, Prefix\App\foo]
bar
,App\bar
was declared, so ifbar
is exposed, alias[App\bar, Prefix\App\bar]
Otherwise, if needs to be aliases for each case:
[Prefix\App\bar, Prefix\Ppa\bar]
*3 reviewing the case of polyfills (no usage found)
In this example
str_starts_with
is marked as internal.In this case where no usage is found and the
str_starts_with
and if not exposed no additional noise is added.*3 reviewing the case of polyfills (usages found)
In this example
str_starts_with
is marked as internal.To implement the solution
This requires the following information:
As shown in the last case with the non-ambiguous call to a polyfilled symbol in
Prefix\FQ
, not exposing the global constants would break. Unfortunately the only viable solution would be to require a second pass knowing in advance the polyfilled symbols. This is not possible as this requires not a second traverse of a file, but a second traverse of the codebase. Maybe possible but much slower and probably the API will be more awkward especially for Box.Maybe another solution is to treat
expose global functions
differently, and expose them only if a usage is found. Either that or add a separate setting for it.The text was updated successfully, but these errors were encountered: