Make inline constraints safer ...
Dave Rolsky [Thu, 21 Apr 2011 20:39:20 +0000 (15:39 -0500)]
If we're defining a temp var - do this in a do {} block to limit its scope and
prevent it from conflicting with another constraint's var.

Don't use return in an inline constraint (this is super bad)

Don't use $_ to iterate over a collection of values in generated code, since
this could easily conflict with the surrounding code.

lib/Moose/Util/TypeConstraints/Builtins.pm

index ad6e1f5..7c636d3 100644 (file)
@@ -60,7 +60,7 @@ sub define_builtins {
         => inline_as {
             'defined(' . $_[1] . ') '
               . '&& (ref(\\' . $_[1] . ') eq "SCALAR"'
-               . '|| ref(\\(my $val = ' . $_[1] . ')) eq "SCALAR")'
+              . '|| ref(\\(my $val = ' . $_[1] . ')) eq "SCALAR")'
         };
 
     subtype 'Num'
@@ -77,7 +77,7 @@ sub define_builtins {
         => inline_as {
             'defined(' . $_[1] . ') '
               . '&& !ref(' . $_[1] . ') '
-              . '&& (my $val = ' . $_[1] . ') =~ /\A-?[0-9]+\z/'
+              . '&& do { (my $val = ' . $_[1] . ') =~ /\A-?[0-9]+\z/ }'
         };
 
     subtype 'CodeRef'
@@ -134,9 +134,10 @@ sub define_builtins {
         }
         => inline_as {
             'Class::MOP::is_class_loaded(' . $_[1] . ') '
-              . '&& (Class::MOP::class_of(' . $_[1] . ') || return)->isa('
-                  . '"Moose::Meta::Role"'
-              . ')'
+              . '&& do {'
+                  . 'my $meta = Class::MOP::class_of(' . $_[1] . ');'
+                  . '$meta && $meta->isa("Moose::Meta::Role");'
+              . '}'
         };
 
     $registry->add_type_constraint(
@@ -189,11 +190,15 @@ sub define_builtins {
                 my $self           = shift;
                 my $type_parameter = shift;
                 my $val            = shift;
-                'ref(' . $val . ') eq "ARRAY" '
-                  . '&& &List::MoreUtils::all('
-                      . 'sub { ' . $type_parameter->_inline_check('$_') . ' }, '
-                      . '@{' . $val . '}'
-                  . ')'
+
+                'do {'
+                    . 'my $check = ' . $val . ';'
+                    . 'ref($check) eq "ARRAY" '
+                        . '&& &List::MoreUtils::all('
+                            . 'sub { ' . $type_parameter->_inline_check('$_') . ' }, '
+                            . '@{$check}'
+                        . ')'
+                . '}';
             },
         )
     );
@@ -220,11 +225,15 @@ sub define_builtins {
                 my $self           = shift;
                 my $type_parameter = shift;
                 my $val            = shift;
-                'ref(' . $val . ') eq "HASH" '
-                  . '&& &List::MoreUtils::all('
-                      . 'sub { ' . $type_parameter->_inline_check('$_') . ' }, '
-                      . 'values %{' . $val . '}'
-                  . ')'
+
+                'do {'
+                    . 'my $check = ' . $val . ';'
+                    . 'ref($check) eq "HASH" '
+                        . '&& &List::MoreUtils::all('
+                            . 'sub { ' . $type_parameter->_inline_check('$_') . ' }, '
+                            . 'values %{$check}'
+                        . ')'
+                . '}';
             },
         )
     );