Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Part of #3324560: Replace strpos/substr with str_starts_with() / str_contains() / str_ends_with().
Proposed resolution
- Replace
strpos($a, $b) !== FALSEwithstr_contains($a, $b) - Replace
strpos($a, $b) === FALSEwith!str_contains($a, $b) - A few are deliberately omitted where they would cause merge conflicts with other child issues.
Remaining tasks
- When reviewing, consider using
git diff --color-wordsorgit diff --porecelain. - Pay attention to the
!. It should be in the expression once either before the change or after the change.
User interface changes
N/A
API changes
N/A
Data model changes
N/A
Release notes snippet
N/A
| Comment | File | Size | Author |
|---|
Issue fork drupal-3328454
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3328454-strpos-contains
changes, plain diff MR !3145
Comments
Comment #2
xjmComment #4
xjmComment #5
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #7
xjmComment #8
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #9
bhanu951 commentedComment #10
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #11
nod_While I'll look into it
Comment #12
xjmThere was a merge conflict due to #3328454: Replace most strpos() !== FALSE or === FALSE with str_contains() moving a line we're changing to a different file. Addressed with the latest merge.
Comment #13
xjmComment #14
smustgrave commentedBeen avoiding this ticket a week haha
Finally got around to reviewing, searching for strpos( and reviewing each found a few files
PathProcessorImageStyles (few instances)
BookJavascriptTest (not sure if tests are included)
UserNameConstraintValidator
Comment #16
jidrone commentedHi,
I added the cases mentioned by @smustgrave and others that I found.
I'm only taking into account the following patterns as suggested in the description:
strpos($a, $b) !== FALSEstrpos($a, $b) === FALSEI was not including:
($pos = strpos($input, '@', $pos)) !== false($start = strpos($message, '{')) !== FALSEstrpos($value, $condition['value']) === 0Comment #17
smustgrave commentedThank you!
I think not including is correct. Think that would make the scope of this too large IMO. This looks good!
Comment #18
catchThere are at least two changes to str_starts_with(), which isn't the scope defined in #16. All the in-scope changes look fine, but let's please limit the MR to those, makes it a lot harder to scan the hundreds of lines of diff if there are multiple patterns dealt with at once.
Note to self: got down to AliasPathProcessor
Comment #19
jidrone commentedReverted the 2 cases that were not strpos replacements.
Comment #20
smustgrave commentedThanks @catch for the review.
Comment #22
catchI'm getting a phpstan error trying to commit this locally, which I don't think is the patch but I sometimes see weird failures from phpstan when it runs on particular files.
I've committed this with --no-verify, and if I'm wrong, well mea culpa.
Committed b3fac4a and pushed to 10.1.x. Thanks!