Re: Make all Perl warnings fatal

On 2023-08-25 Fr 16:49, Dagfinn Ilmari Mannsåker wrote:
> Alvaro Herrera<alvherre@alvh.no-ip.org>  writes:
>
>> On 2023-Aug-10, Peter Eisentraut wrote:
>>
>>> I wanted to figure put if we can catch these more reliably, in the style of
>>> -Werror.  AFAICT, there is no way to automatically turn all warnings into
>>> fatal errors.  But there is a way to do it per script, by replacing
>>>
>>>      use warnings;
>>>
>>> by
>>>
>>>      use warnings FATAL => 'all';
>>>
>>> See attached patch to try it out.
>> BTW in case we do find that there's some unforeseen problem and we want
>> to roll back, it would be great to have a way to disable this without
>> having to edit every single Perl file again later.  However, I didn't
>> find a way to do it -- I thought about creating a separate PgWarnings.pm
>> file that would do the "use warnings FATAL => 'all'" dance and which
>> every other Perl file would use or include; but couldn't make it work.
>> Maybe some Perl expert knows a good answer to this.
> Like most pragmas (all-lower-case module names), `warnings` affects the
> currently-compiling lexical scope, so to have a module like PgWarnings
> inject it into the module that uses it, you'd call warnings->import in
> its import method (which gets called when the `use PgWarnings;``
> statement is compiled, e.g.:
>
> package PgWarnings;
>
> sub import {
> 	warnings->import(FATAL => 'all');
> }
>
> I wouldn't bother with a whole module just for that, but if we have a
> group of pragmas or modules we always want to enable/import and have the
> ability to change this set without having to edit all the files, it's
> quite common to have a ProjectName::Policy module that does that.  For
> example, to exclude warnings that are unsafe, pointless, or impossible
> to fatalise (c.f.https://metacpan.org/pod/strictures#CATEGORY-SELECTIONS):
>
> package PostgreSQL::Policy;
>
> sub import {
> 	strict->import;
> 	warnings->import(
> 		FATAL => 'all',
> 		NONFATAL => qw(exec internal malloc recursion),
> 	);
> 	warnings->uniport(qw(once));
> }
>
> Now that we require Perl 5.14, we might want to consider enabling its
> feature bundle as well, with:
>
> 	feature->import(':5.14')
>
> Although the only features of note that adds are:
>
>     - say: the `say` function, like `print` but appends a newline
>
>     - state: `state` variables, like `my` but only initialised the first
>       time the function they're in is called, and the value persists
>       between calls (like function-scoped `static` variables in C)
>
>     - unicode_strings: use unicode semantics for characters in the
>       128-255 range, regardless of internal representation


We'd probably have to modify the perlcritic rules to account for it. See 
<https://metacpan.org/pod/Perl::Critic::Policy::TestingAndDebugging::RequireUseStrict> 
and similarly for RequireUseWarnings. In any case, it seems a bit like 
overkill.


>> Maybe the BEGIN block of each file can `eval` a new PgWarnings.pm that
>> emits the "use warnings" line?
> That's ugly as sin, and thankfully not necessary.
>

Agreed.


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com