Re: Cleaning up perl code

Michael Paquier <michael@paquier.xyz> writes:

> On Fri, May 24, 2024 at 02:09:49PM +0900, Michael Paquier wrote:
>> For now, I have staged for commit the attached, that handles most of
>> the changes from Alexander (msvc could go for more cleanup?).
>
> This one has been applied as of 0c1aca461481 now that v18 is
> open.
>
>> I'll look at the changes from Dagfinn after that, including if perlcritic
>> could be changed.  I'll handle the first part when v18 opens up, as
>> that's cosmetic.

For clarity, I've rebased my addional unused-variable changes (except
the errcodes-related ones, see below) onto current master, and split it
into separate commits with detailed explaiations for each file file, see
attached.

> I'm still biased about the second set of changes proposed here,
> though.  ProhibitUnusedVariables would have benefits when writing perl
> code in terms of clarity because we would avoid useless stuff, but it
> seems to me that we should put more efforts into the unification of
> the errcodes parsing paths first to have a cleaner long-term picture.
>
> That's not directly the fault of this proposal that we have the same
> parsing rules spread across three PL languages, so perhaps what's
> proposed is fine as-is, at the end.

It turns out there are a couple more places that parse errcodes.txt,
namely doc/src/sgml/generate-errcodes-table.pl and
src/backend/utils/generate-errcodes.pl.  I'll have a go refactoring all
of these into a common function à la Catalog::ParseHeader() that returns
a data structure these scripts can use as as appropriate.

> Any thoughts or comments from others more familiar with
> ProhibitUnusedVariables?

Relatedly, I also had a look at prohibiting unused regex captures
(RegularExpressions::ProhibitUnusedCapture), which found a few real
cases, but also lots of false positives in Catalog.pm, because it
doesn't understand that %+ uses all named captures, so I won't propose a
patch for that until that's fixed upstream
(https://github.com/Perl-Critic/Perl-Critic/pull/1065).

- ilmari