Problem/Motivation

In Drupal core, the best practice is to use new static in general (instead of new self). Previously, using new static was detected as a security issue.

We had a problem extending the class \Drupal\flysystem_s3\Flysystem\S3.
Due to the new self usage, it would now be necessary to (over)write the create() method, which would be an identical copy of the parent's class.

Steps to reproduce

Try to extend the S3 class; as a result, you will see that an instance of \Drupal\flysystem_s3\Flysystem\S3 will always be returned unless you copy-paste the create method.

Proposed resolution

Adhere to Drupal core best practices, and use return new static().

Issue fork flysystem_s3-3446367

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:

About issue forks

Comments

Richard Hoogstad created an issue. See original summary.

Richard Hoogstad opened merge request !18

watergate’s picture

watergate commented
Issue summary: View changes
Status: Active » Needs review
Related issues: +#3409514: Fix PHPCS and PHPStan issues reported by Gitlab CI

In addition, this best practice is instructed to PHPStan via https://git.drupalcode.org/project/drupal/-/blob/11.x/core/phpstan.neon....

adam-vessey’s picture

adam-vessey commented
Status: Needs review » Reviewed & tested by the community

After updating `flysystem_s3`, one of our custom modules which extends Drupal\flysystem_s3\Flysystem\S3 starting throwing some of the "deprecated: dynamic property" warnings due trying to set against an instance of the upstream class instead of the static bound class.

MR from #2 (at e9995f3f8a1db1f2aef0d37ccb7dce0a7184406b) seems to do the trick to get rid of the warning (and restore the behaviours from the extending code).

richardcapricorn closed merge request !18