Hi,

It looks like this module is throwing notices on PHP 7.4.
Are there any plans to support 7.4?

Many thanks,

Russell

CommentFileSizeAuthor
#15 reference_option_limit-php_74_support-15.patch1.51 KBrussellb
#10 reference_option_limit-php_74_support-10.patch1.6 KBrussellb
#8 reference_option_limit-php_74_support-8.patch1.79 KBrussellb
#4 reference_option_limit_php_74_support.patch1.01 KBrussellb

Comments

russellb created an issue. See original summary.

joachim’s picture

joachim commented

It's not something I have time to work on myself, but happy to look at a patch :)

Webbeh’s picture

Webbeh
he/him/his
Location Georgia, USA
commented
Title: PHP 7.4 » PHP 7.4 support
russellb’s picture

russellb commented
StatusFileSize
new reference_option_limit_php_74_support.patch1.01 KB
Webbeh’s picture

Webbeh
he/him/his
Location Georgia, USA
commented
Status: Active » Needs review

Given #4, for review?

russellb’s picture

russellb commented
Version: 7.x-1.6 » 7.x-1.x-dev
russellb’s picture

russellb commented

I'm not sure what's going on with this profile2 test failing. This is working for me. I've deployed this fix to a fairly busy site, I'll report back when it's been live for a bit.

russellb’s picture

russellb commented
StatusFileSize
new reference_option_limit-php_74_support-8.patch1.79 KB

I found another one in testing.

joachim’s picture

joachim commented
Status: Needs review » Needs work 
  1. +++ b/reference_option_limit.module
@@ -417,9 +417,14 @@ function reference_option_limit_field_attach_form($entity_type, $entity, &$form,
+        // Resolve warning
+        // https://www.drupal.org/project/reference_option_limit/issues/3291112

    Code comments shouldn't refer to fixed bugs. There is no point in code being a historical record, it should describe itself as it is now. For the history, there is git. 

  2. +++ b/reference_option_limit.module
@@ -417,9 +417,14 @@ function reference_option_limit_field_attach_form($entity_type, $entity, &$form,
+        if ($values_field_matching) {

    In what cases is this not an array?

    It got set with:

    $values_field_matching = field_get_items($entity_type, $entity, $field_name_matching);

    Why would that be NULL? 

  3. +++ b/reference_option_limit.module
@@ -516,7 +521,12 @@ function reference_option_limit_field_attach_form($entity_type, $entity, &$form,
+    $required = false;

    false should be in caps. 

  4. +++ b/reference_option_limit.module
@@ -516,7 +521,12 @@ function reference_option_limit_field_attach_form($entity_type, $entity, &$form,
+    if (array_key_exists( LANGUAGE_NONE, $element_limited)) $required = $element_limited[LANGUAGE_NONE]['#required'];

    Don't do single-line ifs; use {}.

    Also, could the whole of this be done with the ?? operator instead?

russellb’s picture

russellb commented
StatusFileSize
new reference_option_limit-php_74_support-10.patch1.6 KB

re 2. - in this case I am seeing $values_field_matching FALSE when field_get_items doesn't find any items.

russellb’s picture

russellb commented
Status: Needs work » Needs review

I have actioned code style requests 1. 3. & 4. in the new patch above #10.
Many thanks,
Russell

russellb’s picture

russellb commented

I've had #10 running on a live site for a while now.

Looks good.

joachim’s picture

joachim commented

> in this case I am seeing $values_field_matching FALSE when field_get_items doesn't find any items

Ah yes, I've checked the docs. (Weird choice to not have it return [] so it can always be iterated over!)

+++ b/reference_option_limit.module
@@ -516,7 +519,12 @@ function reference_option_limit_field_attach_form($entity_type, $entity, &$form,
+    $required = FALSE;
+    if (array_key_exists( LANGUAGE_NONE, $element_limited)) {
...
+    }

Could you do this with $required = $element_limited[LANGUAGE_NONE]['#required'] ?? FALSE; instead?

russellb’s picture

russellb commented

Yes I guess we don't need to support 5.6 any more. I'll re-roll with ??

russellb’s picture

russellb commented
StatusFileSize
new reference_option_limit-php_74_support-15.patch1.51 KB

Re-rolled with ?? FALSE

  • joachim committed fc5fe37 on 7.x-1.x authored by russellb 
    Issue #3291112 by russellb: Fixed errors on PHP 7.4.
joachim’s picture

joachim commented
Status: Needs review » Fixed

Thanks!

I'll make a new release.

Status: Fixed » Closed (fixed)

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

solideogloria’s picture

solideogloria commented

Could we get a commit of the fix in #3011848: PHP 7.2: Parameter must be an array or an object that implements Countable? It's also needed for PHP compatibility, as of PHP 7.2