Allow stacked methods

[rvandam]

I found a lot of instances of $self->dbh->quote_identifier in my code that failed to pass because multiple stacked calls were not followed. This small change should allow that to work. Only flaw is it also allows $self->quote->unsafe->thing. If that is a concern, I could further refactor the code to only test the final method call in the stack.

[oalders]

My apologies. For some reason I didn't see this PR until today. :(

If that is a concern, I could further refactor the code to only test the final method call in the stack.

I think that would be helpful. If you're able to do that, I can get this into a new release for you.

[rvandam]

So there's a tradeoff. The problem I identified predates my changes and so "fixing" it will introduce a backwards-incompatible change.

$self->quote->unsafe->thing is already allowed (assuming quote is in quoting_methods) because get_complete_variable bails as soon as it sees a token that looks like a quoting_method. It just returns $self-> as the variable.

I can fix it by removing the last on line 594 and then doing some extra work to put back the previously skipped token if we end up continuing. That would result in the full $self->quote->unsafe->thing being returned as the variable and marked as not quoted. That's probably correct but since it's a breaking change (at least for anyone with such an obscure case in their codebase), I'll leave it up to you whether it's worth addressing. Tentative diff included here for reference:

diff --git a/lib/Perl/Critic/Policy/ValuesAndExpressions/PreventSQLInjection.pm b/lib/Perl/Critic/Policy/ValuesAndExpressions/PreventSQLInjection.pm
index cd26876..4d46c8c 100644
--- a/lib/Perl/Critic/Policy/ValuesAndExpressions/PreventSQLInjection.pm
+++ b/lib/Perl/Critic/Policy/ValuesAndExpressions/PreventSQLInjection.pm
@@ -581,9 +581,19 @@ sub get_complete_variable {
 
         if (   $sibling->isa('PPI::Token::Operator')
             && $sibling->content() eq '->' ) {
+            if ($is_quoted) {
+                # false positive on previous token
+                $variable .= $sibling->previous_sibling();
+                $is_quoted = 0;
+            }
             $variable .= '->';
         }
         elsif ( $sibling->isa('PPI::Structure::Subscript') ) {
+            if ($is_quoted) {
+                # false positive on previous token
+                $variable .= $sibling->previous_sibling();
+                $is_quoted = 0;
+            }
             $variable .= $sibling->content();
         }
         elsif ($sibling->isa('PPI::Token::Word')
@@ -591,7 +601,6 @@ sub get_complete_variable {
             && defined( $self->{'_quoting_methods_regex'} )
             && ( $sibling->content =~ $self->{'_quoting_methods_regex'} ) ) {
             $is_quoted = 1;
-            last;
         }
         elsif ($sibling->isa('PPI::Token::Word')
             && $sibling->method_call()
diff --git a/t/ValuesAndExpressions/PreventSQLInjection.run b/t/ValuesAndExpressions/PreventSQLInjection.run
index d1bf0a9..5ec0eb1 100644
--- a/t/ValuesAndExpressions/PreventSQLInjection.run
+++ b/t/ValuesAndExpressions/PreventSQLInjection.run
@@ -459,6 +459,14 @@ $sql = "UPDATE table_name SET field = " . $dbh->test($value) . "WHERE field = "
 
 $sql = "UPDATE table_name SET field = " . $foo->bar->baz->dbh->test($value) . "WHERE field = 1";
 
+
+## name Ignore non-final quoting methods
+## parms { quoting_methods => 'test' }
+## failures 1
+## cut
+
+$sql = "UPDATE table_name SET field = " . $foo->test->bar->baz($value) . "WHERE field = 1";
+

[oalders]

Thank you! This looks good. I'm ok with a breaking change for this particular case.

[oalders]

If you want to update the PR, I'm happy to move ahead with this.