Perl / perl5

đŸȘ The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 559 forks source link

Warn or disallow some odd uses of => #14441

Closed p5pRT closed 9 years ago

p5pRT commented 9 years ago

Migrated from rt.perl.org#123664 (status was 'rejected')

Searchable as RT123664$

p5pRT commented 9 years ago

From @epa

Created by @epa

The => operator is a special comma which quotes its LHS. As a convenience Perl allows a trailing comma in a list​:

  (1\, 2\,)

That is useful\, but it may make less sense to allow

  (1\, 2 =>)

Since => is normally used to indicate the programmer's intention of a key=>value pair\, as well as for its quoting behaviour\, allowing a trailing => loses the opportunity to check for certain mistakes.

Similarly I suggest that it would be a useful check to forbid having a list item sandwiched between two => operators in the program text\, as

  (1 => 2 => 3)

Finally\, while there may be reasons for skipping over repeated commas in a list (though I do not know what the reasons are)\, this should not apply to the fat comma​:

  (1 => => 2)

All of these checks would help to catch some mistakes such as cut- and-paste errors at compile time rather than run time. I accept that some of the odd constructs above may be intentionally used in existing code. In particular (a => b => c) may be used as an attempt by the programmer to show the flow of data\, or 'wish-fulfilment operator' as Conway calls it. But I think the other two checks should be feasible.

Perl Info ``` Flags: category=core severity=wishlist Site configuration information for perl 5.18.4: Configured by Red Hat, Inc. at Thu Oct 30 14:55:21 UTC 2014. Summary of my perl5 (revision 5 version 18 subversion 4) configuration: Platform: osname=linux, osvers=3.16.3-200.fc20.x86_64, archname=x86_64-linux-thread-multi uname='linux buildvm-18.phx2.fedoraproject.org 3.16.3-200.fc20.x86_64 #1 smp wed sep 17 22:34:21 utc 2014 x86_64 x86_64 x86_64 gnulinux ' config_args='-des -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Dccdlflags=-Wl,--enable-new-dtags -Dlddlflags=-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Wl,-z,relro -Dshrpdir=/usr/lib64 -DDEBUGGING=-g -Dversion=5.18.4 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivlib=/usr/share/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Darchlib=/usr/lib64/perl5 -Dvendorarch=/usr/lib64/perl5/vendor_perl -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Dusedtrace=/usr/bin/dtrace -Duselargefiles -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin -Dusesitecustomize' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic', cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.8.3 20140911 (Red Hat 4.8.3-7)', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='gcc', ldflags =' -fstack-protector' libpth=/usr/local/lib64 /lib64 /usr/lib64 libs=-lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat perllibs=-lresolv -lnsl -ldl -lm -lcrypt -lutil -lpthread -lc libc=, so=so, useshrplib=true, libperl=libperl.so gnulibc_version='2.18' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,--enable-new-dtags' cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Wl,-z,relro ' Locally applied patches: Fedora Patch1: Removes date check, Fedora/RHEL specific Fedora Patch3: support for libdir64 Fedora Patch4: use libresolv instead of libbind Fedora Patch5: USE_MM_LD_RUN_PATH Fedora Patch6: Skip hostname tests, due to builders not being network capable Fedora Patch7: Dont run one io test due to random builder failures Fedora Patch9: Fix find2perl to translate ? glob properly (RT#113054) Fedora Patch10: Update h2ph(1) documentation (RT#117647) Fedora Patch11: Update pod2html(1) documentation (RT#117623) Fedora Patch12: Disable ornaments on perl5db AutoTrace tests (RT#118817) Fedora Patch14: Do not use system Term::ReadLine::Gnu in tests (RT#118821) Fedora Patch15: Define SONAME for libperl.so Fedora Patch16: Install libperl.so to -Dshrpdir value Fedora Patch18: Fix crash with \\&$glob_copy (RT#119051) Fedora Patch19: Fix coreamp.t rand test (RT#118237) Fedora Patch20: Reap child in case where exception has been thrown (RT#114722) Fedora Patch21: Fix using regular expressions containing multiple code blocks (RT#117917) Fedora Patch22: Create site paths by cpan for the first time (CPAN RT#99905) Fedora Patch200: Link XS modules to libperl.so with EU::CBuilder on Linux Fedora Patch201: Link XS modules to libperl.so with EU::MM on Linux @INC for perl 5.18.4: /home/eda/lib/perl5/ /usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 . Environment for perl 5.18.4: HOME=/home/eda LANG=en_GB.UTF-8 LANGUAGE (unset) LC_COLLATE=C LC_CTYPE=en_GB.UTF-8 LC_MESSAGES=en_GB.UTF-8 LC_MONETARY=en_GB.UTF-8 LC_NUMERIC=en_GB.UTF-8 LC_TIME=en_GB.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/eda/bin:/home/eda/bin:/usr/local/bin:/usr/bin:/sbin:/usr/sbin:/sbin:/usr/sbin PERL5LIB=/home/eda/lib/perl5/ PERL_BADLANG (unset) SHELL=/bin/bash ______________________________________________________________________ This email has been scanned by the Symantec Email Security.cloud service. For more information please visit http://www.symanteccloud.com ______________________________________________________________________ ```
p5pRT commented 9 years ago

From @jkeenan

On Fri Jan 23 23​:54​:22 2015\, eda@​waniasset.com wrote​:

This is a bug report for perl from eda@​waniasset.com\, generated with the help of perlbug 1.39 running under perl 5.18.4.

----------------------------------------------------------------- [Please describe your issue here]

The => operator is a special comma which quotes its LHS. As a convenience Perl allows a trailing comma in a list​:

(1\, 2\,)

That is useful\, but it may make less sense to allow

(1\, 2 =>)

Since => is normally used to indicate the programmer's intention of a key=>value pair\, as well as for its quoting behaviour\, allowing a trailing => loses the opportunity to check for certain mistakes.

You have too narrow an interpretation of programmers' intentions for their use of the '=>' operator -- and you have not cited any "mistakes" for which one should check.

It took me only a few minutes using http​://grep.cpan.me (dg++) to find a legitimate example of '=> )'. There are three unit tests in CPAN distro File-Find-Rule (starting at https://metacpan.org/source/RCLAMP/File-Find-Rule-0.33/t/File-Find-Rule.t#L222) which use 'discard =>)' as part of testing that distribution's procedural interface (File​::Find​::Rule​::Procedural).

Similarly I suggest that it would be a useful check to forbid having a list item sandwiched between two => operators in the program text\, as

(1 => 2 => 3)

https://metacpan.org/source/RCLAMP/File-Find-Rule-0.33/t/File-Find-Rule.t#L167 provides a perfectly legitimate use of this syntax.

Finally\, while there may be reasons for skipping over repeated commas in a list (though I do not know what the reasons are)\, this should not apply to the fat comma​:

(1 => => 2)

Why\, other than it looks a bit inelegant?

More to the point\, why should we impose this restriction *in the Perl 5 core*? If\, for instance\, you wanted to rule out this syntax as part of your "shop style"\, you could do so with a Perl-Critic rule. https://metacpan.org/source/KRYDE/Perl-Critic-Pulp-89/lib/Perl/Critic/Policy/ValuesAndExpressions/ProhibitEmptyCommas.pm#L1 appears to do this.

All of these checks would help to catch some mistakes such as cut- and-paste errors at compile time rather than run time. I accept that some of the odd constructs above may be intentionally used in existing code. In particular (a => b => c) may be used as an attempt by the programmer to show the flow of data\, or 'wish-fulfilment operator' as Conway calls it. But I think the other two checks should be feasible.

I don't see any benefits from implementing this feature request that outweigh (a) the effort it would take to implement this in core; (b) the grief that new and frivolous warnings would cause in existing CPAN (and\, I suspect\, production) code.

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 9 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 9 years ago

From @ap

* Ed Avis \perlbug\-followup@​perl\.org [2015-01-24 08​:55]​:

(1\, 2 =>)

-1\, done that on purpose on a handful of occasions.

(1 => 2 => 3)

-1\, done that on purpose a lot. (-37 really\, if that made a difference.)

(1 => => 2)

-0. I wouldn’t like finding this in code\, and I guess if it did warn at compile time (without an error) I wouldn’t mind that it did. But I don’t see the pay-off for adding a warning.

This is a job for a linter.

p5pRT commented 9 years ago

From @epa

Thanks for your reply. I guess => really is used just as a quoting mechanism sometimes\, and not just as a way of creating k=>v pairs.

While => => is useless\, it is also harmless (like \,\,). While I don't think anyone would advocate adding it to the language if it didn't already exist\, it may not be worth the effort to remove it either. The only real argument in favour might be that it'd make it possible to assign some useful semantics to => => later\, or perhaps to remove an ambiguity in the grammar at some future date.

-- Ed Avis \eda@​waniasset\.com

p5pRT commented 9 years ago

From @abigail

On Sat\, Jan 24\, 2015 at 08​:26​:03PM +0000\, Ed Avis wrote​:

Thanks for your reply. I guess => really is used just as a quoting mechanism sometimes\, and not just as a way of creating k=>v pairs.

While => => is useless\, it is also harmless (like \,\,). While I don't think anyone would advocate adding it to the language if it didn't already exist\, it may not be worth the effort to remove it either. The only real argument in favour might be that it'd make it possible to assign some useful semantics to => => later\, or perhaps to remove an ambiguity in the grammar at some future date.

"=> =>" may be useless\, "\, =>" (or "\,=>") is not.

I use it often when I want the visual impact of =>\, without the auto-quoting​:

  use constant APPLE => 1;   use constant BANANA => 2;

  my %fruit_colours = (   APPLE \,=> "red"\,   BANANA \,=> "yellow"\,   );

Abigail

p5pRT commented 9 years ago

From @cpansprout

On Sat Jan 24 15​:19​:48 2015\, abigail@​abigail.be wrote​:

On Sat\, Jan 24\, 2015 at 08​:26​:03PM +0000\, Ed Avis wrote​:

Thanks for your reply. I guess => really is used just as a quoting mechanism sometimes\, and not just as a way of creating k=>v pairs.

While => => is useless\, it is also harmless (like \,\,). While I don't think anyone would advocate adding it to the language if it didn't already exist\, it may not be worth the effort to remove it either. The only real argument in favour might be that it'd make it possible to assign some useful semantics to => => later\, or perhaps to remove an ambiguity in the grammar at some future date.

"=> =>" may be useless\, "\, =>" (or "\,=>") is not.

I use it often when I want the visual impact of =>\, without the auto-quoting​:

use constant APPLE => 1; use constant BANANA => 2;

my %fruit_colours = ( APPLE \,=> "red"\, BANANA \,=> "yellow"\, );

I sometimes use =>=> to set things off with more emphasis.

--

Father Chrysostomos

p5pRT commented 9 years ago

From @epa

Thanks all for your replies. I think this can be closed - unless the p5-porters think it worthwhile to explicitly document that \,\, is equivalent to \, and =>=> equivalent to =>?

p5pRT commented 9 years ago

From @avar

On Sun\, Jan 25\, 2015 at 8​:11 PM\, Ed Avis via RT \perlbug\-followup@​perl\.org wrote​:

Thanks all for your replies. I think this can be closed - unless the p5-porters think it worthwhile to explicitly document that \,\, is equivalent to \, and =>=> equivalent to =>?

From perlop​: "The => operator is a synonym for the comma except that it causes its left operand to be interpreted as a string[...]".

So that establishes that it's like "\,"\, do we document anywhere that repeated "\," (or ";" for that matter) fold down to one?

p5pRT commented 9 years ago

From @jkeenan

On Sun Jan 25 13​:14​:43 2015\, avarab@​gmail.com wrote​:

On Sun\, Jan 25\, 2015 at 8​:11 PM\, Ed Avis via RT \perlbug\-followup@​perl\.org wrote​:

Thanks all for your replies. I think this can be closed - unless the p5-porters think it worthwhile to explicitly document that \,\, is equivalent to \, and =>=> equivalent to =>?

From perlop​: "The => operator is a synonym for the comma except that it causes its left operand to be interpreted as a string[...]".

So that establishes that it's like "\,"\, do we document anywhere that repeated "\," (or ";" for that matter) fold down to one?

That might be useful -- but let's get a patch in a new RT for discussion so that we can close this ticket.

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 9 years ago

From @cpansprout

On Sun Jan 25 13​:17​:13 2015\, jkeenan wrote​:

On Sun Jan 25 13​:14​:43 2015\, avarab@​gmail.com wrote​:

On Sun\, Jan 25\, 2015 at 8​:11 PM\, Ed Avis via RT \perlbug\-followup@​perl\.org wrote​:

Thanks all for your replies. I think this can be closed - unless the p5-porters think it worthwhile to explicitly document that \,\, is equivalent to \, and =>=> equivalent to =>?

From perlop​: "The => operator is a synonym for the comma except that it causes its left operand to be interpreted as a string[...]".

So that establishes that it's like "\,"\, do we document anywhere that repeated "\," (or ";" for that matter) fold down to one?

That might be useful -- but let's get a patch in a new RT for discussion so that we can close this ticket.

perldata already has a paragraph on this. Search for 1\,\,\,3.

--

Father Chrysostomos

p5pRT commented 9 years ago

From @jkeenan

On Sun Jan 25 17​:16​:34 2015\, sprout wrote​:

On Sun Jan 25 13​:17​:13 2015\, jkeenan wrote​:

On Sun Jan 25 13​:14​:43 2015\, avarab@​gmail.com wrote​:

On Sun\, Jan 25\, 2015 at 8​:11 PM\, Ed Avis via RT \perlbug\-followup@​perl\.org wrote​:

Thanks all for your replies. I think this can be closed - unless the p5-porters think it worthwhile to explicitly document that \,\, is equivalent to \, and =>=> equivalent to =>?

From perlop​: "The => operator is a synonym for the comma except that it causes its left operand to be interpreted as a string[...]".

So that establishes that it's like "\,"\, do we document anywhere that repeated "\," (or ";" for that matter) fold down to one?

That might be useful -- but let's get a patch in a new RT for discussion so that we can close this ticket.

perldata already has a paragraph on this. Search for 1\,\,\,3.

On that basis\, I think we can close this ticket.

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 9 years ago

@jkeenan - Status changed from 'open' to 'rejected'