2.x - 🥺 final __constructor really takes the jelly out of my doughnut #2813
Description
Please don't do this to the end users (me). I'm in the midst of using v2rc1 and came across this restriction - which seriously limits my ability to provide services to the class unless I write a bunch of setter methods or create an init function that I have to call every time the class is instantiated (both options aren't great). This removes any possibility of DI, autowiring, and I'm sure a lot of other things I can't think of right now.
In addition, the logic for this decision seems flawed: the build() method also shares the same concern you're discussing in pull 2660 about the constructor - it's just not throwing a warning because PHPStan only looks at the constructor for this potential issue. If I override the build() method with a different signature, it will break just as many things as overriding the __constructor() would. Marking both the build and __constructor as final would basically make initializing the class with additional setup code incredibly messy, if not impossible - but it doesn't make sense to mark one without marking the other for this PHPStan warning.
How do you deal with this PHPStan warning, then? Simply don't define a constructor in the Timber\Post class. The library is already structured exactly how it should be to allow for this. By not defining it, you allow it to be defined by child classes, and the PHPStan warning goes away.
To protect against the issue PHPStan is warning about, but for the build method, define an Interface based on what PostFactory needs. Also protect yourself from improperly written child classes by moving some of the import logic around and marking the import() method as final.
// Timber\PostInterface.php
interface PostInterface {
public static function build(WP_Post $post) : PostInterface;
public function import(WP_Post $post) : void;
}
// in PostFactory.php
class PostFactory {
//...
// declare the interface as the return type, rather than the concrete instance
protected function build(WP_Post $post) : PostInterface
{
$class = $this->get_post_class($post);
if ( ! is_a($class, Timber\PostInterface::class) ) {
throw new Exception("$class must implement Timber\PostInterface");
}
$instance = $class::build($post);
$instance->import($post);
return $instance;
}
//...
}
// in Post.php
class Post extends CoreEntity implements DatedInterface, Setupable, PostInterface {
//...
// new build() method does not call import, since it's now called from PostFactory
// build() is only responsible for returning an instance
public static function build(WP_Post $wp_post) : PostInterface {
$post = new static();
// No longer call import from here
// $post->import($data);
return $post;
}
// mark import() as final
final public function import(WP_Post $wp_post) : void {
// Move post data resolver logic here - which makes more sense anyway,
// and protects `build()` from being overwritten by child classes in a poor way:
$this->id = $wp_post->ID;
$this->ID = $wp_post->ID;
$this->wp_object = $wp_post;
$data = \get_object_vars($wp_post);
$data = $this->get_info($data);
/**
* Filters the imported post data.
*
* Used internally for previews.
*
* @since 2.0.0
* @see Timber::init()
* @param array $data An array of post data to import.
* @param \Timber\Post $post The Timber post instance.
*/
$data = \apply_filters('timber/post/import_data', $data, $post);
parent::import($data);
}
//...
}Now if the user wants to extend and setup their own constructor dependencies - it's easy, and reliable:
// MyPost.php
class MyPost extends Timer\Post implements Timber\PostInterface {
protected ExampleApiService $api;
// This can now be overridden, and the method signature from PostInterface is enforced
public static build(WP_Post $wp_post) : Timber\PostInterface {
return my_di_container()->resolve(self::class, ['post' => $wp_post]);
}
// ExampleApiService resolved via a userland DI container via autowiring (for example)
public function __construct(WP_Post $post, ExampleApiService $api) {
$this->api = $api;
//..
}
}Pretty pretty please with sugar on top, please don't kill this PHPStan mosquito with a final __constructor() cannon.