From c568fe7592917e4f325b171bd64267f526fcf880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Manuel=20Rodri=CC=81guez=20D?= Date: Thu, 17 Aug 2023 01:36:04 -0700 Subject: [PATCH] Replaced hash-key check with word-is-string check Checking if a word is interpreted as a string is a more suitable approach to solve the situations presented in the tests: t/hash-key.t t/hash-key-expression.t t/hash-unquoted-key.t --- lib/App/perlimports/Document.pm | 46 +++++++++++++++++++++----------- t/hash-key-expression.t | 25 +++++++++++++++++ t/hash-unquoted-key.t | 25 +++++++++++++++++ test-data/hash-key-expression.pl | 8 ++++++ test-data/hash-unquoted-key.pl | 7 +++++ 5 files changed, 96 insertions(+), 15 deletions(-) create mode 100644 t/hash-key-expression.t create mode 100644 t/hash-unquoted-key.t create mode 100644 test-data/hash-key-expression.pl create mode 100644 test-data/hash-unquoted-key.pl diff --git a/lib/App/perlimports/Document.pm b/lib/App/perlimports/Document.pm index 2d88a7f..8ed13cd 100644 --- a/lib/App/perlimports/Document.pm +++ b/lib/App/perlimports/Document.pm @@ -20,7 +20,8 @@ use PPIx::Utils::Classification qw( is_hash_key is_method_call ); -use Ref::Util qw( is_plain_arrayref is_plain_hashref ); +use Ref::Util qw( is_plain_arrayref is_plain_hashref ); +use Scalar::Util qw( refaddr ); use Sub::HandlesVia; use Text::Diff (); use Try::Tiny qw( catch try ); @@ -396,21 +397,9 @@ sub _build_possible_imports { # sub any {} next if $self->is_sub_name("$word"); - my $isa_symbol = $word->isa('PPI::Token::Symbol'); + next if !$word->isa('PPI::Token::Symbol') && is_method_call($word); - next if !$isa_symbol && is_method_call($word); - - # A hash key might, for example, be a variable. - if ( - !$isa_symbol - && !( - $word->statement - && $word->statement->isa('PPI::Statement::Variable') - ) - && is_hash_key($word) - ) { - next; - } + next if $self->_is_word_interpreted_as_string($word); push @after, $word; } @@ -1138,6 +1127,33 @@ sub _maybe_cache_inspectors { return; } +sub _is_word_interpreted_as_string { + my ( $self, $word ) = @_; + + return unless $word->statement && $word->isa('PPI::Token::Word'); + my @children = $word->statement->schildren; + + # https://perldoc.perl.org/perlref#Not-so-symbolic-references + return 1 if is_hash_key($word) && @children == 1; + + # The => operator (sometimes pronounced "fat comma") is a synonym for + # the comma except that it causes a word on its left to be interpreted + # as a string if it begins with a letter or underscore and is composed + # only of letters, digits and underscores. This includes operands that + # might otherwise be interpreted as operators, constants, single number + # v-strings or function calls. + # https://perldoc.perl.org/perlop#Comma-Operator + return unless $word->content =~ /^[a-zA-Z_][a-zA-Z0-9_]*$/; + + while ( my $current = shift @children ) { + last if refaddr($current) == refaddr($word); + } + return unless ( my $current = shift @children ); + return 1 + if $current->isa('PPI::Token::Operator') + && $current->content eq '=>'; +} + 1; # ABSTRACT: Make implicit imports explicit diff --git a/t/hash-key-expression.t b/t/hash-key-expression.t new file mode 100644 index 0000000..9fa8d2b --- /dev/null +++ b/t/hash-key-expression.t @@ -0,0 +1,25 @@ +#!/usr/bin/env perl + +use strict; +use warnings; + +use lib 't/lib'; + +use TestHelper qw( file2includes source2pi ); +use Test::More import => [qw( done_testing is )]; +use Test::Needs qw( HTTP::Status ); + +my @includes = file2includes('test-data/hash-key-expression.pl'); + +my $e = source2pi( + 'test-data/hash-key-expression.pl', undef, + { include => $includes[2] } +); + +is( + $e->formatted_ppi_statement, + 'use HTTP::Status qw( is_info );', + 'recognizes is_info as an imported symbol' +); + +done_testing; diff --git a/t/hash-unquoted-key.t b/t/hash-unquoted-key.t new file mode 100644 index 0000000..3193b3e --- /dev/null +++ b/t/hash-unquoted-key.t @@ -0,0 +1,25 @@ +#!/usr/bin/env perl + +use strict; +use warnings; + +use lib 't/lib'; + +use TestHelper qw( file2includes source2pi ); +use Test::More import => [qw( done_testing is )]; +use Test::Needs qw( HTTP::Status ); + +my @includes = file2includes('test-data/hash-unquoted-key.pl'); + +my $e = source2pi( + 'test-data/hash-unquoted-key.pl', undef, + { include => $includes[2] } +); + +is( + $e->formatted_ppi_statement, + 'use HTTP::Status ();', + 'recognizes is_info as a word representing a string' +); + +done_testing; diff --git a/test-data/hash-key-expression.pl b/test-data/hash-key-expression.pl new file mode 100644 index 0000000..9b55417 --- /dev/null +++ b/test-data/hash-key-expression.pl @@ -0,0 +1,8 @@ +use strict; +use warnings; + +use HTTP::Status qw(is_info); + +my %foo; +my $code = 100; +$foo{ is_info $code } = 'bar'; diff --git a/test-data/hash-unquoted-key.pl b/test-data/hash-unquoted-key.pl new file mode 100644 index 0000000..58079c7 --- /dev/null +++ b/test-data/hash-unquoted-key.pl @@ -0,0 +1,7 @@ +use strict; +use warnings; + +use HTTP::Status (); + +my %foo; +$foo{is_info} = 1;