comments in getModuleInfo break installation

[BernhardBaumrock]

Short description of the issue

If you add regular and valid PHP comments in requires array of getModuleInfo of a module the installation via classname breaks!

This was my commit:

The problem is the portion // also set in composer.json...

And this is what @gebeer reported when he tried to install RockMigrations via PW Backend by name:

DEBUG MODE BACKTRACE ($config->debug == true):
#0 /var/www/html/wire/core/Modules.php(688): ProcessWire\ProcessPageEditImageSelect->init()
#1 /var/www/html/wire/core/Modules.php(1455): ProcessWire\Modules->initModule(Object(ProcessWire\ProcessPageEditImageSelect), Array)
#2 /var/www/html/wire/core/Modules.php(1262): ProcessWire\Modules->getModule('ProcessPageEdit...')
#3 /var/www/html/wire/core/Modules.php(1712): ProcessWire\Modules->get('ProcessPageEdit...')
#4 /var/www/html/wire/core/WireArray.php(1789): ProcessWire\Modules->find('alsosetincompos...')
#5 /var/www/html/wire/core/WireArray.php(583): ProcessWire\WireArray->findOne('alsosetincompos...')
#6 /var/www/html/wire/core/Modules.php(1913): ProcessWire\WireArray->get('alsosetincompos...')
#7 /var/www/html/wire/modules/Process/ProcessModule/ProcessModule.module(1166): ProcessWire\Modules->isInstalled('alsosetincompos...')
#8 /var/www/html/wire/core/Wire.php(419): ProcessWire\ProcessModule->___buildDownloadConfirmForm(Array, false)
#9 /var/www/html/wire/core/WireHooks.php(952): ProcessWire\Wire->_callMethod('___buildDownloa...', Array)
#10 /var/www/html/wire/core/Wire.php(484): ProcessWire\WireHooks->runHooks(Object(ProcessWire\ProcessModule), 'buildDownloadCo...', Array)
#11 /var/www/html/wire/modules/Process/ProcessModule/ProcessModule.module(1080): ProcessWire\Wire->__call('buildDownloadCo...', Array)
#12 /var/www/html/wire/modules/Process/ProcessModule/ProcessModule.module(340): ProcessWire\ProcessModule->downloadConfirm('RockMigrations')
#13 /var/www/html/wire/core/Wire.php(413): ProcessWire\ProcessModule->___execute()
#14 /var/www/html/wire/core/WireHooks.php(952): ProcessWire\Wire->_callMethod('___execute', Array)
#15 /var/www/html/wire/core/Wire.php(484): ProcessWire\WireHooks->runHooks(Object(ProcessWire\ProcessModule), 'execute', Array)
#16 /var/www/html/wire/core/ProcessController.php(350): ProcessWire\Wire->__call('execute', Array)
#17 /var/www/html/wire/core/Wire.php(413): ProcessWire\ProcessController->___execute()
#18 /var/www/html/wire/core/WireHooks.php(952): ProcessWire\Wire->_callMethod('___execute', Array)
#19 /var/www/html/wire/core/Wire.php(484): ProcessWire\WireHooks->runHooks(Object(ProcessWire\ProcessController), 'execute', Array)
#20 /var/www/html/wire/core/admin.php(160): ProcessWire\Wire->__call('execute', Array)
#21 /var/www/html/site/templates/admin.php(16): require('/var/www/html/w...')
#22 /var/www/html/wire/core/TemplateFile.php(328): require('/var/www/html/s...')
#23 /var/www/html/wire/core/Wire.php(413): ProcessWire\TemplateFile->___render()
#24 /var/www/html/wire/core/WireHooks.php(952): ProcessWire\Wire->_callMethod('___render', Array)
#25 /var/www/html/wire/core/Wire.php(484): ProcessWire\WireHooks->runHooks(Object(ProcessWire\TemplateFile), 'render', Array)
#26 /var/www/html/wire/modules/PageRender.module(575): ProcessWire\Wire->__call('render', Array)
#27 /var/www/html/wire/core/Wire.php(416): ProcessWire\PageRender->___renderPage(Object(ProcessWire\HookEvent))
#28 /var/www/html/wire/core/WireHooks.php(952): ProcessWire\Wire->_callMethod('___renderPage', Array)
#29 /var/www/html/wire/core/Wire.php(484): ProcessWire\WireHooks->runHooks(Object(ProcessWire\PageRender), 'renderPage', Array)
#30 /var/www/html/wire/core/WireHooks.php(1060): ProcessWire\Wire->__call('renderPage', Array)
#31 /var/www/html/wire/core/Wire.php(484): ProcessWire\WireHooks->runHooks(Object(ProcessWire\Page), 'render', Array)
#32 /var/www/html/wire/modules/Process/ProcessPageView.module(184): ProcessWire\Wire->__call('render', Array)
#33 /var/www/html/wire/modules/Process/ProcessPageView.module(114): ProcessWire\ProcessPageView->renderPage(Object(ProcessWire\Page), Object(ProcessWire\PagesRequest))
#34 /var/www/html/wire/core/Wire.php(416): ProcessWire\ProcessPageView->___execute(true)
#35 /var/www/html/wire/core/WireHooks.php(952): ProcessWire\Wire->_callMethod('___execute', Array)
#36 /var/www/html/wire/core/Wire.php(484): ProcessWire\WireHooks->runHooks(Object(ProcessWire\ProcessPageView), 'execute', Array)
#37 /var/www/html/index.php(55): ProcessWire\Wire->__call('execute', Array)
#38 {main}

And this is the version that works properly (with the comment above the array items): https://github.com/baumrock/RockMigrations/blob/1e02ecf406afa2160e5d2ed1750099e16554a8ab/RockMigrations.module.php#L63-L80

Expected behavior

The module should install properly. PHP comments should not be an issue!

Actual behavior

Comments need to be added above the requires array. If you put the comment beside an item it breaks. But you won't realise until you try to install the module manually yourself.

That's especially a problem for me, because I always install my modules as git submodule via git submodule add - so I don't recognise that there is a problem!

[ryancramerdesign]

@BernhardBaumrock For safety, module info is not executed as PHP but instead parsed as text. Consider it like JSON, but just as a PHP array. Leave out comments, variables, concatenation, or any kind of logic code, in your module info, as it'll interfere with the parsing of it.

[BernhardBaumrock]

Thx @ryancramerdesign hm... that's not so good... I have this in many of my modules now to get fully automated git releases, changelog, tags and version numbers following semver: https://github.com/baumrock/RockFrontend/blob/1cd107abe6c41f6a8cadcf8690abc6825959d2c1/RockFrontend.info.php#L7

See this forum post for background: https://processwire.com/talk/topic/28235-how-to-get-fully-automated-releases-tags-changelog-and-version-numbers-for-your-module/

Actually I wanted to create a request so that PW reads the version number automatically from package.json if that file is present and if the module-info does not contain a version number or uses a version number of "package.json" or such.

Now I'm wondering if what I do is safe to do? I did not see any issues in any of my modules until now. If it is a problem then it would be great to get a solution for this. It's really a great workflow for me having everything numbered automatically and having all the tagging and changelogs in place!

[ryancramerdesign]

@BernhardBaumrock ProcessWire itself will be able to identify the version number no problem, since it executes the PHP. But the modules directory is what parses it, and so it's not going to be able to execute PHP code in your module info array. If you need your version number to be in a separate file, I'd suggest adding something that reads that file and then writes a new Module.info.json or Module.info.php file before pushing new versions to GitHub.

[BernhardBaumrock]

Hey @ryancramerdesign thx for your response. I'm really all in on this automated workflow. I've several rock modules on every project and I'm working on all of them and pushing updates all the time. It's really a huge time (and brain) saver to get automated versions from my git repository. I don't want to miss that any more.

I just push "fix: something" and get the version updated from 1.2.3 to 1.2.4; Or I push "feat: new something" and get v1.3.0 - without having to first look into the correct file, find the version number, increase it manually, save the file. And not to forget: Update the releaselog and add a tag to the repo. This is all done by a simple commit message!

It would really be great if the modules directory could grab the module's version from package.json if it is not able to read it from the module's info file. Like here: https://processwire.com/modules/rock-frontend/

I guess this should be an easy "fix" for the modules directory and it would be a huge help for me and others: https://processwire.com/talk/topic/28235-how-to-get-fully-automated-releases-tags-changelog-and-version-numbers-for-your-module-or-processwire-site/?do=findComment&comment=231432

[BernhardBaumrock]

PS: It would also be great if comments after array elements did not throw errors but would simply be removed by the parser. I'm not asking for support for complex concatenation scenarios or such, but removing comments would be nice :)

[BernhardBaumrock]

Thx for adding this @ryancramerdesign !