Skip to content
Snippets Groups Projects

Issue #3401289 Fix linting errors on gitlab CI

5 unresolved threads

Issue #3401289 by mikelutz: Fix linting errors on gitlab CI

Closes #3401289

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
96 106 * A logger instance.
97 107 * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
98 108 * The configuration factory.
109 * @param \Drupal\Core\PageCache\ResponsePolicy\KillSwitch $page_cache_kill_switch
110 * The page cache kill switch.
99 111 */
100 public function __construct(SimplesamlphpAuthManager $simplesaml, SimplesamlphpDrupalAuth $simplesaml_drupalauth, UrlGeneratorInterface $url_generator, RequestStack $request_stack, AccountInterface $account, PathValidatorInterface $path_validator, LoggerInterface $logger, ConfigFactoryInterface $config_factory) {
  • Options here, We could add a CR and finish this off as a proper deprecation (needs the CR link set in the deprecation notice), we could make the constructor changes without a deprecation notice as this constructor shouldn't be called directly (thus breaking any custom extensions in the wild) Or we could just leave it alone and add the @phpstan-ignore directly on the original \Drupal call.

  • Please register or sign in to reply
  • Michael Lutz
  • 128 * simplesamlphp_auth:5.0.0. Use loginDirectlyWithExternalIdP instead.
    129 * @see https://www.drupal.org/project/simplesamlphp_auth/issues/3401289
    126 130 */
    131 // @codingStandardsIgnoreLine
    127 132 public function login_directly_with_external_IdP(RequestEvent $event) {
    133 @trigger_error(__METHOD__ . '() is deprecated in simplesamplphp_auth:4.1.0 and is removed from simplesamlphp_auth:5.0.0. Use loginDirectlyWithExternalIdP instead. See https://www.drupal.org/project/simplesamlphp_auth/issues/3401289', E_USER_DEPRECATED);
    134 $this->loginDirectlyWithExternalIdP($event);
    135 }
    136
    137 /**
    138 * Redirect anonymous users to the external IdP from the Drupal login page.
    139 *
    140 * @param \Symfony\Component\HttpKernel\Event\RequestEvent $event
    141 * The subscribed event.
    142 */
    143 public function loginDirectlyWithExternalIdP(RequestEvent $event) {
    • Here, we could make a proper CR and keep this deprecation so we could safely remove the original method later, we could simply rename the method with no deprecation (since no one should be calling this method directly, though someone may have extended this event subscriber somewhere in custom code) or we could @codingStandardIgnoreLine here and just leave it be.

    • Please register or sign in to reply
  • Michael Lutz
  • 218 219 $account = $this->externalauth->register($authname, 'simplesamlphp_auth');
    219 220 }
    220 221 catch (\Exception $ex) {
    221 watchdog_exception('simplesamlphp_auth', $ex);
    222 // @todo remove when Drupal 10.0 is no longer supported.
    • Currently I'm setting this up to clear the phpstan deprecation error while still keeping compatibility with 9.5 and 10.0, but with D9 EOL and D10.0 EOL in another month, we could just switch to Error::logException here and bump the drupal compatibility to minimum 10.1 going forward.

    • Please register or sign in to reply
  • I think I flagged all the non-obvious changes that you may want to handle in a different way, @berdir. Just let me know how you want me to finish those up, if you want this mergeable.

  • Michael Lutz added 1 commit

    added 1 commit

    Compare with previous version

  • Michael Lutz
  • 205 205 ->will($this->returnValue($attributes));
    206 206
    207 207 // Set up a mock for SimplesamlphpDrupalAuth class,
    208 // mocking getUserIdforAuthname() and externalRegister() methods.
    208 // mocking externalRegister() method.
    209 209 $simplesaml_drupalauth = $this->getMockBuilder('Drupal\simplesamlphp_auth\Service\SimplesamlphpDrupalAuth')
    210 ->setMethods(['externalLoginFinalize'])
  • 8 10 use Drupal\Core\Path\PathValidatorInterface;
    9 11 use Drupal\Core\Routing\UrlGeneratorInterface;
    10 12 use Drupal\Core\Session\AccountInterface;
    11 13 use Drupal\simplesamlphp_auth\Service\SimplesamlphpAuthManager;
    12 14 use Drupal\simplesamlphp_auth\Service\SimplesamlphpDrupalAuth;
    13 15 use Psr\Log\LoggerInterface;
    16 use Symfony\Component\DependencyInjection\ContainerInterface;
    14 17 use Symfony\Component\HttpFoundation\RedirectResponse;
    15 18 use Symfony\Component\HttpFoundation\RequestStack;
    16 use Symfony\Component\DependencyInjection\ContainerInterface;
    17 use Drupal\Core\Config\ConfigFactoryInterface;
    18 19
    19 20 /**
    20 21 * Controller routines for simplesamlphp_auth routes.
    22 *
    23 * @phpstan-consistent-constructor
  • Michael, thanks for the changes. Looks like a great example, it's first time I see deprecation added to code base. I think this approach looks good, I would split deprecation to separate issues to be able to create CR.

    And I would create phpstan.neon file to ignore error with static files, but current version is good as well.

  • Please register or sign in to reply
    Loading