Fixed referencing to another key when using dots in the keys

[toonjanssens]

I like to use dots in the keys to make the keys more readable. Its like a hierarchy where every part (separated by a dot) represents a level en avoid naming collisions. When you have entities like user_group in a user bundle, the key will be user.user_group.first_group and you can reference with user.user_group.* (this is a bit like SQL syntax)

This fix escapes the dots from the keys so the regex won't match wrong keys.

Issue #309

[theofidry]

Nice one! Could you add a lil' test as well for it please?

[toonjanssens]

No problem if you can tell me where to put the test. Its not clear to me where to add the test yaml.

[theofidry]

Sure, you can add a test in https://github.com/nelmio/alice/blob/master/tests/Nelmio/Alice/Fixtures/LoaderTest.php.

For the test YAML file you can try to place it under alice/tests/Nelmio/Alice/support/fixtures/ I think.

[tshelburne]

@Seldaek How do you feel about this? It's sort of a breaking change - there's definitely potential that users are relying on the regex-like approach as a feature. I don't know if we merge this haphazardly.

[toonjanssens]

I understand your concern, but I don't see how it could break users fixtures. If you don't use dots in the keys, this fix will have no effect. When you are using dots, it wil escape them and the regular expression should work as before. Unless you are using the dots intentionally as part of the regular expression, but this looks more as a sort of hack. Definitely not how it should be used or did I mis a chapter in the documentation?

[tshelburne]

@toonjanssens No, that's what I meant - if you are taking advantage of the fact that the dots can be used for regex matching, it's not backwards compatible. You do make an interesting argument though - our intended (and documented) use-case is the asterisk (*), so I suppose that does mean we are really pushing for more strict expected behavior. I still would like @Seldaek's opinion here.

[tommyvdv]

In my opinion:

The behavior of the asterisk does not match the usage examples in the documentation. Or the usage example lacks specificity.

Update the documentation and note the (quirky) behavior of . or make the behavior match the intent.

[Seldaek]

I tend to agree that while it has a BC break potential it is a bugfix and we never advertised this as a feature, so I'd be rather for fixing it. The fix isn't so great though as it only fixes the . case, the best would be to run preg_quote on $mask to escape all meta-characters and then replace \\* by .* to get a working pattern.

[theofidry]

The King of Regex has spoken.

[toonjanssens]

I changed the code to what @Seldaek suggested. For the Unit test I can use some help.

[theofidry]

@tshelburne would it be possible to add the tests while merging? It's true that the tests are a bit obscure sometimes as it may be hard to tell where to test, if there is any already existing or whatever. But would be a shame not to merge that because of a contributor not able to tell where to add a test and how to test it.

[tshelburne]

@theofidry I'm not sure what you're asking - it sounds like the tests are a bit disorganized for new contributors though, and I agree with that fully. I wouldn't be opposed to a PR to simply regroup the tests into smaller, meaningful chunks.

@toonjanssens For now, I would just add the test to the LoaderTest class and make sure that your use-case is adequately represented. You can do it just as an inline array, using the other test cases as examples. Let me know if you have other questions about it.

[theofidry]

Closed in favour of #346