Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LowExpiryCacheTimeSniff: Account for random generating number func #445

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

rebeccahum
Copy link
Contributor

Fixes #408

@GaryJones GaryJones force-pushed the rebecca/adjust_lowcacheexpirysniff branch from 783c30a to b1b0393 Compare April 17, 2020 19:16
$this->phpcsFile->addWarning( $message, $stackPtr, 'LowCacheTime', $data );
$times = [];
if ( false !== $rand_function ) {
$times = explode( ',', preg_replace( '/[( )|\(|\)|(' . $rand_function . ')]/', '', $time ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this regex is going to miss cases where there are unusual code styling - line breaks before, between, or after string arguments like 20 * HOUR_IN_SECONDS - I'd at least like to see more unit tests, and I think when we switch to be able to use https://phpcsutils.com/phpdoc/classes/PHPCSUtils-Utils-PassedParameters.html#method_getParameters then this will be much easier to be confident with.

@GaryJones
Copy link
Contributor

Conflicts need resolving before it can be reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance logic of LowCacheTime to minimize noise.
2 participants