Perl-Critic / PPI

54 stars 44 forks source link

Use of uninitialized value $word in pattern match (m//) at /usr/share/perl5/vendor_perl/PPI/Tokenizer.pm line 836. #206

Open epa opened 7 years ago

epa commented 7 years ago

Use of uninitialized value $word in pattern match (m//) at /usr/share/perl5/vendor_perl/PPI/Tokenizer.pm line 836.

This is from running perlcritic; if you're unable to reproduce this I can whittle it down to a minimal input file for perlcritic, but I hope the warning message is enough to go on.

chriscapaci commented 7 years ago

I also see this. Somewhere the function __current_token_is_forced_word is getting called without a second parameter causing line 836 to fail.

Like @epa I also see it running perlcritic.

It should be a very simple and trivial fix that will unblock users of perlcritic.

wchristian commented 7 years ago

I've released a temporary fix of this with #208 to CPAN to unblock perlcritic users.

It does however need further research to figure out what is happening. @epa, could you provide a minimized test case of code that triggers this? It'll be easier for me to reason about than trying to reverse engineer triggering code.

epa commented 7 years ago

A minimal input to perlcritic is OK? Or do you need a purely PPI-based test program?

wchristian commented 7 years ago

@epa Input to perlcritic is fine. :)

epa commented 7 years ago
% echo 'sub x {' | perlcritic --brutal -
Use of uninitialized value $word in pattern match (m//) at /usr/share/perl5/vendor_perl/PPI/Tokenizer.pm line 836.
...
% perlcritic --version
1.126
% perl -MPPI -E 'say $PPI::VERSION'
1.224
% perl -v

This is perl 5, version 22, subversion 2 (v5.22.2) built for x86_64-linux-thread-multi
(with 33 registered patches, see perl -V for more detail)
...
wchristian commented 7 years ago

Excellent, that should be enough to make a test. :)

epa commented 7 years ago

You can reproduce it?

wchristian commented 7 years ago

@epa Turns out i actually was not.

Mithaldu@DigitizedSqueak ~$ echo 'sub x {' | perlcritic --brutal -
Error: No word lists can be found for the language "en_US".
perltidy had errors!! at line 1, column 1.  See page 33 of PBP.  (Severity: 1)
Module does not end with "1;" at line 1, column 1.  Must end with a recognizable true value.  (Severity: 4)
Code not contained in explicit package at line 1, column 1.  Violates encapsulation.  (Severity: 4)
No package-scoped "$VERSION" variable found at line 1, column 1.  See page 404 of PBP.  (Severity: 2)
Subroutine name is a homonym for builtin keyword x at line 1, column 1.  See page 177 of PBP.  (Severity: 4)
Code before strictures are enabled at line 1, column 1.  See page 429 of PBP.  (Severity: 5)
Code before warnings are enabled at line 1, column 1.  See page 431 of PBP.  (Severity: 4)

Mithaldu@DigitizedSqueak ~$ perlcritic --version
1.126

Mithaldu@DigitizedSqueak ~$ perl -MPPI -E 'say $PPI::VERSION'
1.224

Mithaldu@DigitizedSqueak ~$ perl -v

This is perl 5, version 22, subversion 3 (v5.22.3) built for cygwin-thread-multi-64int

Copyright 1987-2017, Larry Wall

Perl may be copied only under the terms of either the Artistic License or the
GNU General Public License, which may be found in the Perl 5 source kit.

Complete documentation for Perl, including FAQ lists, should be found on
this system using "man perl" or "perldoc perl".  If you have access to the
Internet, point your browser at http://www.perl.org/, the Perl Home Page.

Mithaldu@DigitizedSqueak ~$

I had also tried with 5.18.4, under Strawberry Perl, and with the segment in a file. Not sure what's going on. Can you dig deeper?

I also just released https://metacpan.org/release/MITHALDU/PPI-1.232 which has a final fix to the warning that doesn't break all the tests. ;)

epa commented 7 years ago

Here's the test case I have so far:

use Perl::Critic::Utils;
use Module::Pluggable;
if ( not @site_policy_names ) {
    import Module::Pluggable(search_path => $POLICY_NAMESPACE, require => 1);
    plugins();
}
my $s = "sub x {";
new PPI::Document(\$s);

I'm having a hard time reducing this further; replacing Perl::Critic::Utils or Module::Pluggable with the program text of those modules stops the warning being triggered, even if I import them afterwards.

wchristian commented 7 years ago

I've been able to kind of reproduce the message:

D:\>perl -we "use PPI::Document;PPI::Document->new( \'sub x {' );"
Use of uninitialized value $word in pattern match (m//) at C:/strawberry/perl/site/lib/PPI/Tokenizer.pm line 836.

D:\>

The problem is that Tokenizer.pm does not use warnings. And something in P::C or M::P is causing not only warnings to be enabled for that part, but if it's actually a crash, to also fatalize those warnings.

There's two things here worthy of doing:

  1. Investigate the two modules and see how they may be activating the warnings. I suspect that M::P can actually be removed entirely and only P::C::U is the culprit.
  2. Make all PPI modules compliant with both strictures and warnings, which i should be doing anyhow, and then cross-compare against all downstream dependencies.
wchristian commented 7 years ago

After further research, according to #142 warnings in PPI are disabled on purpose. This worked fine for a while because EUMM ran tests with -w by default. However this blew up because EUMM changed that, meaning the test suite ran without warnings everywhere.

I'm working on a patch to at least for now, reinstate the -w behavior via the test pragmas.

In future i will be going with option 2 and adding warnings first and eventually fatal warnings to all modules natively to prevent situations like this, i.e. PPI code being released that dies if fatalized from the outside, in the future.

If you still feel like investigating how it is being fatalized, i would be happy to hear that, but otherwise i'm also fine with you closing this ticket if you don't have further objections.

Thanks a lot for helping me with the fact-finding. :)

epa commented 7 years ago

Actually I didn't realize it was a fatal error, I thought it was just warning and continuing.

wchristian commented 7 years ago

@epa Well @chriscapaci mentioned it blocking P::C users, so i assumed it had become fatalized. Maybe double-check?

epa commented 7 years ago

I just mean I didn't notice.

wchristian commented 7 years ago

So you were able to confirm it's a fatal error on your end? :)