-
-
Notifications
You must be signed in to change notification settings - Fork 483
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
WP/CronInterval: bugfix for fully qualified time constants #2073
WP/CronInterval: bugfix for fully qualified time constants #2073
Conversation
Added a second commit addressing the review remarks for easier verification, Please squash the commit on merge (or I can squash and force push ahead of merge). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, with one suggestion / self-note.
6784816
to
fe09fed
Compare
See: #2073 (comment) Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
If the WP time constants would be used in their fully qualified form, as is often encountered in namespaced files, the sniff would no longer be able to correctly determine the interval being scheduled. This commit fixes that. Note: To prevent constants from a different namespace accidentally matching the WP native constants, the namespace separator is replace by a space character rather than just ignored. The regex before running the `eval()` will prevent code with identifier names other than the WP time constants from being passed to the `eval()`, meaning those will still be reported as "interval undetermined". Includes unit tests.
fe09fed
to
935257b
Compare
I've rebased the PR (without changes) to get passed the imaginary merge conflict. I've squashed the second commit with the updates after review into the first one now as well, so if found okay, this PR can now be merged normally. |
If the WP time constants would be used in their fully qualified form, as is often encountered in namespaced files, the sniff would no longer be able to correctly determine the interval being scheduled.
Note: To prevent constants from a different namespace accidentally matching the WP native constants, the namespace separator is replaced by a space character rather than just ignored. The regex before running the
eval()
will prevent code with identifier names other than the WP time constants from being passed to theeval()
, meaning those will still be reported as "interval undetermined".This commit fixes that.
Includes unit tests.