PHP 8.x deprecated function warning at runtime

Are you sure you've updated to the latest codebase? sanitize_value() is already protected against that sort of error, it runs !is_null() before passing the $value to strlen().

My apologies, two of the errors are fixed. I've updated the issue summary. I am still seeing this code in the latest dev version of plugins/views_plugin_style.inc starting on line 151:

  /**
   * Take a value and apply token replacement logic to it.
   */
  public function tokenize_value($value, $row_index) {
    if (strpos($value, '[') !== FALSE || strpos($value, '!') !== FALSE || strpos($value, '%') !== FALSE) {
      $fake_item = array(
        'alter_text' => TRUE,
        'text' => $value,
      );

      // Row tokens might be empty, for example for node row style.
      $tokens = isset($this->row_tokens[$row_index]) ? $this->row_tokens[$row_index] : array();
      if (!empty($this->view->build_info['substitutions'])) {
        $tokens += $this->view->build_info['substitutions'];
      }

      if ($tokens) {
        $value = strtr($value, $tokens);
      }
    }

    return $value;
  }

which is where the error Deprecated function: strpos(): Passing null to parameter #1 ($string) of type string is deprecated in views_plugin_style->tokenize_value() (line 156 of /plugins/views_plugin_style.inc) might be coming from, unless $value is checked before calling the function?

Yeah, well spotted.

It'd be worth prefixing every call to strpos() or stripos() with a quick !is_null(), because evidently we missed some, or some snook back in after the last fix.

I haven't dived in to see how many of these instances represent places where the first argument to strpos could be null at runtime, but there are a bunch. I excluded ones found in tests for now.

$ grep -B 2 -r -e 'strpos' .

./views_ui.module-        $one_path = $display->handler->get_option('path');
./views_ui.module-
./views_ui.module:        if (empty($view->disabled) && strpos($one_path, '%') === FALSE) {
--
./plugins/views_wizard/views_ui_base_views_wizard.class.php-      else {
./plugins/views_wizard/views_ui_base_views_wizard.class.php-        foreach ($fields as $field_name => $value) {
./plugins/views_wizard/views_ui_base_views_wizard.class.php:          if ($pos = strpos($field_name, '.' . $bundle_key)) {
--
./plugins/views_plugin_style_jump_menu.inc-        // so users don't shoot themselves in the foot.
./plugins/views_plugin_style_jump_menu.inc-        $base_path = base_path();
./plugins/views_plugin_style_jump_menu.inc:        if (strpos($path, $base_path) === 0) {
--
./plugins/views_plugin_style.inc-    if ($this->uses_row_class()) {
./plugins/views_plugin_style.inc-      $class = $this->options['row_class'];
./plugins/views_plugin_style.inc:      if (strpos($class, '[') !== FALSE || strpos($class, '!') !== FALSE || strpos($class, '%') !== FALSE) {
--
./plugins/views_plugin_style.inc-   */
./plugins/views_plugin_style.inc-  public function tokenize_value($value, $row_index) {
./plugins/views_plugin_style.inc:    if (strpos($value, '[') !== FALSE || strpos($value, '!') !== FALSE || strpos($value, '%') !== FALSE) {
--
./plugins/views_plugin_pager_full.inc-    $error = FALSE;
./plugins/views_plugin_pager_full.inc-    $exposed_options = $form_state['values']['pager_options']['expose']['items_per_page_options'];
./plugins/views_plugin_pager_full.inc:    if (strpos($exposed_options, '.') !== FALSE) {
--
./plugins/views_plugin_display_page.inc-    switch ($form_state['section']) {
./plugins/views_plugin_display_page.inc-      case 'path':
./plugins/views_plugin_display_page.inc:        if (strpos($form_state['values']['path'], '$arg') !== FALSE) {
--
./plugins/views_plugin_display_page.inc-        }
./plugins/views_plugin_display_page.inc-
./plugins/views_plugin_display_page.inc:        if (strpos($form_state['values']['path'], '%') === 0) {
--
./plugins/views_plugin_display_page.inc-      case 'menu':
./plugins/views_plugin_display_page.inc-        $path = $this->get_option('path');
./plugins/views_plugin_display_page.inc:        if ($form_state['values']['menu']['type'] == 'normal' && strpos($path, '%') !== FALSE) {
--
./plugins/export_ui/views_ui.class.php-      else {
./plugins/export_ui/views_ui.class.php-        // Check whether the tag can be found in the views tag.
./plugins/export_ui/views_ui.class.php:        return strpos($view->tag, $form_state['values']['tag']) === FALSE;
--
./includes/ajax.inc-    $prefix = count($array) ? implode(', ', $array) . ', ' : '';
./includes/ajax.inc-
./includes/ajax.inc:    if (strpos('anonymous', strtolower($last_string)) !== FALSE) {
--
./includes/ajax.inc-      $n = $account;
./includes/ajax.inc-      // Commas and quotes in terms are special cases, so encode 'em.
./includes/ajax.inc:      if (strpos($account, ',') !== FALSE || strpos($account, '"') !== FALSE) {
--
./includes/ajax.inc-      $n = $name;
./includes/ajax.inc-      // Term names containing commas or quotes must be wrapped in quotes.
./includes/ajax.inc:      if (strpos($name, ',') !== FALSE || strpos($name, '"') !== FALSE) {
--
./includes/admin.inc-    if ($display->handler->has_path()) {
./includes/admin.inc-      $one_path = $display->handler->get_option('path');
./includes/admin.inc:      if (strpos($one_path, '%') === FALSE) {
--
./includes/admin.inc-      elseif ($display->handler->has_path()) {
./includes/admin.inc-        $path = $display->handler->get_path();
./includes/admin.inc:        if (strpos($path, '%') === FALSE) {
--
./includes/admin.inc-      list($table, $field) = explode('.', $field, 2);
./includes/admin.inc-
./includes/admin.inc:      if ($cut = strpos($field, '$')) {
--
./includes/admin.inc-  $views = views_get_all_views();
./includes/admin.inc-  foreach ($views as $view) {
./includes/admin.inc:    if (!empty($view->tag) && strpos($view->tag, $string) === 0) {
--
./includes/view.inc-        if ($this->display_handler->uses_breadcrumb() && $argument->uses_breadcrumb()) {
./includes/view.inc-          $path = $this->get_url($breadcrumb_args);
./includes/view.inc:          if (strpos($path, '%') === FALSE) {
--
./includes/view.inc-    }
./includes/view.inc-    // Don't bother working if there's nothing to do.
./includes/view.inc:    if (empty($path) || (empty($args) && strpos($path, '%') === FALSE)) {
--
./handlers/views_handler_field_numeric.inc-    }
./handlers/views_handler_field_numeric.inc-    else {
./handlers/views_handler_field_numeric.inc:      $point_position = strpos($value, '.');
--
./handlers/views_handler_field.inc-   */
./handlers/views_handler_field.inc-  public function tokenize_value($value, $row_index = NULL) {
./handlers/views_handler_field.inc:    if (strpos($value, '[') !== FALSE || strpos($value, '!') !== FALSE || strpos($value, '%') !== FALSE) {
--
./handlers/views_handler_field.inc-        $base_path = base_path();
./handlers/views_handler_field.inc-        // Checks whether the path starts with the base_path.
./handlers/views_handler_field.inc:        if (strpos($more_link_path, $base_path) === 0) {
--
./help/alter-exposed-filter.html-<?php
./help/alter-exposed-filter.html-function MODULENAME_form_views_exposed_form_alter(&$form, $form_state) {
./help/alter-exposed-filter.html:  if (strpos($form['#id'], 'volunteer-directory') !== FALSE) {
--
./views.module- */
./views.module-function views_forms($form_id, $args) {
./views.module:  if (strpos($form_id, 'views_form_') === 0) {
--
./views.module-    // has the rightmost extension removed, but there might still be more,
./views.module-    // such as with .tpl.php, which still has .tpl in $template at this point.
./views.module:    if (($pos = strpos($template, '.')) !== FALSE) {
--
./views.module-      if ($matches) {
./views.module-        foreach ($matches as $match) {
./views.module:          $file = substr($match, 0, strpos($match, '.'));
--
./views.module-    $output = $var ? 'TRUE' : 'FALSE';
./views.module-  }
./views.module:  elseif (is_string($var) && strpos($var, "\n") !== FALSE) {

Passing PHP 8.2 tests, please review.

I don't like some of the logic used, so how about this? Hopefully I didn't bork anything.

The last submitted patch, 8: views-n3336764-8.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Let's see if this fixes the mistake in #8.

@DamianMcKenna, patch 6 passes 100% of PHP 8.2 tests, patch 10 has 1 failure on PHP 8.2

hmm, I'll have another look

I'm expecting patch 13 to pass PHP 8.2 tests

@DamienMcKenna, I took some of your suggested changes, but not the ones that seemed to change the logic flow. Patch 13 passes PHP 8.2 however patch 10 and patch 8 do not.

I suggest we move forward with patch 13.

Agreed - I obviously fumbled something.

The only additional tweak I would suggest is $class = $this->options['row_class']; - that should verify $this->options['row_class'] exists before using it.

Ok try this patch and interdiff.

very strange, with that said, I think it's safe for us to use patch 13 instead of patch 18. Previously there was never a need to do an isset on the array for the options.

The tests pass for 8.2 but not for 8.1?!? What's even going on here?!?

ya very strange

ok go with Patch 18, we'll figure out the other issue later, it's the same in HEAD

I've re-queued PHP 8.2 on patch 13 and patch 18, php 8.1 tests ran a few times, same result.

RTBC, deal with 8.1 later.

I'm using PHP 8.0 and PHP 8.2 currently so this is good for me

there must be a bug in PHP 8.1 it'self. PHP 8.1.16 was released today, perhaps drupal ci needs to upgrade.

https://www.php.net/ChangeLog-8.php#8.1.16

Included in todays PHP 8.1.16 release

Core:

...

• Fixed bug GH-10072 (PHP crashes when execute_ex is overridden and a __call trampoline is used from internal code).
• Fix GH-10251 (Assertion `(flag & (1<<3)) == 0' failed).
• Fix wrong comparison in block optimisation pass after opcode update.

...
plus more

Good insights, hopefully drupalci will be upgraded soon and we'll have error-free test runs.

Committed. Thanks everyone!

w00t! :) yay !!! :)

Another victory for PHP 8.2

@DamianMcKenna, I'd say this one change warrants a tag and release? 7.x-3.29 ??

I was looking for a release but I guess I can switch to dev for now.

Automatically closed - issue fixed for 2 weeks with no activity.