Access and Visibility Modifiers

php.internals

Fleshgrinder

10 years ago
On 3/19/2016 12:51 PM, Fleshgrinder wrote:
> Moving this to a new thread in order to not spam the other. > > On 3/19/2016 11:48 AM, Fleshgrinder wrote: >> On 3/19/2016 11:38 AM, Björn Larsson wrote: >>> Den 2016-03-18 kl. 21:12, skrev Fleshgrinder: >>>> No worries you are not, not at all. I just wanted to thwart you and >>>> others in directly assigning ... >>>> >>>> final class A { >>>> int $x; >>>> } >>>> >>>> ... to be *public* and obstruct the opportunity of assigning it a >>>> meaningful new functionality. >>>> >>> Well, but if one should assign int $x without visibility keyword a >>> meaning, shouldn't it be the same as for function y() {} without >>> keyword, i.e. public? >>> >>> Of course one can change both, but that sounds like a 8.0 topic. >>> >>> Regards //Björn >>> >>> >> >> It is a sad state the implicit public properties use *var* and implicit >> public methods nothing, this makes the introduction of new visibility >> modifiers terribly complicated. You are completely right, they should be >> the same and *var* should be banned. It is consistent and that is very >> important. I cannot and do not want to argue against this. Yet at the >> same time the lack of access (and additional visibility) modifiers is >> what I am missing the most. Seems as if this has to wait for 8.0 (or >> 9.0, depending on the resistance). :( >> > > I just thought about this some more and this is actually not true. > > namespace Fleshgrinder\Examples\PhpInternals; > > final class A { > int $x; > var $y; > $z; > > function getX() { > return $this->x; > } > } > > Everything is public here, as we already agreed upon and changing that > would be a topic for 8.0 or 9.0. The *var* keyword should be banned imho > but I do not know about the *$z* in the example. Is this even possible > with the parser? Otherwise we could keep the *var* keyword because with > my proposal here it would become useful again. > > assembly fleshgrinder/experiments; > > namespace Fleshgrinder\Examples\PhpInternals; > > final class A { > int $x; > > function getX() { > return $this->x; > } > } > > Where *assembly* could also be *module*, *package*, ... and it allows > one to group multiple classes and namespaces together across composer > packages. Furthermore it alters the access and visibility of members. In > the above example where no visibility modifiers are specified both the > class /A/ and the method /getX/ are only accessible within the > *fleshgrinder/experiments* assembly. This approach would allow the > introduction of such a system in a feature release (7.x.0) without > breaking changes. Either the new keyword is present and missing > visibility modifiers are globally scoped (hence implicit public) due to > everything being in the global assembly by default or the new keyword is > present and missing visibility modifiers are restricting access to the > assembly. > > Usage of the word /package/ could be a blessing and a curse at the same > time because the word is already heavily in use in the PHP world, e.g. > /@package/ in DocBlocks or in /composer package/. > > The same holds true for /module/ because it is sometimes used to refer > to PHP extensions and PEAR uses it. > > Hence, *assembly* might be the best choice here and is my proposal. > > The following proposal in regards to access and visibility is not well > thought through but hopefully a starting point: > > # Methods > private := limited to the current class > protected := limited to the current and child classes > := limited to the current assembly > public := global > > # Properties > private := limited to the current class > protected := limited to the current and child classes > := limited to the current assembly > public := global > var := E_STRICT or limited to the current assembly > > # Classes, Interfaces, and Traits > private := limited to the current namespace > protected := limited to the current and child namespaces > := limited to the current assembly > public := global > > Default assembly is always global and old code would not change at all. :) >
Reposting to make sure that this is a new thread.
-- Richard "Fleshgrinder" Fussenegger

Fleshgrinder

10 years ago
On 3/19/2016 12:51 PM, Fleshgrinder wrote:
> I just thought about this some more and this is actually not true. > > namespace Fleshgrinder\Examples\PhpInternals; > > final class A { > int $x; > var $y; > $z; > > function getX() { > return $this->x; > } > } > > Everything is public here, as we already agreed upon and changing that > would be a topic for 8.0 or 9.0. The *var* keyword should be banned imho > but I do not know about the *$z* in the example. Is this even possible > with the parser? Otherwise we could keep the *var* keyword because with > my proposal here it would become useful again. > > assembly fleshgrinder/experiments; > > namespace Fleshgrinder\Examples\PhpInternals; > > final class A { > int $x; > > function getX() { > return $this->x; > } > } > > Where *assembly* could also be *module*, *package*, ... and it allows > one to group multiple classes and namespaces together across composer > packages. Furthermore it alters the access and visibility of members. In > the above example where no visibility modifiers are specified both the > class /A/ and the method /getX/ are only accessible within the > *fleshgrinder/experiments* assembly. This approach would allow the > introduction of such a system in a feature release (7.x.0) without > breaking changes. Either the new keyword is present and missing > visibility modifiers are globally scoped (hence implicit public) due to > everything being in the global assembly by default or the new keyword is > present and missing visibility modifiers are restricting access to the > assembly. > > Usage of the word /package/ could be a blessing and a curse at the same > time because the word is already heavily in use in the PHP world, e.g. > /@package/ in DocBlocks or in /composer package/. > > The same holds true for /module/ because it is sometimes used to refer > to PHP extensions and PEAR uses it. > > Hence, *assembly* might be the best choice here and is my proposal. > > The following proposal in regards to access and visibility is not well > thought through but hopefully a starting point: > > # Methods > private := limited to the current class > protected := limited to the current and child classes > := limited to the current assembly > public := global > > # Properties > private := limited to the current class > protected := limited to the current and child classes > := limited to the current assembly > public := global > var := E_STRICT or limited to the current assembly > > # Classes, Interfaces, and Traits > private := limited to the current namespace > protected := limited to the current and child namespaces > := limited to the current assembly > public := global > > Default assembly is always global and old code would not change at all. :) >
I would like to change above definition for properties since the *var* deprecation was voted negative: # Properties private := limited to the current class protected := limited to the current and child classes var := limited to the current assembly public := global And here is a list of additional keywords I collected that could be used instead of *assembly* (which I do not like that much): - group - crate (used by Rust) - unit - segment - kit - container - bin - ... I was also thinking about prescribing some parsable format for the content of this new keyword. I do not see a real value in doing so since such an identifier does not bare any meaning other than grouping namespaces and classes together, hence, we could simply use a string, e.g.: assembly 'WhatEver!'; On the other side, not specifying anything parsable might result in misuse in the community. An alternative would be to reuse the namespace syntax. The advantage would be that users do not have to learn a new syntax. A disadvantage is that is looks very similar to namespaces. A third option is to use the composer package naming conventions. Yes we would be pulling in something from a userland tool here but we would actually just say that composer did something good here and bless it. The syntax would be: assembly <vendor>/<name>; /[a-z](?:[_-]?[a-z0-9])*\/[a-z](?:[_-]?[a-z0-9])*/Di This syntax would be extremely strict compared to the possibilities for other identifiers in PHP and the PCRE might need to be extended if this is an option you find worth pursuing. Would love to here some general feedback on the idea.
-- Richard "Fleshgrinder" Fussenegger

Rowan Collins

10 years ago
Fleshgrinder wrote on 07/04/2016 18:35:
> I would like to change above definition for properties since the*var* > deprecation was voted negative: > > # Properties > private := limited to the current class > protected := limited to the current and child classes > var := limited to the current assembly > public := global
I'm not convinced of the value of reusing the keyword in this way. To me, there is nothing in the term "var" that means "slightly less than public", and if we want to have package/assembly/whatever visibility, we should reserve a new keyword for that use. Meanwhile, it might be worth checking the list archives for previous discussions of package visibility; I know it has come up a couple of times recently, in various forms, so it would be good not to repeat the same points again and again. Regards,
-- Rowan Collins [IMSoP]

Guilherme Blanco

10 years ago
Hi, I was working on a private-class support for PHP an year ago, until I hit a problem I couldn't fix myself. Now I have some more expertise of what I could do to resolve it, but I still didn't start on rebase/update of patch yet. However, I'd like to describe my line of thinking here, so people can debate about it even without me working on an updated patch yet. It's a preliminary work that needs to happen before people spend time on a patch, that will later be transformed into an RFC. The situation that brought me to work on this is, as a library developer, people reuse internal classes that are not meant to be reused outside of library's code. Changes to its API can potentially break consumers of the library, making upgrades hard to happen. To mitigate this problem, I thought about working on two patches to the language that could help library/application developers, self-containing code that is not meant to be reused. They're detailed as follow: 1- Private-class visibility: Only classes in the same namespace (siblings) or child ones could instantiate the class. Cloning would still be possible outside of namespace's scope, the same to access public members. This is what is currently 90% done in: https://github.com/php/php-src/pull/947 It requires changes to Zend Engine to include a start namespace and an end namespace opcodes, allowing to locate in which scope you are. There are failing tests that are not passing because of this right now. 2- Protected-class visibility: It would work the same way as private-class visibility, but with the difference that other classes in the same namespace/child namespaces could access protected members of the specified protected class. Of course both of them would require to go through RFC writing, idea validation, voting and performance assessment process, but it's already a start. Regards, On Thu, Apr 7, 2016 at 2:05 PM, Rowan Collins <rowan.collins@gmail.com> wrote:
> Fleshgrinder wrote on 07/04/2016 18:35: > >> I would like to change above definition for properties since the*var* >> deprecation was voted negative: >> >> # Properties >> private := limited to the current class >> protected := limited to the current and child classes >> var := limited to the current assembly >> public := global >> > > I'm not convinced of the value of reusing the keyword in this way. To me, > there is nothing in the term "var" that means "slightly less than public", > and if we want to have package/assembly/whatever visibility, we should > reserve a new keyword for that use. > > Meanwhile, it might be worth checking the list archives for previous > discussions of package visibility; I know it has come up a couple of times > recently, in various forms, so it would be good not to repeat the same > points again and again. > > Regards, > -- > Rowan Collins > [IMSoP] >
-- Guilherme Blanco Lead Architect at E-Block

Fleshgrinder

10 years ago
On 4/7/2016 8:32 PM, guilhermeblanco@gmail.com wrote:
> Hi, > > I was working on a private-class support for PHP an year ago, until I hit a > problem I couldn't fix myself. >
Hi, I know and I commented on the PR on GitHub some time ago. I kept the semantics of private classes exactly as you have implemented it in there. On 4/7/2016 8:32 PM, guilhermeblanco@gmail.com wrote:
> The situation that brought me to work on this is, as a library developer, > people reuse internal classes that are not meant to be reused outside of > library's code. Changes to its API can potentially break consumers of the > library, making upgrades hard to happen. >
That is exactly my motivation as well. On 4/7/2016 8:32 PM, guilhermeblanco@gmail.com wrote:
> To mitigate this problem, I thought about working on two patches to the > language that could help library/application developers, self-containing > code that is not meant to be reused. They're detailed as follow: > > 1- Private-class visibility: Only classes in the same namespace (siblings) > or child ones could instantiate the class. Cloning would still be possible > outside of namespace's scope, the same to access public members. > This is what is currently 90% done in: > https://github.com/php/php-src/pull/947 > It requires changes to Zend Engine to include a start namespace and an end > namespace opcodes, allowing to locate in which scope you are. There are > failing tests that are not passing because of this right now. > > 2- Protected-class visibility: It would work the same way as private-class > visibility, but with the difference that other classes in the same > namespace/child namespaces could access protected members of the specified > protected class. > > Of course both of them would require to go through RFC writing, idea > validation, voting and performance assessment process, but it's already a > start. >
All fine with the private class approach and as mentioned I kept it equal. However, the protected class approach you describe has a huge drawback: it does not apply to parent namespaces. This makes it extremely hard to organize the classes nicely, e.g. (symfony/process): namespace Process\Pipes { abstract private class AbstractPipes {} final protected class UnixPipes extends AbstractPipes {} final protected class WinPipes extends AbstractPipes {} } namespace Process { final public class Process {} } Where the process instantiates the pipes as needed. Another problem is that suddenly every class in the current and child namespaces have the ability to access protected properties. However, you might not want that because protected properties are for true childs only. Hence, we need an additional modifier that currently does not exist in order to have full control over the usage of our internals. This is what I tried to achieve with my proposal, cover the logical side of things so that we can retrofit it to PHP without any kind of BC.
-- Richard "Fleshgrinder" Fussenegger

Fleshgrinder

10 years ago
On 4/7/2016 8:05 PM, Rowan Collins wrote:
> Fleshgrinder wrote on 07/04/2016 18:35: >> I would like to change above definition for properties since the*var* >> deprecation was voted negative: >> >> # Properties >> private := limited to the current class >> protected := limited to the current and child classes >> var := limited to the current assembly >> public := global > > I'm not convinced of the value of reusing the keyword in this way. To > me, there is nothing in the term "var" that means "slightly less than > public", and if we want to have package/assembly/whatever visibility, we > should reserve a new keyword for that use. >
The original definition was # Properties private := limited to the current class protected := limited to the current and child classes := limited to the current assembly public := global In other words: same as Java without any modifier. I only changed the proposal because var is to be kept and the current semantic of it (implicit public) but not being truly equal motivated me to change the proposal and to give it a new meaning. Note that the meaning of *var* does not change for old classes that are not using the proposed access/visibility keyword because they are in the global namespace and thus implicitly public. Hence, the special *var* case would finally get a real meaning and would not be an alias anymore. On 4/7/2016 8:05 PM, Rowan Collins wrote:
> Meanwhile, it might be worth checking the list archives for previous > discussions of package visibility; I know it has come up a couple of > times recently, in various forms, so it would be good not to repeat the > same points again and again. >
I check what I can dig up. :)
-- Richard "Fleshgrinder" Fussenegger

Fleshgrinder

10 years ago
On 4/7/2016 8:43 PM, Fleshgrinder wrote:
> In other words: same as Java without any modifier. I only changed the > proposal because var is to be kept and the current semantic of it > (implicit public) but not being truly equal motivated me to change the > proposal and to give it a new meaning. >
That is hard to decipher, let me rephrase: The vote to deprecate and drop *var* was negative: https://wiki.php.net/rfc/var_deprecation This means that we are going to keep it with its current semantic: implicit public. However, *var* is currently not truly the same as *public* as outlined in the RFC above. My proposal would fix that problem.
-- Richard "Fleshgrinder" Fussenegger

Fleshgrinder

10 years ago
On 4/7/2016 8:43 PM, Fleshgrinder wrote:
> On 4/7/2016 8:05 PM, Rowan Collins wrote: >> Meanwhile, it might be worth checking the list archives for previous >> discussions of package visibility; I know it has come up a couple of >> times recently, in various forms, so it would be good not to repeat the >> same points again and again. >> > > I check what I can dig up. :) >
Here we go: # 2007-12: Namespace access modifiers http://marc.info/?t=119750415000002 # 2009-09: New access modifier http://marc.info/?l=php-internals&m=125341032112627 Some nice keyword suggestions in this one: - group - trusted - party # 2011-03: Class Access Modifiers http://marc.info/?t=129917302700001 # 2013-02: I would like to write an RFC for the addition of an internal keyword http://marc.info/?t=136194893000003 # 2014-12: Package private class http://marc.info/?t=141929489300003 # 2015-11: Internal methods proposal http://marc.info/?t=144836744000001 I am sure that I overlooked something since the search turned up a lot of stuff. However, the above list already illustrates that such a feature is often requested and useful. This is further illustrated by other languages providing similar functionality. There were various questions raised in the threads above that I would like and try to answer here.
> Can child classes redeclare visibility or override it.
Yes, every class in an inheritance change can open up the visibility as is already the case for methods in PHP. The reason for that is simple: namespace Process\Pipes { public interface Pipes {} abstract private class AbstractPipes implements Pipes {} protected final class UnixPipes extends AbstractPipes {} protected final class WinPipes extends AbstractPipes {} } The UnixPipes and WinPipes classes are useless if they cannot open up the visibility because they cannot be instantiated from nowhere.
> It would still be possible to get an instance of a private class from > the outside, e.g.: > > namespace Internals { > private class Foo {} > public class Bar {} > public function getFoo() { > return new Foo(); > } > } > }
Yes and this is intentional. The class visibility modifiers only restrict the creation of such objects and not the interaction with them at runtime. Otherwise it would be impossible to implement a public interface in a private class that is being returned to the client. public interface Entities extends \IteratorAggregate {} public interface EntityManager extends Entities {} private class InMemoryEntities implements EntityManager {} private class FilesystemEntities implements EntityManager {} private class MySqlEntities implements EntityManager {} private class PdoEntities implements EntityManager {} switch (ENV) { case PROD: $entities = new MySqlEntities; break; case TEST: $entities = new InMemoryEntities; break; } $dic->addService('entities', $entities, [ 'alias' => 'entitye_manager', ]); This is not /good code/ or anything but should illustrate the usefulness of it. In other words: It supports the I in SOLID. :) https://en.wikipedia.org/wiki/SOLID_%28object-oriented_design%29
-- Richard "Fleshgrinder" Fussenegger