Skip to content
Snippets Groups Projects

Issue #3359649 by arnested, aduthois, cilefen, Manuel.loth, smustgrave:...

Open Issue #3359649 by arnested, aduthois, cilefen, Manuel.loth, smustgrave:...
issue/drupal-3359649:3359649-deprecated-function-explode-11.x into 11.x
1 unresolved thread
Open Arne Jørgensen requested to merge issue/drupal-3359649:3359649-deprecated-function-explode-11.x into 11.x
1 unresolved thread

Issue #3359649 by arnested, aduthois, cilefen, Manuel.loth, smustgrave: Deprecated function: explode(): Passing null to parameter #2 ($string) of type string is deprecated in Drupal\serialization\EventSubscriber\UserRouteAlterSubscriber->onRoutingAlterAddFormats()

Closes #3359649

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Arne Jørgensen added 62 commits

    added 62 commits

    Compare with previous version

  • Arne Jørgensen resolved all threads

    resolved all threads

  • catch @catch started a thread on an old version of the diff
    • core/modules/serialization/src/EventSubscriber/UserRouteAlterSubscriber.php
    52 52 $routes = $event->getRouteCollection();
    53 53 foreach ($route_names as $route_name) {
    54 54 if ($route = $routes->get($route_name)) {
    55 $formats = explode('|', $route->getRequirement('_format'));
    55 $formats = array_filter(explode('|', $route->getRequirement('_format') ?? ''));
    • C
      catch @catch
      Maintainer

      why an empty string rather than an empty array here?

    • Arne Jørgensen
      Arne Jørgensen @arnested
      Author

      We need a string to be able to use explode() on it. The fallback is if getRequirement() returns null instead of a string (e.g., "json|xml"). Explode on an empty string will return an array with one, empty element — this is where the array_filter() comes into use.

    • A
      Alex Pott @alexpott
      Maintainer

      The use of array_filter() and magically removing anything the PHP considers falsey concerns me. I think the array_filter() should use an arrow function to only remove empty strings. Consider https://3v4l.org/kNAfK

      Suggested change
      55 $formats = array_filter(explode('|', $route->getRequirement('_format') ?? ''));
      55 $formats = array_filter(explode('|', $route->getRequirement('_format') ?? ''), fn ($value) => $value !== '');

      But actually we can ignore all of this if we change the code to:

            if (($route = $routes->get($route_name)) && $route->hasRequirement('_format')) {
        $formats = explode('|', $route->getRequirement('_format'));
        $formats = array_unique(array_merge($formats, $this->serializerFormats));
        $route->setRequirement('_format', implode('|', $formats));
      }

      no?

    • shalini jha changed this line in version 7 of the diff

      changed this line in version 7 of the diff

    • Please register or sign in to reply
  • Arne Jørgensen added 52 commits

    added 52 commits

    Compare with previous version

  • shalini jha added 1 commit

    added 1 commit

    Compare with previous version

  • shalini jha added 1 commit

    added 1 commit

    • af9b020a - Issue #3359649: Fixed pipeline failure by updating test assertions based on route alteration logic

    Compare with previous version

  • shalini jha added 1 commit

    added 1 commit

    • 3447a039 - Update UserRouteAlterSubscriber.php

    Compare with previous version

  • Arne Jørgensen added 186 commits

    added 186 commits

    Compare with previous version

    Toggle commit list
  • Alex Pott added 1 commit

    added 1 commit

    • 6b6402dd - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • Alex Pott added 1 commit

    added 1 commit

    • 5df518a4 - Less tests and use event priority correctly

    Compare with previous version

    • Please register or sign in to reply