False positive for mail function checks

[zauan]

Hi guys,

This plugins has many false positives when checking the theme. The example for this ticket is the following Warning :

"WARNING: Found mail in the file theme-functions.php. Mail functions are plugin territory."

This rule should check if the actual mail function is used instead of searching for every "mail" word in the theme.

In our latest theme, we are using the mail word in various locations:

"User email" , form inputs that have the "mail" type, etc.

Can you restrict this check to only search for actual mail() functions instead of the generic mail word?

[scottparry]

There are occasions where false positives will occur no matter what, even when we've used a valid regex search. That is why they are set as WARNINGS rather than REQUIRED.

Anything that triggers a WARNING will not always be cause for rejection as the reviewer will check the context.

[zauan]

I understand this, however, we are talking about changing 1 line of code ( you just have to add "(" ) to make the check better. If you don't have time to improve the plugin, i can make the pull request.

Even if the false positives and Warnings are not subject for rejections, it does make it harder for theme authors and I believe reviewers to look trough many false positives and warning errors.

[scottparry]

@zauan If you check the code, the regex is fine:

'/[^a-z0-9](?<!_)wp_mail\s?\(/' '/[^a-z0-9](?<!_)mail\s?\(/'

[zauan]

Indeed, it looks good. I will investigate to see why I'm getting so many false positives and will get back to you.

[zauan]

It seems that the problem comes from the following line :

$error = ltrim( trim( $matches[0], '(' ) );

This removes the last '(' before passing the error to the tc_grep function which makes a stristr on all the lines in the php file. Since the '(' is removed, it shows all the occurrences of 'mail' strings.

I won't go any further than this on my investigations.

[scottparry]

@zauan Good catch! I missed that! I'll fix it up!

[scottparry]

@zauan I've removed ltrim altogether, and changed trim. See how it works for you now.

[zauan]

@iKreativ It works fine now. it only shows the exact locations where the actual mail() function is used.

[scottparry]

@zauan Excellent! Thanks for spotting!