Problem/Motivation

We skip a number of deprecations in .deprecation-ignore.txt but some of these are outdated or could be tightened up.

For example:

%The "Symfony\\Component\\Validator\\Context\\ExecutionContextInterface::.*\(\)" method is considered internal Used by the validator engine\. (Should not be called by user\W+code\. )?It may change without further notice\. You should not extend it from "[^"]+"\.%

This doesn't seem to trigger any more.

%Method "Twig\\NodeVisitor\\NodeVisitorInterface::[^"]+" might add "[^"]+" as a native return type declaration in the future. Do the same in (child class|implementation) "[^"]+" now to avoid errors or add an explicit @return annotation to suppress this message%

Any implementation in contrib would also skip this deprecation - but we want contrib to add return types now, so core can add them in the future when Twig also does. We can tighten this up so it only triggers on the core implementation.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3523383

Command icon 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:

Comments

longwave created an issue. See original summary.

longwave’s picture

Title: [meta] Remove or tighten deprecation skips » Tidy up and tighten deprecation skips
Category: Plan » Task

I was wrong about Select::hasAllTags() and friends, but we can tighten up many of the other skips and remove a few - two where our implementations have been removed and one where we don't need to use a custom stub in a test.

longwave’s picture

Status: Active » Needs review
longwave’s picture

Notes for reviewers are in the MR.

longwave’s picture

Issue summary: View changes
mondrake’s picture

This tells me it would be great to find a way to report ignored deprecations that are no longer necessary… like PHPStan does. Will be hard though as PHPUnit tests are run in parallel jobs in Drupal and results would need some sort of consolidation.

mondrake’s picture

Status: Needs review » Needs work
Issue tags: +Couple of nits inline, +NW just to have a look
mondrake’s picture

Issue tags: -Couple of nits inline, -NW just to have a look

Oh my... entered comment in the issue tags field...

Couple of nits inline, NW just to have a look

longwave’s picture

Status: Needs work » Needs review

Thanks for reviewing. I split out the PHPUnit one but think we should leave DrupalSelenium2Driver for now.

mondrake’s picture

Status: Needs review » Reviewed & tested by the community

Seems all good to me, we are narrowing the ignores so FTW.

longwave’s picture

Issue tags: +11.2.0 release target

Thanks again - let's try to get this into 11.2 so contrib actually finds out about some of these that may have previously been skipped.

quietone’s picture

Everything looks in order here and the followup was made to find a better way. I read the MR and the changes look correct to me as well.

  • larowlan committed 69852494 on 11.2.x
    Issue #3523383 by longwave, mondrake, quietone: Tidy up and tighten...

  • larowlan committed 90461b18 on 11.x
    Issue #3523383 by longwave, mondrake, quietone: Tidy up and tighten...
larowlan’s picture

Version: 11.x-dev » 11.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 11.x and backported to 11.2.x thanks folks

Status: Fixed » Closed (fixed)

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