Please stop writing poor questionable Perl code

[koitsu]

...
Running sa-learn --spam over SSH...
Possible precedence issue with control flow operator at /usr/local/lib/perl5/site_perl/IO/Socket/SSL.pm line 2599.
...
$ pkg which /usr/local/lib/perl5/site_perl/IO/Socket/SSL.pm
/usr/local/lib/perl5/site_perl/IO/Socket/SSL.pm was installed by package p5-IO-Socket-SSL-2.087
$ perl --version
This is perl 5, version 36, subversion 3 (v5.36.3) built for amd64-freebsd-thread-multi
$ vim /usr/local/lib/perl5/site_perl/IO/Socket/SSL.pm

2594                 Net::SSLeay::CTX_set_psk_server_callback($ctx, sub {
2595                     my ($ssl,$identity,$psklen) = @_;
2596                     if (ref($psk) eq 'HASH') {
2597                         return $psk->{$identity} || $psk->{''} or return
2598                             IO::Socket::SSL->_internal_error(
2599                             "no PSK for given identity '$identity' and no default");
2600                     } else {
2601                         return $psk;
2602                     }
2603                 });

What is being complained about is lines 2597-2599. This code is still present in 2.088 / master branch: https://github.com/noxxi/p5-io-socket-ssl/blob/595e4c2169e628571dde45d1ca18a00e4a3df260/lib/IO/Socket/SSL.pm#L2594-L2603

Offending commits:

• https://github.com/noxxi/p5-io-socket-ssl/commit/387c02b621e690ba5f2bf5ee16a90a65db9e5c0a
• https://github.com/noxxi/p5-io-socket-ssl/commit/80315ed1a6a0b7968ce26284ed81b56c0ab288d9

Perl is complaining for a good reason. I myself am a Perl programmer going back to 1993. Please stop writing Perl code like this. Please write things that do not conflate operator precedence (|| vs. or -- they are not the same!) along with multiple return statements. Write it cleanly. Use parenthesis to make it clear. Even I cannot determine what exactly the intention is here.

[koitsu]

I'm closing this ticket because the goal here is solely to chastise for writing code in this fashion (and not testing the code path; had you, you would've seen the warning in question). 2.088 seems to have changed this mechanism, but it is clear that the 2.087 version is questionable at best. It is my experience that unless open-source contributors are called out for things, they will often repeat the same mistakes in the future, leading to worse situations.

Pro-tip, take it or leave it: do not get in the habit of writing complex one-liners in Perl, especially ones which offer no real performance benefit. If you are already in such a habit, break yourself of it. Parentheses and braces will not hurt you.

I appreciate your efforts maintaining this critical module, but please treat it as such. Thank you.

[mauke]

and not testing the code path; had you, you would've seen the warning in question

That's not true. The warning is emitted by the compiler; the code doesn't have to be executed. The reason you might not see this warning normally is that IO::Socket::SSL does not use warnings (which is in itself a questionable practice). The reason the warning appears with cpan is that the latter manually sets $^W = 1, which is the old (pre-5.6) mechanism to enable warnings, and it is global (affects all code that doesn't use warnings, including modules).

Even I cannot determine what exactly the intention is here.

Seems fairly straightforward if you know what || does.