[redcap] redcap2linst tool, generated LINST instrument terminal `{-@-}` crash

[regisoc]

Describe the bug
The {-@-} added in all redcap instruments makes the instrument_LINST class crash, as it tries to interpret the final line as another field.

Here is the log:

[CRITICAL] /var/www/Loris/php/libraries/NDB_BVL_Instrument_LINST.class.inc:404: Uncaught TypeError: strpos(): Argument #1 ($haystack) must be of type string, null given in /var/www/Loris/php/libraries/NDB_BVL_Instrument_LINST.class.inc:404
Stack trace:
#0 /var/www/Loris/php/libraries/NDB_BVL_Instrument_LINST.class.inc(404): strpos()
#1 /var/www/Loris/php/libraries/NDB_BVL_Instrument.class.inc(308): Loris\\Behavioural\
DB_BVL_Instrument_LINST->loadInstrumentFile()
#2 /var/www/Loris/modules/instruments/php/instrumentqueryengine.class.inc(58): NDB_BVL_Instrument::factory()
#3 /var/www/Loris/modules/dataquery/php/query.class.inc(409): LORIS\\instruments\\InstrumentQueryEngine->getDataDictionary()
#4 /var/www/Loris/modules/dataquery/php/query.class.inc(458): LORIS\\dataquery\\Query->_populateModuleDictCache()
#5 /var/www/Loris/modules/dataquery/php/query.class.inc(174): LORIS\\dataquery\\Query->getFields()
#6 /var/www/Loris/src/Data/Filters/AccessibleResourceFilter.php(47): LORIS\\dataquery\\Query->isAccessibleBy()
#7 /var/www/Loris/src/Data/ProvisionerInstance.php(130): LORIS\\Data\\Filters\\AccessibleResourceFilter->filter()
#8 [internal function]: LORIS\\Data\\ProvisionerInstance->{closure:LORIS\\Data\\ProvisionerInstance::execute():129}()
#9 [internal function]: CallbackFilterIterator->accept()
#10 /var/www/Loris/modules/dataquery/php/module.class.inc(107): FilterIterator->next()
#11 /var/www/Loris/modules/dashboard/php/dashboard.class.inc(89): LORIS\\dataquery\\Module->getWidgets()
#12 /var/www/Loris/php/libraries/Module.class.inc(224): LORIS\\dashboard\\Dashboard->__construct()
#13 /var/www/Loris/php/libraries/Module.class.inc(307): Module->loadPage()
#14 /var/www/Loris/src/Middleware/ResponseGenerator.php(51): Module->handle()
#15 /var/www/Loris/src/Middleware/AuthMiddleware.php(64): LORIS\\Middleware\\ResponseGenerator->process()
#16 /var/www/Loris/src/Router/ModuleRouter.php(75): LORIS\\Middleware\\AuthMiddleware->process()
#17 /var/www/Loris/src/Middleware/ExceptionHandlingMiddleware.php(55): LORIS\\Router\\ModuleRouter->handle()
#18 /var/www/Loris/src/Router/BaseRouter.php(160): LORIS\\Middleware\\ExceptionHandlingMiddleware->process()
#19 /var/www/Loris/src/Middleware/ResponseGenerator.php(51): LORIS\\Router\\BaseRouter->handle()
#20 /var/www/Loris/src/Middleware/MFA.php(46): LORIS\\Middleware\\ResponseGenerator->process()
#21 /var/www/Loris/src/Middleware/RedirectControl.php(36): LORIS\\Middleware\\MFA->process()
#22 /var/www/Loris/src/Middleware/ContentSecurityPolicy.php(57): LORIS\\Middleware\\RedirectControl->process()
#23 /var/www/Loris/src/Middleware/AWS.php(35): LORIS\\Middleware\\ContentSecurityPolicy->process()
#24 /var/www/Loris/src/Middleware/ContentLength.php(53): LORIS\\Middleware\\AWS->process()
#25 /var/www/Loris/src/Middleware/LorisMenu.php(34): LORIS\\Middleware\\ContentLength->process()
#26 /var/www/Loris/src/Middleware/ContentLength.php(53): LORIS\\Middleware\\LorisMenu->process()
#27 /var/www/Loris/src/Middleware/Language.php(141): LORIS\\Middleware\\ContentLength->process()
#28 /var/www/Loris/src/Middleware/RequestAttributeBuilder.php(60): LORIS\\Middleware\\Language->process()
#29 /var/www/Loris/htdocs/index.php(96): LORIS\\Middleware\\RequestAttributeBuilder->process()
#30 {main}
  thrown, referer: https://test-dev-270.loris.ca/999999

Other instruments like bmi and eeg_acquisition_form not having {-@-} tag are not triggering this error.
Removing the tag from the instrument solves the issue, but there will be an interaction with/change to apply to the data dictionary builder tool.

To Reproduce
Steps to reproduce the behavior (attach screenshots if applicable):

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

What did you expect to happen?
A clear and concise description of what you expected to happen.

Browser Environment (please complete the following information):

• OS: [e.g. Plan9, HURD, Oberon, etc]
• Browser [e.g. chrome, firefox] (note that only these two browsers are supported)
• Version [e.g. 22]

Server Environment (if known):
Note: We only support the most recent release of LORIS.

• LORIS Version: [e.g. 22.0.0]
• Linux distribution and Version: [e.g. Ubuntu 16.04, CentOS 7]
• MySQL/MariaDB Version: [e.g. MySQL 5.7, MariaDB 10.3]

Additional context
Add any other context about the problem here.

[ridz1208]

@MaximeBICMTL @regisoc let's take this to slack. the PR that made the change in the first place was by my request so I wanna make sure the fix doesnt break my usecase

[regisoc]

I was trying to piece things together. Here are the points:

• there is no "tools" testing in the v29 testing spreadsheet
• {-@-} is used in multiple tools:
  • lorisform_parser
  • generate_tables_sql, based on ip_output (output of lorisform_parser).
  • generate_tables_sql_and_testNames, based on ip_output (output of lorisform_parser).
  • exporters/data_dictionary_builder, based on ip_output (output of lorisform_parser).
• This topics was quickly discussed in Loris slack.
• CBIG includes meta file before the linst file in ip_output, hence the issue.
• Projects status:
  • HBCD is managing that at the beginning of each instrument linst file (redcap2linst and manual)..
  • IBIS is also using the beginning of the file notation.
  • CBIG is adding it at the end of each instruments during ip_output building process. Were there any overrides for the other tools using ip_output?
• as discussed on slack, this should only target the old data dictionary as the new one does not need tools to parse files.
• associated PR + discsussion: [redcap] Fix REDCap to LINST test name prefix #9949

[ridz1208]

I think in the CBIG case I'm okai with either it not being in there at all or be at the end of the file since I use meta files for each instrument.

[regisoc]

@ridz1208 is adding the meta file for each instrument in ip_ouput something that can be generalizable to all projects? (What is the root of that change?)