Multiline macro in Impex inspection

[uvoigt]

Hi,
Would it be possible to respect multiline macros in Impex inspection?

$consentJoin = left join Consent as cons on {cons.customer}={cust.pk} \
 left join ConsentTemplate as ct on {ct.pk}={cons.consentTemplate}

[mlytvyn]

Hello, @uvoigt thank you for highlighting this.
ImpEx Lexer and Parser have to be adjusted accordingly.
I’ll try to find a free minute to look into it.

[mlytvyn]

@uvoigt , it looks like something like that is also possible and it may make me sleepless :)

As for now, I'm not sure how to properly adjust Parser/Lexer for such cases

$myVarPrefix-\
        secondVarPart\
        -\
        lastVarPart = valPrefix-\
        firstVarPart

# which results into:
$myvarPrefix-secondVarPart-lastVarPart = valPrefix-firstVarPart

INSERT_UPDATE SolrIndexerQuery; \
    solrIndexedType(identifier)[unique = true]; \
        identifier[unique = true]; \
            type(code); \
                injectCurrentDate[default = true]; \
                    injectCurrentTime[default = true]; \
                        injectLastIndexTime[default = true]; \
                            query; \
                                user(uid); \
                                    solrIndexerQueryParameters(name); \
                                        parameterProvider
    ; solrTypeAbc \
        ; country-updateQuery \
....

[mlytvyn]

btw, Rauf Aliev described multi-line support in 2016 :)
https://hybrismart.com/2016/10/30/mastering-hybris-impex-recipes-and-secrets/

suggesting - not to use it.

[uvoigt]

@mlytvyn I see this as a Lexer problem only. Since there is no line break like in multiline strings, the Lexer could simply remove the backslash (keeping positional information of course) and continue on the next line

[mlytvyn]

To be able to properly implement it, the whole approach has to be changed - instead of parsing it line by line content has to be split by ; CSV separator into 'cells'.
Also taking into account that this separator can be specified via project properties, which we may know only after indexing the project.

For example, this macro is valid, but will be imported not in complete:

$macro = someValue &$)(; second part, which will be ignored.

Btw, I've already adjusted lexer for macro name and macro value, but it is not ideal and doesn't not support special symbols ={}? you require in your sample FxS.

I'd like to remind about possible separation of the ImpEx and FlexibleSearch, last one can be extracted into own fxs file and loaded via FileLoaderValueTranslator (https://help.sap.com/doc/61058561bffb416395816991f147f6f0/2005/en-US/de/hybris/platform/commerceservices/impex/impl/FileLoaderValueTranslator.html).

[uvoigt]

@mlytvyn I agree with you that fundamentally changing the approach may be too much effort. It would be much easier if the original authors of the Impex engine would have used a grammar based parser as well.
I tried the FileLoaderValueTranslator. Unfortunately, it is not capable of simply reading URLs from the classpath :-/
If you decide to not implement that enhancement, no problem. I can definitely live with some warnings in an Impex file.

[mlytvyn]

That's odd, in quite many projects translator worked well.

Can you please check out this thread and ensure that all files are in right place?
https://stackoverflow.com/questions/47072786/impex-error-at-referencing-vm-file

There is a Slack channel for the Plugin (see readme.md), if you want we can discuss details of that specific issue.

[uvoigt]

Yes, you're right. It works using the path from the source root with a leading slash.

[mlytvyn]

here is Lexer regex for macro name:
backslash = [\\]
macro_name_declaration = [$](([\w\d- ])+({backslash}\s*)*)+[=]

macro value regex is way more complex, will try to adjust it according to your case

[uvoigt]

This is rather strange: to me it seems backslashes can be at any place in Impex. Some kind of pre-processing and not polluting the grammar with the handling of backslashes could be a better approach, while I don't know if this can be done with the Flex implementation. Positional information would be dropped if there is no such input as a stream of tokens holding the original positional info

Even this statement seems to be valid:

$\
test\
me\
=\
whatever

[mlytvyn]

indeed, and that's only half of an issue.
when Lexer/Parser will be adjusted, References and Code Completion will require some nasty refactoring, because some spaces are ignore by SAP Commerce and some not.

btw, so far regex for macro value (parser also had to be adjusted to support { } = ?
macro_value = ({identifier}({dot})?|({backslash}\s*))+

for sure, it will not be clear for everyone what the hell is this :)

[uvoigt]

Could it be an option to simply remove all backslashes before feeding the Lexer?

[mlytvyn]

backslashes required to know that two lines are related and should not be treated as separate entities.

in other words: \ can be treated differently and depends on the placement.

[uvoigt]

But that is what the CSVReader essentially does. It looks for a backslash at the end of the line (forgot to mention that) and then wraps all following lines ending with the backslash to one until end of the stream or a line without a backslash at the end.

[mlytvyn]

sure, I understand your point, it is valid for actual processing and import of the ImpEx file by CSVReader.
During file processing CSVReader manipulates actual content of the file and converts it into another, but in IntelliJ IDEA we're working with actual source of the file without actual modification.

p.s. thanks for reminding about CSVReader

[uvoigt]

What I meant by #666 (comment) is that it might be possible to intercept the input CharSequence provided to the Lexer and replace it by an own implementation of CharSequence which might be able to restore original position information of joined multilines by doing the same CSVReader does but save original multiline positions while joining. But I don't have enough inside into the current Lexer implementation to see if that is possible or not.

[mlytvyn]

unfortunately, I'm in the same busy boat.

here is flex file: https://github.com/epam/sap-commerce-intellij-idea-plugin/blob/main/src/com/intellij/idea/plugin/hybris/impex/Impex.flex, while ImpexLexer is always auto-generated and parses input charset from IntelliJ.
As for now I'm not sure that current implementation will be easy adjustable for such manipulations.

let's proceed with baby-steps.

[uvoigt]

I agree.
Since I don't have a plugin development workspace right here I cannot try it but it might be possible to override some methods in ImpexLexerAdapter e.g. start and to inject that modified CharSequence here. Maybe this class could even hold the multiline info and use it in locateToken.

[mlytvyn]

@uvoigt , added multi-line \ character support for:

• macro declaration name
• macro declaration value
• header line parameter
• value line value group

take a note, that for header parameter and value group only combination of the \ + \n supported, without any spaces after \

I've also:

• adjusted code completion and reference resolution for macros with \,
• inspection rule + fix to inform Developer about possible complications when using \ in the macro declaration name.

As usual, it will be part of the next 2023.2.8 release (some time in September), in meanwhile I will try to check new logic with some complex ImpEx files.
If you will have free minute, please build the Plugin from the release branch and verify at your side.

[uvoigt]

@mlytvyn nice work!
Only thing I see which might be a problem is

Is there a task that won't finish?
I tried to find/suspend the corresponding DefaultDispatcher-worker but was not successful finding out wat is happening.
But it is the same in the main branch so it should be okay.

[mlytvyn]

@uvoigt , thank you and, once again, thank you for bringing this up! I dig into OOTB translators and know more now :).

Analyzing - oh, it depends...

IntelliJ may re-trigger Inspections many-many times, some inspections we have rely on readiness of the Index, so they are usually constantly re-triggered and cancelled till the moment Index is ready.

This state may rely on size of the ImpEx file, we have quite a few Inspections and some of them are not very efficient :(

[mlytvyn]

@uvoigt let's call it a day and close as completed.

Thank you and looking forward to your suggestions!