Add deprecation warnings for cols/include_columns

[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / ResultSet.pm

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index 545f993..b64140d 100644 (file)
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -5,7 +5,7 @@ use warnings;
 use base qw/DBIx::Class/;
 use DBIx::Class::Carp;
 use DBIx::Class::ResultSetColumn;
-use Scalar::Util qw/blessed weaken/;
+use Scalar::Util qw/blessed weaken reftype/;
 use Try::Tiny;
 use Data::Compare (); # no imports!!! guard against insane architecture
 
@@ -25,6 +25,10 @@ use overload
         'bool'   => "_bool",
         fallback => 1;
 
+# this is real - CDBICompat overrides it with insanity
+# yes, prototype won't matter, but that's for now ;)
+sub _bool () { 1 }
+
 __PACKAGE__->mk_group_accessors('simple' => qw/_result_class result_source/);
 
 =head1 NAME
@@ -389,11 +393,11 @@ sub search_rs {
   my $cache;
   my %safe = (alias => 1, cache => 1);
   if ( ! List::Util::first { !$safe{$_} } keys %$call_attrs and (
-    ! defined $_[0]
+    ! defined $call_cond
       or
-    ref $_[0] eq 'HASH' && ! keys %{$_[0]}
+    ref $call_cond eq 'HASH' && ! keys %$call_cond
       or
-    ref $_[0] eq 'ARRAY' && ! @{$_[0]}
+    ref $call_cond eq 'ARRAY' && ! @$call_cond
   )) {
     $cache = $self->get_cache;
   }
@@ -435,6 +439,7 @@ sub search_rs {
 
     # older deprecated name, use only if {columns} is not there
     if (my $c = delete $new_attrs->{cols}) {
+      carp_unique( "Resultset attribute 'cols' is deprecated, use 'columns' instead" );
       if ($new_attrs->{columns}) {
         carp "Resultset specifies both the 'columns' and the legacy 'cols' attributes - ignoring 'cols'";
       }
@@ -481,8 +486,12 @@ sub _normalize_selection {
   my ($self, $attrs) = @_;
 
   # legacy syntax
-  $attrs->{'+columns'} = $self->_merge_attr($attrs->{'+columns'}, delete $attrs->{include_columns})
-    if exists $attrs->{include_columns};
+  if ( exists $attrs->{include_columns} ) {
+    carp_unique( "Resultset attribute 'include_columns' is deprecated, use '+columns' instead" );
+    $attrs->{'+columns'} = $self->_merge_attr(
+      $attrs->{'+columns'}, delete $attrs->{include_columns}
+    );
+  }
 
   # columns are always placed first, however
 
@@ -1591,18 +1600,22 @@ sub _count_subq_rs {
 
       my ($lquote, $rquote, $sep) = map { quotemeta $_ } ($sql_maker->_quote_chars, $sql_maker->name_sep);
 
-      my $sql = $sql_maker->_parse_rs_attrs ({ having => $attrs->{having} });
+      my $having_sql = $sql_maker->_parse_rs_attrs ({ having => $attrs->{having} });
+      my %seen_having;
 
       # search for both a proper quoted qualified string, for a naive unquoted scalarref
       # and if all fails for an utterly naive quoted scalar-with-function
-      while ($sql =~ /
+      while ($having_sql =~ /
         $rquote $sep $lquote (.+?) $rquote
           |
         [\s,] \w+ \. (\w+) [\s,]
           |
         [\s,] $lquote (.+?) $rquote [\s,]
       /gx) {
-        push @parts, ($1 || $2 || $3);  # one of them matched if we got here
+        my $part = $1 || $2 || $3;  # one of them matched if we got here
+        unless ($seen_having{$part}++) {
+          push @parts, $part;
+        }
       }
     }
 
@@ -1632,9 +1645,6 @@ sub _count_subq_rs {
                   ->get_column ('count');
 }
 
-sub _bool {
-  return 1;
-}
 
 =head2 count_literal
 
@@ -2320,15 +2330,29 @@ sub new_result {
 
   my ($merged_cond, $cols_from_relations) = $self->_merge_with_rscond($values);
 
-  my %new = (
+  my $new = $self->result_class->new({
     %$merged_cond,
-    @$cols_from_relations
+    ( @$cols_from_relations
       ? (-cols_from_relations => $cols_from_relations)
-      : (),
+      : ()
+    ),
     -result_source => $self->result_source, # DO NOT REMOVE THIS, REQUIRED
-  );
+  });
+
+  if (
+    reftype($new) eq 'HASH'
+      and
+    ! keys %$new
+      and
+    blessed($new)
+  ) {
+    carp_unique (sprintf (
+      "%s->new returned a blessed empty hashref - a strong indicator something is wrong with its inheritance chain",
+      $self->result_class,
+    ));
+  }
 
-  return $self->result_class->new(\%new);
+  $new;
 }
 
 # _merge_with_rscond
@@ -3780,7 +3804,7 @@ case the key is the C<as> value, and the value is used as the C<select>
 expression). Adds C<me.> onto the start of any column without a C<.> in
 it and sets C<select> from that, then auto-populates C<as> from
 C<select> as normal. (You may also use the C<cols> attribute, as in
-earlier versions of DBIC.)
+earlier versions of DBIC, but this is deprecated.)
 
 Essentially C<columns> does the same as L</select> and L</as>.
 
@@ -3799,10 +3823,10 @@ is the same as
 
 =back
 
-Indicates additional columns to be selected from storage. Works the same
-as L</columns> but adds columns to the selection. (You may also use the
-C<include_columns> attribute, as in earlier versions of DBIC). For
-example:-
+Indicates additional columns to be selected from storage. Works the same as
+L</columns> but adds columns to the selection. (You may also use the
+C<include_columns> attribute, as in earlier versions of DBIC, but this is
+deprecated). For example:-
 
   $schema->resultset('CD')->search(undef, {
     '+columns' => ['artist.name'],