NodeViewBuilder should use static where possible, subclassing is currently too painful

Good call!

I'm not really sure if I fully understand the issue. Is only this what is required?

tagging.

Ok, doing a deeper review on this.

That's all I found. Not really experienced with PHP OOP, so not sure if it's the correct syntax to use, also I might be missing something else.

Trying to fix the errors.

Ok, I think that's it.

The function drupal_render_cache_generate_placeholder ( https://api.drupal.org/api/drupal/core%21includes%21common.inc/function/... ) checks if the $callback is a string or a "callable variable". The is_callable function ( http://php.net/manual/en/function.is-callable.php ) allows passing an object ($this) and a function name. So, that should work.

I'm not sure if afterwards, the $callback function is used in a different way, that only allows string variables. I thought also about using the get_class function, but that has a problem, as it only returns the class (NodeViewBuilder) and it should return the namespace (\Drupal\node\NodeViewBuilder).

I'm not sure why the tests are failing, I'm trying to run those tests on a clean and updated D8 installation and the tests are not passing. I've tried it in a couple other laptops and still fails.

I'm working on this.

Ok, I've talked with tstoeckler and he told me that even if $callback = array($this, 'function'); should work, it usually does some weird things when working with caches.

So I replaced them with the get_class as mentioned on #13 because it seems that it returns a fully qualified domain name.

I also found that running some specific tests from the shell doesn't work and they work from the web interface. I haven't found the reason why, but I would love to know what's going on.

Ok, next try. We can't use get_class($this) on the renderLink function because it's a static function and $this will not be defined. We can't use get_class because it returns the parent class (NodeViewBuilder). I'm using get_called_class() as it should return the child class, but locally some tests are failing.

Is there any function like getClass() to be able to do some late static binding? http://php.net/manual/en/language.oop5.late-static-bindings.php

Setting to needs work for using the same approach for both cases.

Thanks for working on this! :)

New patch. Is there any other class where this is happening?

During DrupalCon I found the CommentViewBuilder ( https://www.drupal.org/node/2348547 ) and we created the issue.

Setting to needs work for using the same approach for both cases.

@WimLeers: Looks like the new patch is doing that now? static:: is fine.

It would be great if this pattern is part of some documentation of how to use the render cache patterns.

Sure, but where would we document something like this? It's not something strongly related to render caching, it's really a general "static method invocations when writing a class that may be subclassed" kind of problem, which is something that applies to Drupal 8 in general, since we now are so heavily OOPified.
In any case: I'm open to suggestions :)

Committed 1688321 and pushed to 8.0.x. Thanks!