[RFC Discussion] "var" Deprecation

Colin O'Dell

10 years ago

My apologies, I posted the wrong discussion link.  Here's the correct one:
http://markmail.org/message/wn3ykdwgplfctho7

Colin

On Thu, Mar 10, 2016 at 12:14 PM Colin O'Dell <colinodell@gmail.com> wrote:

Lester Caine

10 years ago

On 10/03/16 17:16, Colin O'Dell wrote:

> - Ensuring that all major arguments for & against have been documented.

While the no-brain conversion of one to the other may be acceptable to
some, it's both the follow though on associated documentation and a
proper review of the code which is CURRENTLY the reason that the code
has not yet been converted. I have no problem with code needing to be
updated, it's the unnecessary pressure that it has to be done now when
there is no gain currently from spending that time.

There are other areas of code review that would be more productive, and
new libraries replacing the older code may be a more productive way
forward than having to address code that is currently doing the job.

--
Lester Caine - G8HFL
-----------------------------
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

Rowan Collins

10 years ago

On 10/03/2016 20:19, Lester Caine wrote:

> I have no problem with code needing to be
> updated, it's the unnecessary pressure that it has to be done now when
> there is no gain currently from spending that time.

To be clear, the pressure added by this deprecation would be to do it
"sometime in the next few years", which is quite a mild pressure.

But in general, I agree with your sentiment that there is effort
involved that could be spent elsewhere.

Regards,

--
Rowan Collins
[IMSoP]

Rowan Collins

10 years ago

Colin O'Dell wrote on 10/03/2016 17:14:

> Hello all,
>
> I have completed my initial draft of the RFC to deprecate "var" in favor of
> "public": https://wiki.php.net/rfc/var_deprecation
>

Hi Colin,

Thanks for a very thorough RFC, particularly in clearly expressing the
arguments for and against, based on the pre-draft discussion.

I remain not-quite-convinced that the benefits outweigh the costs, but
don't feel very strongly; I'm kind of "-0.5" on this (and don't have an
actual vote anyway).

Regards,

--
Rowan Collins
[IMSoP]

Stas Malyshev

10 years ago

Hi!

> I have completed my initial draft of the RFC to deprecate "var" in favor of
> "public": https://wiki.php.net/rfc/var_deprecation

As I already stated here, I think this is needless change which only
creates BC problems and does not add anything to the language. The RFC
incorrectly claims "var" is "legacy functionality", in fact it is not -
the functionality, namely public properties, are still very much in use,
it's just different keyword to invoke it. I still see absolutely no
value in claimed "clean up", which achieves no usable goal (minimizing
number of constructs in the language has never been considered a worthy
goal in PHP).

--
Stas Malyshev
smalyshev@gmail.com

Yasuo Ohgaki

10 years ago

Hi!

On Fri, Mar 11, 2016 at 2:14 AM, Colin O'Dell <colinodell@gmail.com> wrote:

> I have completed my initial draft of the RFC to deprecate "var" in favor of
> "public": https://wiki.php.net/rfc/var_deprecation
>
> I would greatly appreciate any feedback on this RFC, especially with the
> following:
>
> - Ensuring that all major arguments for & against have been documented.
> - Any impact this may have on SAPIs, extensions, or the opcache.
> - Any assistance or guidance on creating the patch (I've never written C or
> worked with the core codebase before).
>
> If you'd like to read the pre-draft discussion it can be found here:
> https://wiki.php.net/rfc/mailing_list_discussion

I'm 0 for this RFC, but providing upgrading script
https://gist.github.com/colinodell/5fb5e5d474674f294a38
is good approach for this kind of changes!
It works perfectly.

Good luck!

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Jakub Kubíček

10 years ago

From the RFC:

> var is therefore a limited subset of public.

No, this is wrong. `var` was here much much longer than `public`, therefore
`var` is not a subset of public, but rather `public` is _an extension_ of
`var`. The keyword `var` is also unique for it's semantics in the later
versions of the language viz. previous discussion:

- `var` represents not any certain visibility, it states that the
visibility of property is yet unspecified
- `var` can semantically also represent an internal dependency, as I have
described earlier

On 10 March 2016 at 22:44, Colin O'Dell <colinodell@gmail.com> wrote:

> Hello all,
>
> I have completed my initial draft of the RFC to deprecate "var" in favor of
> "public": https://wiki.php.net/rfc/var_deprecation
>
> I would greatly appreciate any feedback on this RFC, especially with the
> following:
>
> - Ensuring that all major arguments for & against have been documented.
> - Any impact this may have on SAPIs, extensions, or the opcache.
> - Any assistance or guidance on creating the patch (I've never written C or
> worked with the core codebase before).
>
> If you'd like to read the pre-draft discussion it can be found here:
> https://wiki.php.net/rfc/mailing_list_discussion
>
> Regards,
>
> Colin O'Dell
>

--
Cheers,
Kubis

James Titcumb

10 years ago

On 11 March 2016 at 10:03, Jakub Kubíček <kelerest123@gmail.com> wrote:

>
> - `var` can semantically also represent an internal dependency, as I have
> described earlier

An internal property should be `private`. The use of `var` in the specific
project you referenced earlier should only be considered a very, very weak
argument for keeping `var`. If it was a de-facto standard in many
projects/frameworks/libraries, then maybe it would have a stronger case,
but I have never seen `var` used in this way before - it seems like Nette
has "invented" a new semantic meaning for their use case.

Arvids Godjuks

10 years ago

2016-03-11 12:03 GMT+02:00 Jakub Kubíček <kelerest123@gmail.com>:

> From the RFC:
>
>
> > var is therefore a limited subset of public.
>
>
> No, this is wrong. `var` was here much much longer than `public`, therefore
> `var` is not a subset of public, but rather `public` is _an extension_ of
> `var`. The keyword `var` is also unique for it's semantics in the later
> versions of the language viz. previous discussion:
>
> - `var` represents not any certain visibility, it states that the
> visibility of property is yet unspecified
> - `var` can semantically also represent an internal dependency, as I have
> described earlier
>
> On 10 March 2016 at 22:44, Colin O'Dell <colinodell@gmail.com> wrote:
>
> > Hello all,
> >
> > I have completed my initial draft of the RFC to deprecate "var" in favor
> of
> > "public": https://wiki.php.net/rfc/var_deprecation
> >
> > I would greatly appreciate any feedback on this RFC, especially with the
> > following:
> >
> > - Ensuring that all major arguments for & against have been documented.
> > - Any impact this may have on SAPIs, extensions, or the opcache.
> > - Any assistance or guidance on creating the patch (I've never written C
> or
> > worked with the core codebase before).
> >
> > If you'd like to read the pre-draft discussion it can be found here:
> > https://wiki.php.net/rfc/mailing_list_discussion
> >
> > Regards,
> >
> > Colin O'Dell
> >
>
>
>
> --
> Cheers,
> Kubis
>

I'm gonna fire anyone who does anything like you described from my company
straight out the door no questions asked. Period.

var $_id; => is that intended to be "private"? But I can access as a
public. Or is it meant to be "protected"? Can I actually modify it directly
or is there a setter/getter? IDE gives me autocomplete from outside of the
object, then I surely can do $object->_id = $something;
var $id; => Well, all of the above.
Internal dependency? Good god, that's what private/protected are for.
You really overestimate how much people really care about the 3rd party
code and how it's intended to be used. I know of instances that people just
took 3rd party code, modified some property to be public and didn't give a
damn about that. And you want to define internal properties as "var
$property" to be available for public access?
If some project is using "var" as a special snowflake, well, it's their
problem, not the communities. And our PHP community is so large, that even
projects like Symfony aren't really dominating the landscape, and those
guys do have influence.

Sorry, but that's just bad decision making.
Also, there are docs, official docs here
http://php.net/manual/en/language.oop5.visibility.php that state:

"Class properties must be defined as public, private, or protected. If
declared using var, the property will be defined as public."

Sorry, but "var" is a legacy at this point. No sane code is written using
it, you don't even have a dedicated manual page or paragraph for it, just a
single line stating that "var is essentially public". And replacing var
with public is already automated for you with a script provided with the
RFC.

I'm sorry for my ramblings, but I just couldn't leave statements like that
without opposition.

Rowan Collins

10 years ago

Jakub Kubíček wrote on 11/03/2016 10:03:

>  From the RFC:
>
>
>> var is therefore a limited subset of public.
>
> No, this is wrong. `var` was here much much longer than `public`, therefore
> `var` is not a subset of public, but rather `public` is _an extension_ of
> `var`.

This is like arguing based on the origin of a word, rather than its
current meaning. Regardless of which came first, within the language as
it is today, "var" can be used in certain places as a synonym of
"public". That is how the engine sees it, and always has been - private
and protected visibility didn't exist in PHP 4, so you could write a
patched version of PHP 4 that recognised "public" as a synonym of "var",
and it would behave exactly as expected.

>   The keyword `var` is also unique for it's semantics in the later
> versions of the language viz. previous discussion:
>
> - `var` represents not any certain visibility, it states that the
> visibility of property is yet unspecified

This is not really semantic, but pragmatic - seeing "var" is a smell
that the code hasn't been updated to properly restrict visibility. It is
a disadvantage of running an automated conversion script, but it is not
a disadvantage of deprecation as such (which would just make the smell
stronger).

In fact, you could make the conversion script add a comment to each
variable it changed, and get a stronger smell that way as well. e.g.
"var $something;" -> "public $something; // TODO: property may not need
to be public"

> - `var` can semantically also represent an internal dependency, as I have
> described earlier

As others have said, this is a very odd hack added in one particular
code base, as a workaround for lack of package visibility / friend
methods / etc. Since it's just a convention, it could equally be done
other ways, such as @internal docblock annotations or a prefix on
property names - both of which would actually be more visible in IDEs.

I would put this in the same class as an overheating spacebar
[https://xkcd.com/1172/] - it's not the language's responsibility to
support every workflow that exploits unintended functionality.

Regards,

--
Rowan Collins
[IMSoP]

Thomas Punt

10 years ago

Hi!

> Hello all,
>
> I have completed my initial draft of the RFC to deprecate "var" in favor of
> "public": https://wiki.php.net/rfc/var_deprecation
>
> I would greatly appreciate any feedback on this RFC, especially with the
> following:
>
> - Ensuring that all major arguments for & against have been documented.
> - Any impact this may have on SAPIs, extensions, or the opcache.
> - Any assistance or guidance on creating the patch (I've never written C or
> worked with the core codebase before).

I'm still on the fence about this, but here's a patch for it:
https://github.com/php/php-src/compare/master...tpunt:deprecate-var

Thanks,
Tom

Colin O'Dell

10 years ago

>
> I'm still on the fence about this, but here's a patch for it:
> https://github.com/php/php-src/compare/master...tpunt:deprecate-var

Thank you so much Thomas!  I have added your patch to the RFC.  I really
appreciate you taking the time to implement this.

Regards,

Colin

Patrick ALLAERT

10 years ago

Le jeu. 10 mars 2016 à 18:15, Colin O'Dell <colinodell@gmail.com> a écrit :

> Hello all,
>
> I have completed my initial draft of the RFC to deprecate "var" in favor of
> "public": https://wiki.php.net/rfc/var_deprecation
>
> I would greatly appreciate any feedback on this RFC, especially with the
> following:
>
> - Ensuring that all major arguments for & against have been documented.
> - Any impact this may have on SAPIs, extensions, or the opcache.
> - Any assistance or guidance on creating the patch (I've never written C or
> worked with the core codebase before).
>
> If you'd like to read the pre-draft discussion it can be found here:
> https://wiki.php.net/rfc/mailing_list_discussion
>
> Regards,
>
> Colin O'Dell
>

Hello,

Related to what's mentioned in the RFC under: "public Is Not The Same", one
can use "var" to declare a property without specifying its visibility and
this is true for method as well: "function" is equivalent to "public
function".
Forcing people to specify a visibility for properties and not for methods
would add yet another inconsistency in the language.

Besides that, there isn't 2 ways for declaring a property, but (at least) 3:

#1
class Example {
var $foo;
}

#2
class Example {
public $foo;
}

#3
class Example {
public function __construct() {
$this->foo = null;
}
}

If I understand this RFC, example #1 is handled by it, but not #3.
Using #1 would emit a deprecation notice, but #3 wouldn't?
As far as I am concerned, I think it is still better to declare the
properties rather than not and this RFC would (somehow) favour not
declaring them at all than doing so with "var".

I'm generally in favour of not having duplicated ways of doing the exact
same thing.
I'm generally in favour of not introducing (uneeded) BC issues.
I'm not convinced that the number of people that will benefit the
uniqueness of declaring a variable would be greater than the one suffering
upgrade issues.
For that reason and the ones above I'm -1 about this RFC, even if I am not
satisfied with the fact we can still use "var".

Regards,
Patrick

Colin O'Dell

10 years ago

>
> Forcing people to specify a visibility for properties and not for methods
> would add yet another inconsistency in the language.
>

That's a really good point.  However, I think we currently have a different
inconsistency: declaring functions without explicit visibility is possible
without needing an extra (and redundant) keyword.

The inconsistency you mention could easily be solved by a future RFC.  We
could:

1. Require visibility for class methods.
2. Allow declaring properties without using any keywords.
3. Use some other approach?

I don't think we necessarily need to choose an option at this time (or
perhaps ever).

> #1
> class Example {
>     var $foo;
> }
>
> #2
> class Example {
>     public $foo;
> }
>
> #3
> class Example {
>     public function __construct() {
>         $this->foo = null;
>     }
> }
>
> If I understand this RFC, example #1 is handled by it, but not #3.
> Using #1 would emit a deprecation notice, but #3 wouldn't?
>

Correct.  This RFC only targets the deprecation and removal of T_VAR.

> As far as I am concerned, I think it is still better to declare the
> properties rather than not and this RFC would (somehow) favour not
> declaring them at all than doing so with "var".
>

I disagree - this RFC does not prevent anyone from declaring properties,
you just need to explicitly declare the visibility.

> I'm generally in favour of not having duplicated ways of doing the exact
> same thing.
> I'm generally in favour of not introducing (uneeded) BC issues.
> I'm not convinced that the number of people that will benefit the
> uniqueness of declaring a variable would be greater than the one suffering
> upgrade issues.
>

I disagree on the third point - I think the automated upgrade process is
literally the second-easiest upgrade possible (#1 would be no changes
required).  Nevertheless, I truly appreciate you considering and weighing
the different aspects of this proposal.  I'm hoping others will take a
similar approach in deciding whether they support this RFC.

Thanks for your feedback!

Colin

James Titcumb

10 years ago

On 14 Mar 2016 6:14 p.m., "Colin O'Dell" <colinodell@gmail.com> wrote:

>
> >
> > Forcing people to specify a visibility for properties and not for

methods

> > would add yet another inconsistency in the language.
> >
>
> That's a really good point.

Yup, agree with this. In my opinion, I'd like to see two birds with one
stone, but a separate RFC is acceptable I suppose. The only risk is one
change getting voted in and not the other, leaving an even worse
inconsistency IMO. I think this RFC should be all or nothing: require
visibility for both methods and properties, or do not require anything.

Fleshgrinder

10 years ago

On 3/14/2016 7:26 PM, James Titcumb wrote:

> Yup, agree with this. In my opinion, I'd like to see two birds with one
> stone, but a separate RFC is acceptable I suppose. The only risk is one
> change getting voted in and not the other, leaving an even worse
> inconsistency IMO. I think this RFC should be all or nothing: require
> visibility for both methods and properties, or do not require anything.
>

+1, but ...

On 3/14/2016 6:34 PM, Patrick ALLAERT wrote:

> Besides that, there isn't 2 ways for declaring a property, but (at

least) 3:

>
> #1
> class Example {
>     var $foo;
> }
>
> #2
> class Example {
>     public $foo;
> }
>
> #3
> class Example {
>     public function __construct() {
>         $this->foo = null;
>     }
> }
>
> If I understand this RFC, example #1 is handled by it, but not #3.
> Using #1 would emit a deprecation notice, but #3 wouldn't?
> As far as I am concerned, I think it is still better to declare the
> properties rather than not and this RFC would (somehow) favour not
> declaring them at all than doing so with "var".
>

... #3 is a different language feature of PHP: dynamic assignment of
properties to objects. Userland code does not and should not care if the
internals code creates these dynamic properties with *var* or *public*
or *very-special-internals-visibility-modifier*. ;)

--
Richard "Fleshgrinder" Fussenegger

Björn Larsson

10 years ago

Den 2016-03-14 kl. 19:14, skrev Colin O'Dell:

>> Forcing people to specify a visibility for properties and not for methods
>> would add yet another inconsistency in the language.
>>
> That's a really good point.  However, I think we currently have a different
> inconsistency: declaring functions without explicit visibility is possible
> without needing an extra (and redundant) keyword.
>
> The inconsistency you mention could easily be solved by a future RFC.  We
> could:
>
>    1. Require visibility for class methods.
>    2. Allow declaring properties without using any keywords.
>    3. Use some other approach?
>
> I don't think we necessarily need to choose an option at this time (or
> perhaps ever).
>

Interesting :)  Think option 1 implies a BC break. Would it also affects
interface
declarations making public keyword mandatory? Personally I like option
2, but
feels a bit beyond the scope of this RFC.

Regards //Björn