Stream: fhirpath
Topic: FHIRPath test suite for precedence correct?
Tilo Christ (Jan 08 2022 at 21:06):
I continue to work my way through the FHIRPath test suite: I am wondering whether test cases 'testPrecedence3' and 'testPrecedence4' are correct? They are '1 > 2 is Boolean' with expected result true, and '1 | 1 is Integer' with expected result true. But 'is' has higher precedence than '>' and '|'. In my mind the proper outcome of the first expression is thus a failure, since it basically evaluates to '1 > false'. The outcome of the second one should be '[1, true]', as '1 is Integer' equals true. Or am I mistaken?
Brian Postlethwaite (Jan 08 2022 at 21:19):
The dotnet engine gives > a higher precedence than is (just tested it here)
Brian Postlethwaite (Jan 08 2022 at 21:20):
However I think you're right based on the spec. I'll check the code and grammar to compare.
Tilo Christ (Jan 08 2022 at 21:20):
I'm looking at this spec: http://hl7.org/fhirpath/#operator-precedence
Brian Postlethwaite (Jan 08 2022 at 21:21):
Did you test the Java or fhirpathjs implementation?
Tilo Christ (Jan 08 2022 at 21:21):
not yet, I'm contributing to @Grey Faulkenberry 's Dart version.
Brian Postlethwaite (Jan 08 2022 at 21:22):
I assume you have the tests file and walking through that yeah?
Tilo Christ (Jan 08 2022 at 21:23):
exactly. Very helpful in general, but a very small number of test cases leave me with some doubts.
Tilo Christ (Jan 08 2022 at 21:25):
The fhirpath.js is also doing the precedence as expected in the test suite and not as in the spec.
Brian Postlethwaite (Jan 08 2022 at 21:28):
The grammar file is consistent with spec
Tilo Christ (Jan 08 2022 at 21:33):
Do you know what the procedure is to give feedback on the test suite? Similar as with SDC, where one files a JIRA issue? I just found the next test case that should signal an error according to the spec, but the test suite expects it to pass.
Brian Postlethwaite (Jan 08 2022 at 21:33):
Just jira the same.
Brian Postlethwaite (Jan 08 2022 at 21:34):
(might not be an offical artifact, but makes sense to track in the same way)
Brian Postlethwaite (Jan 08 2022 at 21:35):
(and also here too for discussion like this)
Brian Postlethwaite (Jan 08 2022 at 21:36):
Actually, the grammar used by the fhirpathjs is different to the official one on the site...
Tilo Christ (Jan 08 2022 at 21:37):
Same here, there was no good ANTLR for Dart, so it is based on Petite Parser. Tries to get the spec correctly implemented, but cannot use the grammar file.
Brian Postlethwaite (Jan 08 2022 at 21:39):
This is what is in the fhirpathjs implementation...
https://github.com/HL7/fhirpath.js/blob/master/src/parser/FHIRPath.g4
And that moved the precedence of the is down below > and |
Brian Postlethwaite (Jan 08 2022 at 21:40):
Thus different to https://hl7.org/fhirpath/grammar.html
Brian Postlethwaite (Jan 08 2022 at 21:40):
(still trying to locate the specific part of the .net code that does this)
Brian Postlethwaite (Jan 08 2022 at 21:44):
Looks like the dotnet parser does the same as the fhirpathjs there too (precedence wise)
https://github.com/FirelyTeam/firely-net-common/blob/7c6f7fb726f41f5a6645a9bc9239af9ca4ec9f82/src/Hl7.FhirPath/FhirPath/Parser/Grammar.cs#L134
Tilo Christ (Jan 08 2022 at 21:47):
I think from a common sense perspective the lower precedence for 'is' makes sense. But I guess it would be hard to argue against a released ANSI standard...
Brian Postlethwaite (Jan 08 2022 at 21:50):
(deleted)
Last updated: Apr 12 2022 at 19:14 UTC
