From 271477d8c26794abd8310e2abb89b746204660af Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Thu, 22 Apr 2010 11:02:17 -0700 Subject: Bug 560009: Use firstidx from List::MoreUtils instead of lsearch r=timello, a=mkanat --- Bugzilla.pm | 30 ++++++++++++++++-------------- Bugzilla/Bug.pm | 7 +++++-- Bugzilla/CGI.pm | 2 +- Bugzilla/Config/Common.pm | 10 +++++----- Bugzilla/DB/Schema.pm | 7 ++++--- Bugzilla/Field.pm | 4 ++-- Bugzilla/Migrate/Gnats.pm | 15 +++++++++------ Bugzilla/Template.pm | 6 +++++- Bugzilla/User.pm | 5 +++-- Bugzilla/Util.pm | 32 +------------------------------- attachment.cgi | 6 ++---- buglist.cgi | 14 +++++++------- contrib/bzdbcopy.pl | 8 +++----- editflagtypes.cgi | 8 ++++++-- enter_bug.cgi | 11 +++++++---- process_bug.cgi | 3 ++- report.cgi | 4 +++- showdependencygraph.cgi | 2 +- t/007util.t | 8 +------- 19 files changed, 83 insertions(+), 99 deletions(-) diff --git a/Bugzilla.pm b/Bugzilla.pm index 8bcef7b28..2f21c6a18 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -62,17 +62,17 @@ use Safe; ##################################################################### # Scripts that are not stopped by shutdownhtml being in effect. -use constant SHUTDOWNHTML_EXEMPT => [ - 'editparams.cgi', - 'checksetup.pl', - 'migrate.pl', - 'recode.pl', -]; +use constant SHUTDOWNHTML_EXEMPT => qw( + editparams.cgi + checksetup.pl + migrate.pl + recode.pl +); # Non-cgi scripts that should silently exit. -use constant SHUTDOWNHTML_EXIT_SILENTLY => [ - 'whine.pl' -]; +use constant SHUTDOWNHTML_EXIT_SILENTLY => qw( + whine.pl +); ##################################################################### # Global Code @@ -112,8 +112,10 @@ sub init_page { }; } + my $script = basename($0); + # Because of attachment_base, attachment.cgi handles this itself. - if (basename($0) ne 'attachment.cgi') { + if ($script ne 'attachment.cgi') { do_ssl_redirect_if_required(); } @@ -123,14 +125,14 @@ sub init_page { # # This code must go here. It cannot go anywhere in Bugzilla::CGI, because # it uses Template, and that causes various dependency loops. - if (Bugzilla->params->{"shutdownhtml"} - && lsearch(SHUTDOWNHTML_EXEMPT, basename($0)) == -1) + if (Bugzilla->params->{"shutdownhtml"} + && !grep { $_ eq $script } SHUTDOWNHTML_EXEMPT) { # Allow non-cgi scripts to exit silently (without displaying any # message), if desired. At this point, no DBI call has been made # yet, and no error will be returned if the DB is inaccessible. - if (lsearch(SHUTDOWNHTML_EXIT_SILENTLY, basename($0)) > -1 - && !i_am_cgi()) + if (!i_am_cgi() + && grep { $_ eq $script } SHUTDOWNHTML_EXIT_SILENTLY) { exit; } diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 3edab563a..def8ad360 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -49,6 +49,7 @@ use Bugzilla::Group; use Bugzilla::Status; use Bugzilla::Comment; +use List::MoreUtils qw(firstidx); use List::Util qw(min first); use Storable qw(dclone); use URI; @@ -3057,8 +3058,10 @@ sub editable_bug_fields { # Obsolete custom fields are not editable. my @obsolete_fields = Bugzilla->get_fields({obsolete => 1, custom => 1}); @obsolete_fields = map { $_->name } @obsolete_fields; - foreach my $remove ("bug_id", "reporter", "creation_ts", "delta_ts", "lastdiffed", @obsolete_fields) { - my $location = lsearch(\@fields, $remove); + foreach my $remove ("bug_id", "reporter", "creation_ts", "delta_ts", + "lastdiffed", @obsolete_fields) + { + my $location = firstidx { $_ eq $remove } @fields; # Custom multi-select fields are not stored in the bugs table. splice(@fields, $location, 1) if ($location > -1); } diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index 6e9dfd0ce..75b7f18d7 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -115,7 +115,7 @@ sub canonicalise_query { my @parameters; foreach my $key (sort($self->param())) { # Leave this key out if it's in the exclude list - next if lsearch(\@exclude, $key) != -1; + next if grep { $_ eq $key } @exclude; # Remove the Boolean Charts for standard query.cgi fields # They are listed in the query URL already diff --git a/Bugzilla/Config/Common.pm b/Bugzilla/Config/Common.pm index 7416b1794..8fc1b6037 100644 --- a/Bugzilla/Config/Common.pm +++ b/Bugzilla/Config/Common.pm @@ -144,7 +144,7 @@ sub check_utf8 { sub check_priority { my ($value) = (@_); my $legal_priorities = get_legal_field_values('priority'); - if (lsearch($legal_priorities, $value) < 0) { + if (!grep($_ eq $value, @$legal_priorities)) { return "Must be a legal priority value: one of " . join(", ", @$legal_priorities); } @@ -154,7 +154,7 @@ sub check_priority { sub check_severity { my ($value) = (@_); my $legal_severities = get_legal_field_values('bug_severity'); - if (lsearch($legal_severities, $value) < 0) { + if (!grep($_ eq $value, @$legal_severities)) { return "Must be a legal severity value: one of " . join(", ", @$legal_severities); } @@ -164,7 +164,7 @@ sub check_severity { sub check_platform { my ($value) = (@_); my $legal_platforms = get_legal_field_values('rep_platform'); - if (lsearch(['', @$legal_platforms], $value) < 0) { + if (!grep($_ eq $value, '', @$legal_platforms)) { return "Must be empty or a legal platform value: one of " . join(", ", @$legal_platforms); } @@ -174,7 +174,7 @@ sub check_platform { sub check_opsys { my ($value) = (@_); my $legal_OS = get_legal_field_values('op_sys'); - if (lsearch(['', @$legal_OS], $value) < 0) { + if (!grep($_ eq $value, '', @$legal_OS)) { return "Must be empty or a legal operating system value: one of " . join(", ", @$legal_OS); } @@ -184,7 +184,7 @@ sub check_opsys { sub check_bug_status { my $bug_status = shift; my @closed_bug_statuses = map {$_->name} closed_bug_statuses(); - if (lsearch(\@closed_bug_statuses, $bug_status) < 0) { + if (!grep($_ eq $bug_status, @closed_bug_statuses)) { return "Must be a valid closed status: one of " . join(', ', @closed_bug_statuses); } return ""; diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index 6520766f3..8e0458a51 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -44,6 +44,7 @@ use Bugzilla::Constants; use Carp qw(confess); use Digest::MD5 qw(md5_hex); use Hash::Util qw(lock_value unlock_hash lock_keys unlock_keys); +use List::MoreUtils qw(firstidx); use Safe; # Historical, needed for SCHEMA_VERSION = '1.00' use Storable qw(dclone freeze thaw); @@ -2451,7 +2452,7 @@ sub delete_column { my ($self, $table, $column) = @_; my $abstract_fields = $self->{abstract_schema}{$table}{FIELDS}; - my $name_position = lsearch($abstract_fields, $column); + my $name_position = firstidx { $_ eq $column } @$abstract_fields; die "Attempted to delete nonexistent column ${table}.${column}" if $name_position == -1; # Delete the key/value pair from the array. @@ -2540,7 +2541,7 @@ sub set_index { sub _set_object { my ($self, $table, $name, $definition, $array_to_change) = @_; - my $obj_position = lsearch($array_to_change, $name) + 1; + my $obj_position = (firstidx { $_ eq $name } @$array_to_change) + 1; # If the object doesn't exist, then add it. if (!$obj_position) { push(@$array_to_change, $name); @@ -2573,7 +2574,7 @@ sub delete_index { my ($self, $table, $name) = @_; my $indexes = $self->{abstract_schema}{$table}{INDEXES}; - my $name_position = lsearch($indexes, $name); + my $name_position = firstidx { $_ eq $name } @$indexes; die "Attempted to delete nonexistent index $name on the $table table" if $name_position == -1; # Delete the key/value pair from the array. diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index 2513a19ae..b53abfb61 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -1154,8 +1154,8 @@ sub check_field { } if (!defined($value) - || trim($value) eq "" - || lsearch($legalsRef, $value) < 0) + or trim($value) eq "" + or !grep { $_ eq $value } @$legalsRef) { return 0 if $no_warn; # We don't want an error to be thrown; return. trick_taint($name); diff --git a/Bugzilla/Migrate/Gnats.pm b/Bugzilla/Migrate/Gnats.pm index 232100f2d..ff24f73b5 100644 --- a/Bugzilla/Migrate/Gnats.pm +++ b/Bugzilla/Migrate/Gnats.pm @@ -25,12 +25,13 @@ use base qw(Bugzilla::Migrate); use Bugzilla::Constants; use Bugzilla::Install::Util qw(indicate_progress); -use Bugzilla::Util qw(format_time trim generate_random_password lsearch); +use Bugzilla::Util qw(format_time trim generate_random_password); use Email::Address; use Email::MIME; use File::Basename; use IO::File; +use List::MoreUtils qw(firstidx); use List::Util qw(first); use constant REQUIRED_MODULES => [ @@ -168,14 +169,14 @@ use constant NON_COMMENT_FIELDS => qw( # we list out here the exact order of fields at the end of a PR # and wait for the next field to consider that we actually have # a field to parse. -use constant END_FIELD_ORDER => [qw( +use constant END_FIELD_ORDER => qw( Description How-To-Repeat Fix Release-Note Audit-Trail Unformatted -)]; +); use constant CUSTOM_FIELDS => { cf_type => { @@ -374,10 +375,12 @@ sub _get_gnats_field_data { # If this is one of the last few PR fields, then make sure # that we're getting our fields in the right order. my $new_field_valid = 1; - my $current_field_pos = - lsearch(END_FIELD_ORDER, $current_field || ''); + my $search_for = $current_field || ''; + my $current_field_pos = firstidx { $_ eq $search_for } + END_FIELD_ORDER; if ($current_field_pos > -1) { - my $new_field_pos = lsearch(END_FIELD_ORDER, $new_field); + my $new_field_pos = firstidx { $_ eq $new_field } + END_FIELD_ORDER; # We accept any field, as long as it's later than this one. $new_field_valid = $new_field_pos > $current_field_pos ? 1 : 0; } diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index da46774a1..a4e250cfc 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -55,6 +55,7 @@ use File::Find; use File::Path qw(rmtree mkpath); use File::Spec; use IO::Dir; +use List::MoreUtils qw(firstidx); use Scalar::Util qw(blessed); use base qw(Template); @@ -716,7 +717,10 @@ sub create { 'time2str' => \&Date::Format::time2str, # Generic linear search function - 'lsearch' => \&Bugzilla::Util::lsearch, + 'lsearch' => sub { + my ($array, $item) = @_; + return firstidx { $_ eq $item } @$array; + }, # Currently logged in user, if any # If an sudo session is in progress, this is the user we're faking diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 5c6209811..351dddfae 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -562,10 +562,11 @@ sub get_products_by_permission { # No need to go further if the user has no "special" privs. return [] unless scalar(@$product_ids); + my %product_map = map { $_ => 1 } @$product_ids; # We will restrict the list to products the user can see. my $selectable_products = $self->get_selectable_products; - my @products = grep {lsearch($product_ids, $_->id) > -1} @$selectable_products; + my @products = grep { $product_map{$_->id} } @$selectable_products; return \@products; } @@ -1490,7 +1491,7 @@ sub is_mover { if (!defined $self->{'is_mover'}) { my @movers = map { trim($_) } split(',', Bugzilla->params->{'movers'}); $self->{'is_mover'} = ($self->id - && lsearch(\@movers, $self->login) != -1); + && grep { $_ eq $self->login } @movers); } return $self->{'is_mover'}; } diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index de67d5a59..03826c143 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -36,7 +36,7 @@ use base qw(Exporter); html_quote url_quote xml_quote css_class_quote html_light_quote url_decode i_am_cgi correct_urlbase remote_ip - lsearch do_ssl_redirect_if_required use_attachbase + do_ssl_redirect_if_required use_attachbase diff_arrays trim wrap_hard wrap_comment find_wrap_point format_time format_time_decimal validate_date @@ -306,18 +306,6 @@ sub use_attachbase { && $attachbase ne Bugzilla->params->{'sslbase'}) ? 1 : 0; } -sub lsearch { - my ($list,$item) = (@_); - my $count = 0; - foreach my $i (@$list) { - if ($i eq $item) { - return $count; - } - $count++; - } - return -1; -} - sub diff_arrays { my ($old_ref, $new_ref) = @_; @@ -680,9 +668,6 @@ Bugzilla::Util - Generic utility functions for bugzilla my $is_cgi = i_am_cgi(); my $urlbase = correct_urlbase(); - # Functions for searching - $loc = lsearch(\@arr, $val); - # Data manipulation ($removed, $added) = diff_arrays(\@old, \@new); @@ -821,21 +806,6 @@ otherwise. =back -=head2 Searching - -Functions for searching within a set of values. - -=over 4 - -=item C - -Returns the position of C<$item> in C<$list>. C<$list> must be a list -reference. - -If the item is not in the list, returns -1. - -=back - =head2 Data Manipulation =over 4 diff --git a/attachment.cgi b/attachment.cgi index 0b389501b..17846866f 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -207,12 +207,10 @@ sub attachmentIsPublic { # Validates format of a diff/interdiff. Takes a list as an parameter, which # defines the valid format values. Will throw an error if the format is not # in the list. Returns either the user selected or default format. -sub validateFormat -{ +sub validateFormat { # receives a list of legal formats; first item is a default my $format = $cgi->param('format') || $_[0]; - if ( lsearch(\@_, $format) == -1) - { + if (not grep($_ eq $format, @_)) { ThrowUserError("invalid_format", { format => $format, formats => \@_ }); } diff --git a/buglist.cgi b/buglist.cgi index 3090b2a88..576b95572 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -683,7 +683,7 @@ my @selectcolumns = ("bug_id", "bug_severity", "priority", "bug_status", "resolution", "product"); # remaining and actual_time are required for percentage_complete calculation: -if (lsearch(\@displaycolumns, "percentage_complete") >= 0) { +if (grep { $_ eq "percentage_complete" } @displaycolumns) { push (@selectcolumns, "remaining_time"); push (@selectcolumns, "actual_time"); } @@ -906,12 +906,12 @@ $buglist_sth->execute(); # of Perl records. # If we're doing time tracking, then keep totals for all bugs. -my $percentage_complete = lsearch(\@displaycolumns, 'percentage_complete') >= 0; -my $estimated_time = lsearch(\@displaycolumns, 'estimated_time') >= 0; -my $remaining_time = ((lsearch(\@displaycolumns, 'remaining_time') >= 0) - || $percentage_complete); -my $actual_time = ((lsearch(\@displaycolumns, 'actual_time') >= 0) - || $percentage_complete); +my $percentage_complete = grep($_ eq 'percentage_complete', @displaycolumns); +my $estimated_time = grep($_ eq 'estimated_time', @displaycolumns); +my $remaining_time = grep($_ eq 'remaining_time', @displaycolumns) + || $percentage_complete; +my $actual_time = grep($_ eq 'actual_time', @displaycolumns) + || $percentage_complete; my $time_info = { 'estimated_time' => 0, 'remaining_time' => 0, diff --git a/contrib/bzdbcopy.pl b/contrib/bzdbcopy.pl index a5e81d7f8..8491238b5 100755 --- a/contrib/bzdbcopy.pl +++ b/contrib/bzdbcopy.pl @@ -71,11 +71,9 @@ my $ident_char = $target_db->get_info( 29 ); # SQL_IDENTIFIER_QUOTE_CHAR # has customized their source DB, we still want the script to work, # and it may otherwise fail in that situation (that is, the tables # may not exist in the target DB). -my @table_list = $target_db->bz_table_list_real(); - -# We don't want to copy over the bz_schema table's contents. -my $bz_schema_location = lsearch(\@table_list, 'bz_schema'); -splice(@table_list, $bz_schema_location, 1) if $bz_schema_location > 0; +# +# We don't want to copy over the bz_schema table's contents, though. +my @table_list = grep { $_ ne 'bz_schema' } $target_db->bz_table_list_real(); # Instead of figuring out some fancy algorithm to insert data in the right # order and not break FK integrity, we just drop them all. diff --git a/editflagtypes.cgi b/editflagtypes.cgi index 4f85e6c65..d389c6db7 100755 --- a/editflagtypes.cgi +++ b/editflagtypes.cgi @@ -237,11 +237,15 @@ sub processCategoryChange { } elsif ($categoryAction eq 'removeInclusion') { my @inclusion_to_remove = $cgi->param('inclusion_to_remove'); - @inclusions = map {(lsearch(\@inclusion_to_remove, $_) < 0) ? $_ : ()} @inclusions; + foreach my $remove (@inclusion_to_remove) { + @inclusions = grep { $_ ne $remove } @inclusions; + } } elsif ($categoryAction eq 'removeExclusion') { my @exclusion_to_remove = $cgi->param('exclusion_to_remove'); - @exclusions = map {(lsearch(\@exclusion_to_remove, $_) < 0) ? $_ : ()} @exclusions; + foreach my $remove (@exclusion_to_remove) { + @exclusions = grep { $_ ne $remove } @exclusions; + } } # Convert the array @clusions('prod_ID:comp_ID') back to a hash of diff --git a/enter_bug.cgi b/enter_bug.cgi index efca5491d..9c6a1c6b4 100755 --- a/enter_bug.cgi +++ b/enter_bug.cgi @@ -508,14 +508,17 @@ else { # parameter. $vars->{'version'} = [map($_->name, @{$product->versions})]; +my $version_cookie = $cgi->cookie("VERSION-" . $product->name); + if ( ($cloned_bug_id) && ($product->name eq $cloned_bug->product ) ) { $default{'version'} = $cloned_bug->version; } elsif (formvalue('version')) { $default{'version'} = formvalue('version'); -} elsif (defined $cgi->cookie("VERSION-" . $product->name) && - lsearch($vars->{'version'}, $cgi->cookie("VERSION-" . $product->name)) != -1) { - $default{'version'} = $cgi->cookie("VERSION-" . $product->name); +} elsif (defined $version_cookie + and grep { $_ eq $version_cookie } @{ $vars->{'version'} }) +{ + $default{'version'} = $version_cookie; } else { $default{'version'} = $vars->{'version'}->[$#{$vars->{'version'}}]; } @@ -556,7 +559,7 @@ $vars->{'bug_status'} = \@status; # Otherwise, and only if the user has privs, set the default # to the first confirmed bug status on the list, if available. -if (formvalue('bug_status') && (lsearch(\@status, formvalue('bug_status')) >= 0)) { +if (formvalue('bug_status') && grep { $_ eq formvalue('bug_status') } @status) { $default{'bug_status'} = formvalue('bug_status'); } elsif (scalar @status == 1) { $default{'bug_status'} = $status[0]; diff --git a/process_bug.cgi b/process_bug.cgi index 6afb9cc91..3f1e81bcb 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -61,6 +61,7 @@ use Bugzilla::Flag; use Bugzilla::Status; use Bugzilla::Token; +use List::MoreUtils qw(firstidx); use Storable qw(dclone); my $user = Bugzilla->login(LOGIN_REQUIRED); @@ -208,7 +209,7 @@ if (defined $cgi->param('id')) { if ($cgi->cookie("BUGLIST")) { @bug_list = split(/:/, $cgi->cookie("BUGLIST")); } - my $cur = lsearch(\@bug_list, $cgi->param('id')); + my $cur = firstidx { $_ eq $cgi->param('id') } @bug_list; if ($cur >= 0 && $cur < $#bug_list) { my $next_bug_id = $bug_list[$cur + 1]; detaint_natural($next_bug_id); diff --git a/report.cgi b/report.cgi index 33d46675f..4c9a98b8a 100755 --- a/report.cgi +++ b/report.cgi @@ -349,7 +349,9 @@ sub get_names { # the normal order. # # This is O(n^2) but it shouldn't matter for short lists. - @sorted = map {lsearch(\@unsorted, $_) == -1 ? () : $_} @{$field_list}; + foreach my $item (@$field_list) { + push(@sorted, $item) if grep { $_ eq $item } @unsorted; + } } elsif ($isnumeric) { # It's not a field we are preserving the order of, so sort it diff --git a/showdependencygraph.cgi b/showdependencygraph.cgi index e73b1f633..a036ee0c9 100755 --- a/showdependencygraph.cgi +++ b/showdependencygraph.cgi @@ -101,7 +101,7 @@ my @valid_rankdirs = ('LR', 'RL', 'TB', 'BT'); my $rankdir = $cgi->param('rankdir') || 'TB'; # Make sure the submitted 'rankdir' value is valid. -if (lsearch(\@valid_rankdirs, $rankdir) < 0) { +if (!grep { $_ eq $rankdir } @valid_rankdirs) { $rankdir = 'TB'; } diff --git a/t/007util.t b/t/007util.t index af36e94ac..742c2b2d2 100644 --- a/t/007util.t +++ b/t/007util.t @@ -26,7 +26,7 @@ use lib 't'; use Support::Files; -use Test::More tests => 16; +use Test::More tests => 13; BEGIN { use_ok(Bugzilla); @@ -50,12 +50,6 @@ is(html_quote(""),"<lala&@>",'html_quote'); #url_quote(): is(url_quote("gaa\"'[]{\\"),"%3Clala%26%3Egaa%22%27%5B%5D%7B%5C",'url_quote'); -#lsearch(): -my @list = ('apple','pear','plum','<"\\%'); -is(lsearch(\@list,'pear'),1,'lsearch 1'); -is(lsearch(\@list,'<"\\%'),3,'lsearch 2'); -is(lsearch(\@list,'kiwi'),-1,'lsearch 3 (missing item)'); - #trim(): is(trim(" fg<*\$%>+=~~ "),'fg<*$%>+=~~','trim()'); -- cgit v1.2.3-24-g4f1b