Support new array syntax

[olafgrabienski]

After running the Tocbot module through Coder upgrade, I couldn't enable Tocbot on a test site due to the following errors:

ParseError: syntax error, unexpected ''textfield'' (T_CONSTANT_ENCAPSED_STRING), expecting ']' in backdrop_load() (line 87 of /.../modules/tocbot/tocbot.module).

syntax error, unexpected ''textfield'' (T_CONSTANT_ENCAPSED_STRING), expecting ']'

Line 87 is the type textfield related one in this code part:

$form['moduleSettings']['tocbot_extrabodyclass'] =[
  '#type' 'textfield'

Related code from the D7 module:

$form['moduleSettings']['tocbot_extrabodyclass'] = [
  '#type' => 'textfield',

Coder upgrade strips the => between typeand textfield and the comma at the end of the line.

Also, according to @docwilmot in Gitter, Coder upgrade doesnt handle the square brackets syntax yet. At the moment a line like

$form['moduleSettings']['tocbot_extrabodyclass'] = [

has to be converted manually to:

$form['moduleSettings']['tocbot_extrabodyclass'] = array(

Note: The bracket syntax shouldn't be used because it's not available in PHP 5.3 (Backdrop supports 5.3).

[indigoxela]

@olafgrabienski just a note on php compability: if using square bracket array syntax, the module should require php 5.4 as minimum php version (in the .info file). Backdrop core attempts to support 5.3.

[olafgrabienski]

@indigoxela Thanks for your note! In my opinion, also modules should try to support PHP 5.3. Don't know if it's possible but ideally Coder Upgrade would offer to convert square bracket syntax to the other one.

[laryn]

I've run into this too where it messes up square bracket arrays, and it also messes up Classes (removing slashes, e.g. backdrop-contrib/uc_stripe@101a08a )

[rbargerhuff]

Regarding this issue, we are experiencing the same problem. All the => and commas after each entry in an array are being stripped in the converted module/code.

Is this intended behavior? Our modules are 7.4.x compliant and use the array short syntax.

What is the plan moving forward in regards to this critical issue?

[docwilmot]

May be time to look at upgrading the grammar parser library.

[rbargerhuff]

Thank you for responding. Is that directed at the developer, as in my supervisor and I, or is that being directed at the developers of the coder_upgrade project? We are using the latest from branch 1.x-1.x.

[rbargerhuff]

Never mind. We now understand that upgrading to the 7.x-2.x version of the Grammar Parser Library is going to require code adjustments to Coder Upgrade. We are not familiar enough with the project to make these changes ourselves.

[docwilmot]

This was a note to self actually🙂

[rbargerhuff]

I appreciate the work that you put into this. The institution I work for is considering on moving to Backdrop CMS as Drupal 7's EOL gets closer to the horizon. Coder Upgrade is going to be a big part so we again we appreciate the effort and time put into the development of this module.

[rbargerhuff]

@docwilmot Regarding the above, what kind of momentum does this project have as well as Backdrop. Like I said, we are considering moving to Backdrop CMS and part of the process of its consideration is determining how much time the conversion process will take. Coder Upgrade from the looks of it will be a integral part. Unfortunately, our code base, which is primarily custom code, has been upgraded to be compliant with PHP 7.4.x. So if there anyway you could tell us if this one issue, along with other issues, will be addressed in the upcoming weeks, or month, or would it be months? Because if it's the latter, we may need to explore other options. Thank you.

[docwilmot]

@rbargerhuff I consider Code Upgrade a helper module, and I never expect that it will give you good code, and I will never be able to keep up constantly with changing standards. I expect that whatever I do, the developer will have to get into the Coder output and manually correct stuff, even sometimes lots of stuff. Maybe I should put this statement in the README.

Having said that though, I like a challenge, and have essentially copy/pasted the square bracket-fixing code from Grammar Parser and, well, by gosh, it seemed to work!

Have a go, see if you can break it, and let me know what breaks. Thanks for testing. 🙂

[rbargerhuff]

@docwilmot Wow!! Many thanks for putting out fixes to the issues I have submitted and for your candid response to this issue. We greatly appreciate it. This type of response definitely helps with our consideration to move to Backdrop CMS.

We never intended to use Code Upgrade helper module blindly without looking at the code it produced. But we were concerned when we saw general PHP coding standards being altered, mishandled, or removed. Our workflow will always include reviewing the diff/patch produced by the helper module before doing any kind of final commit.

We attempted to convert our test module. This module is very basic that contains code that we need to make sure converts properly and the results were half and half as far as successful code is concerned. We are concerned with some of the output though:

Concern (code removed):
-  $logDir = config_get('online_admin.settings', 'online_admin_config')['log_directory'];
+  $logDir = config_get('online_admin.settings', 'online_admin_config') [];

---

Seems to affected only when the type array is present.

+function online_admin_form_rules_ui_add_edit_element_callback(? array &$form, ? array &$formState, ?string $formId) {

---

Concern (code removed)

-          'data' => ['timefield' => $js_settings],
+          'data' => ['timefield' =>],

---

Concern (code missing)

+    form_set_value($element, ['value' =>], $formState);

---

Concern (code removed)

   // Add submit button
   $form['submit'] = [
     '#type' => 'submit',
     '#value' => t('Save configuration'),
-    '#submit' => ['online_admin_static_paths_form_submit'],
+    '#submit' => [],
   ];
	 
	 // Add validation
-  $form['#validate'] = ['online_admin_static_paths_form_validate'];
+  $form['#validate'] = [];

Please let me know what you think and thanks again. Cheers!

Edit: We believe that a lot of these issues stem from the array short syntax handling.

[rbargerhuff]

@docwilmot A lot of the issues above are due to the support of array short syntax. We were able to solve the majority of the issues with the following fix:

diff --git a/grammar_parser/reader.inc b/grammar_parser/reader.inc
index 627b06c..cae8e96 100644
--- a/grammar_parser/reader.inc
+++ b/grammar_parser/reader.inc
@@ -2218,7 +2218,7 @@ class PGPReader extends PGPParser {
             $expression->values->insertLast($temp, 'value');
             $temp = NULL;
           }
-          elseif ($type == ')') {
+          elseif ($type == ')' || $type == ']') {
             /*
              * The last item in the value expression can be an expression
              * (e.g. inlineelse) in which case we need to look inside the

This seems to address the issue with array short syntax.

Please let us know what you think.

[bugfolder]

Fixed by #41.