Perl-Critic / PPI

53 stars 44 forks source link

[Feature request] filename attribute to PPI::Document->new #180

Open van-de-bugger opened 8 years ago

van-de-bugger commented 8 years ago

Dist::Zilla::Role::PPI uses PPI in such a way:

my $encoded_content = $file->encoded_content;
....
my $document = PPI::Document->new(\$encoded_content);

Thus, $document does not know its filename. This makes good user-friendly error messages hardly possible.

For example, scan_for_prereqs method of any of Perl::PrereqScanner::XXX modules is called with 3 arguments: $self, $ppi_doc, and $req. Scanning the document, the method may operate with tokens and other elements. Every token has location property which contains line, column, charatcter, and filename. However, being called from Dist::Zilla, filename is always undefined.

To solve this problem, I propose PPI::Document constructor recognizes filename attribute, e. g.:

my $document = PPI::Document->new(\$encoded_content, filename => $file->name);

The problem could be fixed in Dist::Zilla by prepending file content with #line directive. However:

van-de-bugger commented 8 years ago

Err... Probably I do not understand something. I supposed that creating a PPI::Document from file name, e. g.:

my $d = PPI::Document->new( 'assa.pl' );

Creates a PPI document which known its own name, so location of an element can be retrieved with element's methods logical_line_number, logical_filename, and others. It seems my assumption was wrong:

$ cat assa.pl 
#!/usr/bin/perl
use strict;
use warnings;
use PPI;
my $d = PPI::Document->new( './assa.pl' );  # Second comment.
$d->index_locations;                        # Third comment.
my $c = $d->find( 'Token::Comment' ) || [];
for my $e ( @$c ) {
    printf( "%s at <%s> %d.\n", $e->content, $e->logical_filename, $e->logical_line_number );
};

$ ./assa.pl 
Use of uninitialized value in printf at ./assa.pl line 9.
#!/usr/bin/perl
 at <> 1.
Use of uninitialized value in printf at ./assa.pl line 9.
# Second comment. at <> 5.
Use of uninitialized value in printf at ./assa.pl line 9.
# Third comment. at <> 6.

logical_filename always return undef. It seems the only way to set filename is using #line directive in source. Why filename is not initialized from actual filename passed to PPI::Document->new?