does not detect method calls sometimes

[vsespb]

my $table = 'a';
my $sql = "select * from ".$xyz->abc($table);
print $sql;

(there is warning)

my $table = 'a';
my $sql = "select * from ".xyz->abc($table);
print $sql;

(no warning)

use case for last example is: xyz() is a function returning a global object, like locale(). or function used in DSL.

[guillaumeaubert]

Hi Victor,

The gh13_function_calls branch now correctly reviews class method/functions, and your examples produce the right results.

Remaining things to do before I can merge to master and do a release:

• Since I added the ability to specify safe methods in SQL safe comments, I need to rename "safe variables" into "safe elements" in several places to be consistent with this new use of SQL safe comments. I also need to update the documentation with examples for this use case.
• I need to provide a way to configure in .perlcriticrc a list of class functions / class methods / methods that are safe. The goal being to be able to customize $QUOTING_METHODS_REGEX and $SAFE_FUNCTIONS, which are currently read-only.

Thank you for all your suggestions / bug reports!

[guillaumeaubert]

The first bullet point has been taken care of. I'm going to address the .perlcriticrc part a bit later this week.

[guillaumeaubert]

The list of safe functions and safe quoting methods is now configurable via .perlcriticrc, which completes this ticket.

@vsespb - Would you like to review https://github.com/guillaumeaubert/Perl-Critic-Policy-ValuesAndExpressions-PreventSQLInjection/compare/gh13_function_calls before I merge it to master and release it out to CPAN?

[vsespb]

I've read docs and testcases - looks good. I did not test code (well, if tests pass there is no need to test manually). I did not tried to "hack" it and find edge cases/bugs in this new code. I have couple of ideas how I can "hack" it, but I think it's not related to this new functionality and I'll probably try after it merged to master.

[vsespb]

one thing that breaks now:

## name ZZZ
## failures 0
## cut

my $sql = "select from " . $x->( $y ); ## SQL safe ($x, $y)

(test failed)

[guillaumeaubert]

I've read docs and testcases - looks good.

Thank you, Victor!

I did not test code (well, if tests pass there is no need to test manually). I did not tried to "hack" it and find edge cases/bugs in this new code. I have couple of ideas how I can "hack" it, but I think it's not related to this new functionality and I'll probably try after it merged to master.

Sounds good - then I'm going to merge to master now, so that you can tweak the code from there.

one thing that breaks now:

## name ZZZ
## failures 0
## cut

my $sql = "select from " . $x->( $y ); ## SQL safe ($x, $y) (test failed)

I hadn't considered anonymous function calls like this, but it's an excellent case to add to the code. Would you mind opening a pull request for that test? Then I'll see how to make the parser look for that case and handle it properly.

Again, many thanks for all your contributions!

[vsespb]

ok, i'll try to submit PR(s) in a couple of days

[guillaumeaubert]

Thank you, Victor. I'm going to release v1.3.0 in the meantime then, so that we get the new features out before we add more :)