Bug 560009: Use firstidx from List::MoreUtils instead of lsearch

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;
         }

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);
     }

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

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 "";

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.

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);

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;
             }

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

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'};
 }

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<lsearch($list, $item)>
-
-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

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 => \@_ });
   }
 

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,

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.

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

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];

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);

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 

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';
 }
 

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&@>"),"&lt;lala&amp;&#64;&gt;",'html_quote');
 #url_quote():
 is(url_quote("<lala&>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()');