Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For loop tests in 0084-feel-for-loops.dmn #681

Open
opatrascoiu opened this issue Oct 30, 2024 · 3 comments
Open

For loop tests in 0084-feel-for-loops.dmn #681

opatrascoiu opened this issue Oct 30, 2024 · 3 comments

Comments

@opatrascoiu
Copy link
Contributor

We have recently merged a PR with new tests in the above file.

I believe that some of the tests are incorrect. For example,

<decision name="decision_023" id="_decision_023">
    <!-- a valid numeric range is permitted as iteration context -->
    <variable name="decision_023"/>
    <literalExpression>
        <text>for i in [1..2] return i</text>
    </literalExpression>
</decision>

<decision name="decision_024" id="_decision_024">
    <!-- a valid date range is permitted as iteration context -->
    <variable name="decision_024"/>
    <literalExpression>
        <text>for i in [@"1980-01-01"..@"1980-01-03"] return i</text>
    </literalExpression>
</decision>

According to the spec (10.3.2.14 For loop expression):

An iteration context may either be an expression that returns a list of elements, or two expressions that return integers connected by “..”.

The above tests are incorrect (the expected value should be null) as the for loops have an iteration context made of one single expression, that is not a list, is a range. The correct syntax is 1..2 or @"1980-01-01"..@"1980-01-03" (see examples in the spec).

I am in favor of extending the spec to support the above, as they make sense, but we need to specify the semantics for open ends (e.g. (1, 2] ).

@StrayAlien
Copy link
Contributor

StrayAlien commented Oct 30, 2024

Hi @opatrascoiu - I was basing it on the spec text:

The for loop expression iterates over lists of elements or ranges of numbers or dates

I guess "range" is ambiguous. The tests following those do some checking for what might be invalid ranges (like the start being lesser than the end - and so on) but not open ended.

If they're contentious, I'm happy to comment them for now. Btw, if "proper" ranges are to be included there, then modifications to the spec should also consider <=10, !=10,=10 etc comparitive ranges as well. Perhaps likely to say they're not permitted.

Also, worth noting is that spec text you referenced seems not to have been updated when dates where added in 1.5.

@opatrascoiu
Copy link
Contributor Author

opatrascoiu commented Oct 31, 2024

Hi @opatrascoiu - I was basing it on the spec text:

The for loop expression iterates over lists of elements or ranges of numbers or dates

@StrayAlien Yes, the range concept introduced in the paragraph that you mentioned (section 10.3.2.14 For loop expression) is ambiguous. IMO the rest of the section clarifies the meaning of range in the context of for loops: 1..2 and not [1..2]. There are also some examples that support this approach.

I suggest we remove the tests and raise an RTF ticket to clarify.

@StrayAlien
Copy link
Contributor

@opatrascoiu - happy to. I agree. Will do. I'll raise the ticket as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants