Typehint Views field handler methods with ResultRow [#2034979]

Updated: Comment #13

Problem/Motivation

To provide a better interface, field handler methods should typehint the result row values (Drupal\views\ResultRow).

Proposed resolution

Typehinting params of field handler methods using Drupal\views\ResultRow and adjust docblocks appropriately:

render()
advancedRender()
renderLink()
getValue()
theme()

Remaining tasks

None.

User interface changes

None.

API changes

None.

#2034947: [Change notice] Change view results to use a classed object

Original report by @damiankloip

After #2034947: [Change notice] Change view results to use a classed object is in, we can typehint all the places this is used. Field handler methods render, advancedRender, render_link, getValue, maybe more...

Comment	File	Size	Author
#11	2034979-11-views_resultrow_typehint.patch	27 KB	derhasi
#11
#11	interdiff.txt	1.19 KB	derhasi
#8	2034979-8-views_resultrow_typehint.patch	27 KB	derhasi
#8
#8	interdiff.txt	19.55 KB	derhasi
#4	2034979-4-views_resultrow_typehint.patch	25.6 KB	derhasi
#4

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

damiankloip CreditAttribution: damiankloip commented 13 September 2013 at 09:28

Status:

Postponed

» Active

Good to go

Comment #2

Crell CreditAttribution: Crell commented 13 September 2013 at 22:29

Issue tags:

+Needs issue summary update

This needs more precise directions for a novice issue. (Hello from Drupal Camp Costa Rica! Pura Vida!)

Comment #3

derhasi CreditAttribution: derhasi commented 27 September 2013 at 10:48

Assigned:	Unassigned	» derhasi
Issue tags:	-Needs issue summary update

Applied summary template and now going to work on it.

Comment #4

derhasi CreditAttribution: derhasi commented 27 September 2013 at 14:33

Status:

Active

» Needs review

File	Size
2034979-4-views_resultrow_typehint.patch	25.6 KB

Attached is the patch for typehinting ResultRow in all views field plugins. It's quite a big patch, as there where a lot of docblocks not properly provided.

Comment #5

derhasi CreditAttribution: derhasi commented 27 September 2013 at 14:33

Assigned:

derhasi

» Unassigned

Comment #5.0

derhasi CreditAttribution: derhasi commented 27 September 2013 at 14:33

Issue summary:

View changes

Applying summary template

Comment #6

dawehner

German

CreditAttribution: dawehner commented 27 September 2013 at 23:06

Wow this was a big patch. Thank you for working on that!!

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/Link.php
@@ -51,6 +51,17 @@ public function render(ResultRow $values) {
   protected function renderLink($data, ResultRow $values) {

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkApprove.php
@@ -24,6 +24,17 @@ public function access() {
   protected function renderLink($data, ResultRow $values) {

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkDelete.php
@@ -24,6 +24,17 @@ public function access() {
   protected function renderLink($data, ResultRow $values) {

It would maybe make sense to typehint CommentInterface on the common base class (the link handler for comments) so you get better autocompletion in this single function but yeah this is maybe out of scope.

+++ b/core/modules/node/lib/Drupal/node/Plugin/views/field/Link.php
@@ -55,6 +55,17 @@ public function render(ResultRow $values) {
+   * @param ResultRow $values

+++ b/core/modules/node/lib/Drupal/node/Plugin/views/field/LinkEdit.php
@@ -21,7 +21,15 @@
+   * @param ResultRow $values

Let's use the full namespace here as well.

Comment #7

damiankloip CreditAttribution: damiankloip commented 28 September 2013 at 11:33

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkDelete.php
@@ -24,6 +24,17 @@ public function access() {
+   *   Returns string for the link text.

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkEdit.php
@@ -37,6 +37,17 @@ public function buildOptionsForm(&$form, &$form_state) {
+   *   Returns string for the link text.

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkReply.php
@@ -24,6 +24,17 @@ public function access() {
+   *   Returns string for the link text.

'Returns a string for the link text'

+++ b/core/modules/contextual/lib/Drupal/contextual/Plugin/views/field/ContextualLinks.php
@@ -68,6 +68,9 @@ public function preRender(&$values) {
+   *   The values retrieved from the database.

Can this be more general?

Just the values from a row of view results or something? (I have nothing that good right now! :))

Otherwise, generally looking pretty good.

Comment #8

derhasi CreditAttribution: derhasi commented 30 September 2013 at 09:04

File	Size
interdiff.txt	19.55 KB
2034979-8-views_resultrow_typehint.patch	27 KB

It would maybe make sense to typehint CommentInterface on the common base class (the link handler for comments) so you get better autocompletion in this single function but yeah this is maybe out of scope.

@dawehner, yes, should make sense, but I think it is out of scope for this issue.

dawehner, damiankloip: I implemented your proposed changes (see interdiff.txt).

Comment #9

dawehner

German

CreditAttribution: dawehner commented 30 September 2013 at 09:51

Status:

Needs review

» Reviewed & tested by the community

Good work!

Comment #10

damiankloip CreditAttribution: damiankloip commented 30 September 2013 at 10:16

Status:

Reviewed & tested by the community

» Needs work

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/Counter.php
@@ -8,6 +8,8 @@
+

Extra line.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php
@@ -1591,8 +1591,14 @@ protected function documentSelfTokens(&$tokens) { }
+   * @param \Drupal\views\ResultRow

No variable name.

Sorry.. :/

*hides*