Name describe_class_methods arguments earlier
[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / _Util.pm
index bfb6a2a..371db28 100644 (file)
@@ -1,11 +1,15 @@
 package # hide from PAUSE
   DBIx::Class::_Util;
 
+use DBIx::Class::StartupCheck;  # load es early as we can, usually a noop
+
 use warnings;
 use strict;
 
 use constant SPURIOUS_VERSION_CHECK_WARNINGS => ( "$]" < 5.010 ? 1 : 0);
 
+my $mro_recursor_stack;
+
 BEGIN {
   package # hide from pause
     DBIx::Class::_ENV_;
@@ -23,21 +27,21 @@ BEGIN {
 
     UNSTABLE_DOLLARAT => ( "$]" < 5.013002 ) ? 1 : 0,
 
-    DBICTEST => $INC{"DBICTest/Util.pm"} ? 1 : 0,
-
-    # During 5.13 dev cycle HELEMs started to leak on copy
-    # add an escape for these perls ON SMOKERS - a user will still get death
-    PEEPEENESS => ( eval { DBICTest::RunMode->is_smoker } && ( "$]" >= 5.013005 and "$]" <= 5.013006) ),
-
-    SHUFFLE_UNORDERED_RESULTSETS => $ENV{DBIC_SHUFFLE_UNORDERED_RESULTSETS} ? 1 : 0,
-
-    ASSERT_NO_INTERNAL_WANTARRAY => $ENV{DBIC_ASSERT_NO_INTERNAL_WANTARRAY} ? 1 : 0,
-
-    ASSERT_NO_INTERNAL_INDIRECT_CALLS => $ENV{DBIC_ASSERT_NO_INTERNAL_INDIRECT_CALLS} ? 1 : 0,
-
-    STRESSTEST_UTF8_UPGRADE_GENERATED_COLLAPSER_SOURCE => $ENV{DBIC_STRESSTEST_UTF8_UPGRADE_GENERATED_COLLAPSER_SOURCE} ? 1 : 0,
-
-    STRESSTEST_COLUMN_INFO_UNAWARE_STORAGE => $ENV{DBIC_STRESSTEST_COLUMN_INFO_UNAWARE_STORAGE} ? 1 : 0,
+    ( map
+      #
+      # the "DBIC_" prefix below is crucial - this is what makes CI pick up
+      # all envvars without further adjusting its scripts
+      # DO NOT CHANGE to the more logical { $_ => !!( $ENV{"DBIC_$_"} ) }
+      #
+      { substr($_, 5) => !!( $ENV{$_} ) }
+      qw(
+        DBIC_SHUFFLE_UNORDERED_RESULTSETS
+        DBIC_ASSERT_NO_INTERNAL_WANTARRAY
+        DBIC_ASSERT_NO_INTERNAL_INDIRECT_CALLS
+        DBIC_STRESSTEST_UTF8_UPGRADE_GENERATED_COLLAPSER_SOURCE
+        DBIC_STRESSTEST_COLUMN_INFO_UNAWARE_STORAGE
+      )
+    ),
 
     IV_SIZE => $Config{ivsize},
 
@@ -47,11 +51,86 @@ BEGIN {
   if ( "$]" < 5.009_005) {
     require MRO::Compat;
     constant->import( OLD_MRO => 1 );
+
+    #
+    # Yes, I know this is a rather PHP-ish name, but please first read
+    # https://metacpan.org/source/BOBTFISH/MRO-Compat-0.12/lib/MRO/Compat.pm#L363-368
+    #
+    # Even if we are using Class::C3::XS it still won't work, as doing
+    #   defined( *{ "SubClass::"->{$_} }{CODE} )
+    # will set pkg_gen to the same value for SubClass and *ALL PARENTS*
+    #
+    *DBIx::Class::_Util::get_real_pkg_gen = sub ($) {
+      require Digest::MD5;
+      require Math::BigInt;
+
+      # the non-assign-unless-there-is-a-hash is deliberate
+      ( $mro_recursor_stack->{cache} || {} )->{$_[0]}{gen} ||= (
+        Math::BigInt->new( '0x' . ( Digest::MD5::md5_hex( join "\0", map {
+
+          ( $mro_recursor_stack->{cache} || {} )->{$_}{methlist} ||= do {
+
+            my $class = $_;
+
+            no strict 'refs';
+            my %methlist =
+              map
+                # this is essentially a uniq_by step
+                # it is crucial on OLD_MRO
+                {( Scalar::Util::refaddr($_) => $_ )}
+                map
+                  {
+                    (
+                      ref(\ "${class}::"->{$_} ) ne 'GLOB'
+                        or
+                      defined( *{ "${class}::"->{$_} }{CODE} )
+                    )
+                    ? ( \&{"${class}::$_"} )
+                    : ()
+                  }
+                  keys %{ "${class}::" }
+            ;
+
+            # RV to be hashed up and turned into a number
+            join "\0", (
+              $class,
+              map {(
+                $_, # refaddr is sufficient, ignore names entirely
+                attributes::get( $methlist{$_} )
+              )} sort keys %methlist
+            ),
+          }
+        } ( 'UNIVERSAL', @{
+          ( $mro_recursor_stack->{cache} || {} )->{$_[0]}{linear_isa}
+            ||=
+           mro::get_linear_isa($_[0])
+        } ) ) ) )
+      );
+    };
   }
   else {
     require mro;
     constant->import( OLD_MRO => 0 );
+    *DBIx::Class::_Util::get_real_pkg_gen = \&mro::get_pkg_gen;
   }
+
+  # Both of these are no longer used for anything. However bring
+  # them back after they were purged in 08a8d8f1, as there appear
+  # to be outfits with *COPY PASTED* pieces of lib/DBIx/Class/Storage/*
+  # in their production codebases. There is no point in breaking these
+  # if whatever they used actually continues to work
+  my $warned;
+  my $sigh = sub {
+
+    require Carp;
+    my $cluck = "The @{[ (caller(1))[3] ]} constant is no more - adjust your code" . Carp::longmess();
+
+    warn $cluck unless $warned->{$cluck}++;
+
+    0;
+  };
+  sub DBICTEST () { &$sigh }
+  sub PEEPEENESS () { &$sigh }
 }
 
 # FIXME - this is not supposed to be here
@@ -62,8 +141,9 @@ use B ();
 use Carp 'croak';
 use Storable 'nfreeze';
 use Scalar::Util qw(weaken blessed reftype refaddr);
-use List::Util qw(first);
-use Sub::Quote qw(qsub quote_sub);
+use Sub::Quote qw(qsub);
+use Sub::Name ();
+use attributes ();
 
 # Already correctly prototyped: perlbrew exec perl -MStorable -e 'warn prototype \&Storable::dclone'
 BEGIN { *deep_clone = \&Storable::dclone }
@@ -72,14 +152,79 @@ use base 'Exporter';
 our @EXPORT_OK = qw(
   sigwarn_silencer modver_gt_or_eq modver_gt_or_eq_and_lt
   fail_on_internal_wantarray fail_on_internal_call
-  refdesc refcount hrefaddr
-  scope_guard is_exception detected_reinvoked_destructor
-  quote_sub qsub perlstring serialize deep_clone
+  refdesc refcount hrefaddr set_subname describe_class_methods
+  scope_guard detected_reinvoked_destructor
+  is_exception dbic_internal_try visit_namespaces
+  quote_sub qsub perlstring serialize deep_clone dump_value uniq
+  parent_dir mkdir_p
   UNRESOLVABLE_CONDITION
 );
 
 use constant UNRESOLVABLE_CONDITION => \ '1 = 0';
 
+BEGIN {
+  # add preliminary attribute support
+  # FIXME FIXME FIXME
+  # To be revisited when Moo with proper attr support ships
+  Sub::Quote->VERSION(2.002);
+}
+# Override forcing no_defer, and adding naming consistency checks
+sub quote_sub {
+  Carp::confess( "Anonymous quoting not supported by the DBIC sub_quote override - supply a sub name" ) if
+    @_ < 2
+      or
+    ! defined $_[1]
+      or
+    length ref $_[1]
+  ;
+
+  Carp::confess( "The DBIC sub_quote override expects sub name '$_[0]' to be fully qualified" )
+    unless $_[0] =~ /::/;
+
+  Carp::confess( "The DBIC sub_quote override expects the sub name '$_[0]' to match the supplied 'package' argument" ) if
+    $_[3]
+      and
+    defined $_[3]->{package}
+      and
+    index( $_[0], $_[3]->{package} ) != 0
+  ;
+
+  my @caller = caller(0);
+  my $sq_opts = {
+    package => $caller[0],
+    hints => $caller[8],
+    warning_bits => $caller[9],
+    hintshash => $caller[10],
+    %{ $_[3] || {} },
+
+    # explicitly forced for everything
+    no_defer => 1,
+  };
+
+  my $cref = Sub::Quote::quote_sub( $_[0], $_[1], $_[2]||{}, $sq_opts );
+
+  # FIXME FIXME FIXME
+  # To be revisited when Moo with proper attr support ships
+  if(
+    # external application does not work on things like :prototype(...), :lvalue, etc
+    my @attrs = grep {
+      $_ !~ /^[a-z]/
+        or
+      Carp::confess( "The DBIC sub_quote override does not support applying of reserved attribute '$_'" )
+    } @{ $sq_opts->{attributes} || []}
+  ) {
+    Carp::confess( "The DBIC sub_quote override does not allow mixing 'attributes' with 'no_install'" )
+      if $sq_opts->{no_install};
+
+    # might be different from $sq_opts->{package};
+    my ($install_into) = $_[0] =~ /(.+)::[^:]+$/;
+
+    attributes->import( $install_into, $cref, @attrs );
+  }
+
+  $cref;
+}
+
 sub sigwarn_silencer ($) {
   my $pattern = shift;
 
@@ -114,11 +259,96 @@ sub refcount ($) {
   B::svref_2object($_[0])->REFCNT;
 }
 
+sub visit_namespaces {
+  my $args = { (ref $_[0]) ? %{$_[0]} : @_ };
+
+  my $visited_count = 1;
+
+  # A package and a namespace are subtly different things
+  $args->{package} ||= 'main';
+  $args->{package} = 'main' if $args->{package} =~ /^ :: (?: main )? $/x;
+  $args->{package} =~ s/^:://;
+
+  if ( $args->{action}->($args->{package}) ) {
+    my $ns =
+      ( ($args->{package} eq 'main') ? '' :  $args->{package} )
+        .
+      '::'
+    ;
+
+    $visited_count += visit_namespaces( %$args, package => $_ ) for
+      grep
+        # this happens sometimes on %:: traversal
+        { $_ ne '::main' }
+        map
+          { $_ =~ /^(.+?)::$/ ? "$ns$1" : () }
+          do { no strict 'refs'; keys %$ns }
+    ;
+  }
+
+  $visited_count;
+}
+
+# FIXME In another life switch this to a polyfill like the one in namespace::clean
+sub set_subname ($$) {
+
+  # fully qualify name
+  splice @_, 0, 1, caller(0) . "::$_[0]"
+    if $_[0] !~ /::|'/;
+
+  &Sub::Name::subname;
+}
+
 sub serialize ($) {
   local $Storable::canonical = 1;
   nfreeze($_[0]);
 }
 
+sub uniq {
+  my( %seen, $seen_undef, $numeric_preserving_copy );
+  grep { not (
+    defined $_
+      ? $seen{ $numeric_preserving_copy = $_ }++
+      : $seen_undef++
+  ) } @_;
+}
+
+my $dd_obj;
+sub dump_value ($) {
+  local $Data::Dumper::Indent = 1
+    unless defined $Data::Dumper::Indent;
+
+  my $dump_str = (
+    $dd_obj
+      ||=
+    do {
+      require Data::Dumper;
+      my $d = Data::Dumper->new([])
+        ->Purity(0)
+        ->Pad('')
+        ->Useqq(1)
+        ->Terse(1)
+        ->Freezer('')
+        ->Quotekeys(0)
+        ->Bless('bless')
+        ->Pair(' => ')
+        ->Sortkeys(1)
+        ->Deparse(1)
+      ;
+
+      $d->Sparseseen(1) if modver_gt_or_eq (
+        'Data::Dumper', '2.136'
+      );
+
+      $d;
+    }
+  )->Values([$_[0]])->Dump;
+
+  $dd_obj->Reset->Values([]);
+
+  $dump_str;
+}
+
 sub scope_guard (&) {
   croak 'Calling scope_guard() in void context makes no sense'
     if ! defined wantarray;
@@ -138,9 +368,11 @@ sub scope_guard (&) {
     eval {
       $_[0]->[0]->();
       1;
-    } or do {
-      Carp::cluck "Execution of scope guard $_[0] resulted in the non-trappable exception:\n\n$@";
-    };
+    }
+      or
+    Carp::cluck(
+      "Execution of scope guard $_[0] resulted in the non-trappable exception:\n\n$@"
+    );
   }
 }
 
@@ -157,6 +389,7 @@ sub is_exception ($) {
 
   my ($not_blank, $suberror);
   {
+    local $SIG{__DIE__} if $SIG{__DIE__};
     local $@;
     eval {
       # The ne() here is deliberate - a plain length($e), or worse "$e" ne
@@ -223,13 +456,98 @@ sub is_exception ($) {
 }
 
 {
-  my $destruction_registry = {};
+  my $callstack_state;
+
+  # Recreate the logic of try(), while reusing the catch()/finally() as-is
+  #
+  # FIXME: We need to move away from Try::Tiny entirely (way too heavy and
+  # yes, shows up ON TOP of profiles) but this is a batle for another maint
+  sub dbic_internal_try (&;@) {
+
+    my $try_cref = shift;
+    my $catch_cref = undef;  # apparently this is a thing... https://rt.perl.org/Public/Bug/Display.html?id=119311
+
+    for my $arg (@_) {
+
+      if( ref($arg) eq 'Try::Tiny::Catch' ) {
+
+        croak 'dbic_internal_try() may not be followed by multiple catch() blocks'
+          if $catch_cref;
+
+        $catch_cref = $$arg;
+      }
+      elsif ( ref($arg) eq 'Try::Tiny::Finally' ) {
+        croak 'dbic_internal_try() does not support finally{}';
+      }
+      else {
+        croak(
+          'dbic_internal_try() encountered an unexpected argument '
+        . "'@{[ defined $arg ? $arg : 'UNDEF' ]}' - perhaps "
+        . 'a missing semi-colon before or ' # trailing space important
+        );
+      }
+    }
+
+    my $wantarray = wantarray;
+    my $preexisting_exception = $@;
 
-  sub CLONE {
-    $destruction_registry = { map
-      { defined $_ ? ( refaddr($_) => $_ ) : () }
-      values %$destruction_registry
+    my @ret;
+    my $all_good = eval {
+      $@ = $preexisting_exception;
+
+      local $callstack_state->{in_internal_try} = 1
+        unless $callstack_state->{in_internal_try};
+
+      # always unset - someone may have snuck it in
+      local $SIG{__DIE__} if $SIG{__DIE__};
+
+      if( $wantarray ) {
+        @ret = $try_cref->();
+      }
+      elsif( defined $wantarray ) {
+        $ret[0] = $try_cref->();
+      }
+      else {
+        $try_cref->();
+      }
+
+      1;
     };
+
+    my $exception = $@;
+    $@ = $preexisting_exception;
+
+    if ( $all_good ) {
+      return $wantarray ? @ret : $ret[0]
+    }
+    elsif ( $catch_cref ) {
+      for ( $exception ) {
+        return $catch_cref->($exception);
+      }
+    }
+
+    return;
+  }
+
+  sub in_internal_try { !! $callstack_state->{in_internal_try} }
+}
+
+{
+  my $destruction_registry = {};
+
+  sub DBIx::Class::__Util_iThreads_handler__::CLONE {
+    %$destruction_registry = map {
+      (defined $_)
+        ? ( refaddr($_) => $_ )
+        : ()
+    } values %$destruction_registry;
+
+    weaken($_) for values %$destruction_registry;
+
+    # Dummy NEXTSTATE ensuring the all temporaries on the stack are garbage
+    # collected before leaving this scope. Depending on the code above, this
+    # may very well be just a preventive measure guarding future modifications
+    undef;
   }
 
   # This is almost invariably invoked from within DESTROY
@@ -295,8 +613,8 @@ sub modver_gt_or_eq ($$) {
     local $SIG{__WARN__} = sigwarn_silencer( qr/\Qisn't numeric in subroutine entry/ )
       if SPURIOUS_VERSION_CHECK_WARNINGS;
 
+    local $SIG{__DIE__} if $SIG{__DIE__};
     local $@;
-    local $SIG{__DIE__};
     eval { $mod->VERSION($ver) } ? 1 : 0;
   };
 
@@ -317,6 +635,208 @@ sub modver_gt_or_eq_and_lt ($$$) {
 }
 
 {
+  # FIXME - should be a private my(), but I'm too uncertain whether
+  # all bases are covered
+  our $describe_class_query_cache;
+
+  sub describe_class_methods {
+    my ($class) = @_;
+
+    croak "Expecting a class name"
+      if not defined $class or $class !~ $module_name_rx;
+
+    # use a cache on old MRO, since while we are recursing in this function
+    # nothing can possibly change (the speedup is immense)
+    # (yes, people could be tie()ing the stash and adding methods on access
+    # but there is a limit to how much crazy can be supported here)
+    #
+    # we use the cache for linear_isa lookups on new MRO as well - it adds
+    # a *tiny* speedup, and simplifies the code a lot
+    #
+    local $mro_recursor_stack->{cache} = {}
+      unless $mro_recursor_stack->{cache};
+
+    my $my_gen = 0;
+
+    $my_gen += get_real_pkg_gen($_) for (
+      'UNIVERSAL',
+      ( $class, my @my_ISA ) = @{
+        $mro_recursor_stack->{cache}{$class}{linear_isa}
+          ||=
+        mro::get_linear_isa($class)
+      }
+    );
+
+    my $slot = $describe_class_query_cache->{$class} ||= {};
+
+    unless ( ($slot->{cumulative_gen}||0) == $my_gen ) {
+
+      # reset
+      %$slot = (
+        class => $class,
+        isa => [ @my_ISA ], # copy before we shove UNIVERSAL into it
+        mro => {
+          type => mro::get_mro($class),
+        },
+        cumulative_gen => $my_gen,
+      );
+      $slot->{mro}{is_c3} = ($slot->{mro}{type} eq 'c3') ? 1 : 0;
+
+      push @my_ISA, 'UNIVERSAL';
+
+      # ensure the cache is populated for the parents, code below can then
+      # efficiently operate over the query_cache directly
+      describe_class_methods($_) for reverse @my_ISA;
+
+      my ($methods_seen_via_ISA_on_old_mro, $current_node_refaddr);
+      no strict 'refs';
+
+      # combine full ISA-order inherited and local method list into a
+      # "shadowing stack"
+
+      (
+        $current_node_refaddr = refaddr($_)
+
+          and
+
+        # on complex MI herarchies the method can be anywhere in the
+        # shadow stack - look through the entire slot, not just [0]
+        ( ! grep {
+          refaddr($_) == $current_node_refaddr
+        } @{ $slot->{methods}{ $_->{name} } || [] } )
+
+          and
+
+        unshift @{ $slot->{methods}{$_->{name}} }, $_
+
+          and
+
+        @{ $slot->{methods}{$_->{name}} } > 1
+
+          and
+
+        $slot->{methods_with_supers}{$_->{name}} = $slot->{methods}{$_->{name}}
+
+      ) for (
+
+        # what describe_class_methods for @my_ISA produced above
+        ( map { $_->[0] } map {
+          values %{ $describe_class_query_cache->{$_}{methods} }
+        } reverse @my_ISA ),
+
+        # our own non-cleaned subs + their attributes
+        ( map {
+          (
+            # these 2 OR-ed checks are sufficient for 5.10+
+            (
+              ref(\ "${class}::"->{$_} ) ne 'GLOB'
+                or
+              defined( *{ "${class}::"->{$_} }{CODE} )
+            )
+              and
+            # need to account for dummy helper crefs under OLD_MRO
+            (
+              ! DBIx::Class::_ENV_::OLD_MRO
+                or
+              (
+                $methods_seen_via_ISA_on_old_mro ||= do {
+                  my $rv = {};
+                  $rv->{$_->{name}}->{ refaddr( \&{ "$_->{via_class}::$_->{name}"} ) } = 1 for
+                    map { @$_ } map
+                      { values %{ $describe_class_query_cache->{$_}{methods} } }
+                      @my_ISA;
+                  $rv;
+                }
+                  and
+                (
+                  ! $methods_seen_via_ISA_on_old_mro->{$_}
+                    or
+                  ! $methods_seen_via_ISA_on_old_mro->{$_}{ refaddr( \&{"${class}::${_}"} ) }
+                )
+              )
+            )
+          ) ? {
+              via_class => $class,
+              name => $_,
+              attributes => {
+                map { $_ => 1 } attributes::get( \&{"${class}::${_}"} )
+              },
+            }
+            : ()
+        } keys %{"${class}::"} )
+      );
+
+
+      # recalculate the pkg_gen on newer perls under Taint mode,
+      # because of shit like:
+      # perl -T -Mmro -e 'package Foo; sub bar {}; defined( *{ "Foo::"->{bar}}{CODE} ) and warn mro::get_pkg_gen("Foo") for (1,2,3)'
+      #
+      if (
+        ! DBIx::Class::_ENV_::OLD_MRO
+          and
+        ${^TAINT}
+      ) {
+
+        $slot->{cumulative_gen} = 0;
+        $slot->{cumulative_gen} += get_real_pkg_gen($_)
+          for $class, @my_ISA;
+      }
+    }
+
+    # RV
+    +{ %$slot };
+  }
+}
+
+
+#
+# Why not just use some higher-level module or at least File::Spec here?
+# Because:
+# 1)  This is a *very* rarely used function, and the deptree is large
+#     enough already as it is
+#
+# 2)  (more importantly) Our tooling is utter shit in this area. There
+#     is no comprehensive support for UNC paths in PathTools and there
+#     are also various small bugs in representation across different
+#     path-manipulation CPAN offerings.
+#
+# Since this routine is strictly used for logical path processing (it
+# *must* be able to work with not-yet-existing paths), use this seemingly
+# simple but I *think* complete implementation to feed to other consumers
+#
+# If bugs are ever uncovered in this routine, *YOU ARE URGED TO RESIST*
+# the impulse to bring in an external dependency. During runtime there
+# is exactly one spot that could potentially maybe once in a blue moon
+# use this function. Keep it lean.
+#
+sub parent_dir ($) {
+  ( $_[0] =~ m{  [\/\\]  ( \.{0,2} ) ( [\/\\]* ) \z }x )
+    ? (
+      $_[0]
+        .
+      ( ( length($1) and ! length($2) ) ? '/' : '' )
+        .
+      '../'
+    )
+    : (
+      require File::Spec
+        and
+      File::Spec->catpath (
+        ( File::Spec->splitpath( "$_[0]" ) )[0,1],
+        '/',
+      )
+    )
+  ;
+}
+
+sub mkdir_p ($) {
+  require File::Path;
+  # do not ask for a recent version, use 1.x API calls
+  File::Path::mkpath([ "$_[0]" ]);  # File::Path does not like objects
+}
+
+
+{
   my $list_ctx_ok_stack_marker;
 
   sub fail_on_internal_wantarray () {
@@ -362,8 +882,8 @@ sub modver_gt_or_eq_and_lt ($$$) {
       ), 'with_stacktrace');
     }
 
-    my $mark = [];
-    weaken ( $list_ctx_ok_stack_marker = $mark );
+    weaken( $list_ctx_ok_stack_marker = my $mark = [] );
+
     $mark;
   }
 }
@@ -375,19 +895,36 @@ sub fail_on_internal_call {
     $fr = [ CORE::caller(1) ];
     $argdesc = ref $DB::args[0]
       ? DBIx::Class::_Util::refdesc($DB::args[0])
-      : undef
+      : ( $DB::args[0] . '' )
     ;
   };
 
+  my @fr2;
+  # need to make allowance for a proxy-yet-direct call
+  my $check_fr = (
+    $fr->[0] eq 'DBIx::Class::ResultSourceProxy'
+      and
+    @fr2 = (CORE::caller(2))
+      and
+    (
+      ( $fr->[3] =~ /([^:])+$/ )[0]
+        eq
+      ( $fr2[3] =~ /([^:])+$/ )[0]
+    )
+  )
+    ? \@fr2
+    : $fr
+  ;
+
   if (
     $argdesc
       and
-    $fr->[0] =~ /^(?:DBIx::Class|DBICx::)/
+    $check_fr->[0] =~ /^(?:DBIx::Class|DBICx::)/
       and
-    $fr->[1] !~ /\b(?:CDBICompat|ResultSetProxy)\b/  # no point touching there
+    $check_fr->[1] !~ /\b(?:CDBICompat|ResultSetProxy)\b/  # no point touching there
   ) {
     DBIx::Class::Exception->throw( sprintf (
-      "Illegal internal call of indirect proxy-method %s() with argument %s: examine the last lines of the proxy method deparse below to determine what to call directly instead at %s on line %d\n\n%s\n\n    Stacktrace starts",
+      "Illegal internal call of indirect proxy-method %s() with argument '%s': examine the last lines of the proxy method deparse below to determine what to call directly instead at %s on line %d\n\n%s\n\n    Stacktrace starts",
       $fr->[3], $argdesc, @{$fr}[1,2], ( $fr->[6] || do {
         require B::Deparse;
         no strict 'refs';