Fix ModelInstance RegEx pattern for class names.

[MartinPeverelli]

The updated XSD files forbid the usage of digits in the class or namespaces for the modelInstance value.

Description

Changes '[A-Za-z_\\]+' for '[A-Za-z0-9_\\]+' to support namespaces with digits.

Fixed Issues (if relevant)

1. import.xsd doesn't allow numbers in vendor name and class name #4470
2. Exception error on Module name when it has number(s) on product_options.xml - Regex issue #9985

Manual testing scenarios

Create a module with a vendor name containing a digit. The following XML should be valid.

<!-- app/code/Interactiv4/TestProduct/etc/product_types.xml -->
<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Catalog:etc/product_types.xsd">
    <type name="test" label="Test Product" modelInstance="Interactiv4\TestProduct\Model\Product\Type\Course" />
</config>

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[orlangur]

To be processed together with #11062 as we agreed.

[adrian-martinez-interactiv4]

@orlangur @MartinPeverelli This PR and #11062 propose using [a-zA-Z0-9_\\]+ expression, but it may not be valid, since it allows expression starting with a number like 4nteractiv4\CourseProduct\Model\Product\Type\Course, as can be seen in the following link, https://regex101.com/r/7WTFSy/1, and here :

Starting numbers weren't allowed before, and I think opening xsd that way would result in a regression. Also, current expression allows expressions starting with underscores like ___Interactiv4\CourseProduct\Model\Product\Type\Course, what I think shouldn't be allowed.

I'd propose one of the following expressions:

• [A-Z][a-zA-Z0-9_\\]* if expected model name must start with a capital letter
• [A-Z\\][a-zA-Z0-9_\\]* if expected model name must start with a capital letter or backslash
• [a-zA-Z\\][a-zA-Z0-9_\\]* if expected model name must start with a letter (capital or not) or backslash

What do you think?

[vkublytskyi]

@adrian-martinez-interactiv4 I do agree with you that regexp from PR is too wide and allows invalid strings.

As Model name potentially could be any valid PHP class name (FQCN to be precise) and according to PHP documentation full regexp should looks like:

(?:\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)+

[orlangur]

@vkublytskyi http://php.net/manual/en/language.variables.basics.php has nothing to do with class names, it's about variables, http://php.net/manual/en/language.oop5.basic.php fits better.

Looks like you added backslash by your own while referring to unrelated documentation page :D Leading digit is still allowed: https://regex101.com/r/U0VCMk/1 (don't forget about ^...$). Note that leading backslash MUST NOT be allowed. No reason to allow class names violating PSR-2 => probably leading lowercase letter is not needed to be allowed?

Another place with FQCN regexp: https://github.com/magento/magento2/pull/11765/files It is not so bad as leading backslash or digit is not allowed, characters allowed are not so wide as in mentioned regexp though.

(?:\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)+

-> (just what PHP allows)

^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(?:\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)*$

-> (just what seems reasonable for us)

^[A-Z][a-zA-Z0-9_]*(?:\\[A-Z][a-zA-Z0-9_]*)*$

Let's pick the best variant in Slack and fix all places.

[vkublytskyi]

@orlangur RegExp from PHP site describes any names in PHP ("the same rules as other labels in PHP") which could be: variables, class names, function names, method names, property names, constants. I've added backslash to apply this pattern to FQCN as FQCN is a sequence of names joined by a backslash.

Also, not allowing leading backslash in Magento xml files (e.g. layouts, di, etc.) is ok. But blocks import process if someone specifies a valid model name with leading backslash is too hard and not necessary limitation.

Of course, any regexp that we choose should be wrapped with ^$

[ihor-sviziev]

@orlangur any updates?

[slavvka]

Closed due to inactivity. Feel free to re-open it if you wish to continue working on it.