better warnings when code is stupid and calls ->params(undef)

diff --git a/Changes b/Changes
index 2bb1c21..541f876 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,10 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+5.90072 - 2014-09-15
+  - In the case where you call $c->req->param(undef), warn with a more useful
+    warning (now gives the line of your code that called param with the undef,
+    so you can go to hunt it out.
+
 5.90071 - 2014-08-10
   - Travis config now performs basic reverse dependency testing.
   - Restored deprecated 'env' code in Engine.pm b/c it is still being used out

diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index f02d62a..958e005 100644 (file)
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -127,7 +127,7 @@ __PACKAGE__->stats_class('Catalyst::Stats');
 __PACKAGE__->_encode_check(Encode::FB_CROAK | Encode::LEAVE_SRC);
 
 # Remember to update this in Catalyst::Runtime as well!
-our $VERSION = '5.90071';
+our $VERSION = '5.90072';
 
 sub import {
     my ( $class, @arguments ) = @_;

diff --git a/lib/Catalyst/Request.pm b/lib/Catalyst/Request.pm
index 09fb8d5..39cc3a4 100644 (file)
--- a/lib/Catalyst/Request.pm
+++ b/lib/Catalyst/Request.pm
@@ -645,9 +645,15 @@ sub param {
         return keys %{ $self->parameters };
     }
 
-    if ( @_ == 1 ) {
+    # If anything in @_ is undef, carp about that, and remove it from
+    # the list;
+    
+    my @params = grep { defined($_) ? 1 : do {carp "You called ->params with an undefined value"; 0} } @_;
+
+    if ( @params == 1 ) {
 
-        my $param = shift;
+        defined(my $param = shift @params) ||
+          carp "You called ->params with an undefined value 2";
 
         unless ( exists $self->parameters->{$param} ) {
             return wantarray ? () : undef;
@@ -664,8 +670,8 @@ sub param {
               : $self->parameters->{$param};
         }
     }
-    elsif ( @_ > 1 ) {
-        my $field = shift;
+    elsif ( @params > 1 ) {
+        my $field = shift @params;
         $self->parameters->{$field} = [@_];
     }
 }

diff --git a/lib/Catalyst/Runtime.pm b/lib/Catalyst/Runtime.pm
index f2fb334..446286f 100644 (file)
--- a/lib/Catalyst/Runtime.pm
+++ b/lib/Catalyst/Runtime.pm
@@ -7,7 +7,7 @@ BEGIN { require 5.008003; }
 
 # Remember to update this in Catalyst as well!
 
-our $VERSION = '5.90071';
+our $VERSION = '5.90072';
 
 =head1 NAME
 

diff --git a/t/undef-params.t b/t/undef-params.t
new file mode 100644 (file)
index 0000000..8ce8e2c
--- /dev/null
+++ b/t/undef-params.t
@@ -0,0 +1,44 @@
+use warnings;
+use strict ;
+use Test::More;
+use HTTP::Request::Common;
+use Plack::Test;
+
+# If someone does $c->req->params(undef) you don't get a very good
+# error message.  This is a test to see if the proposed change improves
+# that.
+
+
+{
+  package MyApp::Controller::Root;
+  $INC{'MyApp/Controller/Root.pm'} = __FILE__;
+
+  use base 'Catalyst::Controller';
+
+  sub test :Local {
+    my ($self, $c) = @_;
+    my $value = $c->req->param(undef);
+
+    $c->response->body("This is the body");
+  }
+
+  package MyApp;
+  use Catalyst;
+
+  $SIG{__WARN__} = sub {
+    my $error = shift;
+    Test::More::is($error, "You called ->params with an undefined value at t/undef-params.t line 20.\n");
+  };
+
+  MyApp->setup, 'setup app';
+}
+
+ok my $psgi = MyApp->psgi_app, 'build psgi app';
+
+test_psgi $psgi, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/root/test");
+    is $res->code, 200, 'OK';
+};
+
+done_testing;