Formatting check succeeded!

Codecov Report

❌ Patch coverage is 0% with 112 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.65%. Comparing base (d0643c5) to head (419c3dd).
⚠️ Report is 1 commits behind head on master.

@@             Coverage Diff              @@
##             master    #1640      +/-   ##
============================================
- Coverage     65.91%   65.65%   -0.27%     
  Complexity     1645     1645              
============================================
  Files           478      480       +2     
  Lines         27664    27776     +112     
  Branches       5499     5523      +24     
============================================
  Hits          18235    18235              
- Misses         7104     7216     +112     
  Partials       2325     2325              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I don't have any comments on the code (haven't looked at it), but I'm curious -- what's the motivation/vision for this CQL AST?

The ultimate goal is to split the compiler into a frontend (parsing, AST generation, type inference, some types of optimization like inlining) and a backend (ELM generation). The motivations for that are multi-fold...

Thanks for additional insight. That's helpful. It's been a while since I've written any significant CQL-to-ELM code (probably nearing a decade). I didn't realize the ELM model itself caused so much complexity -- I assumed it was mainly the type inference, function resolution, and other various CQL magic, but it sounds like that stuff is just shifting to the AST generator now. That said, ELM was based on Health e-Decisions, so I shouldn't be surprised that conforming to that schema might introduce additional complications.

There's not much to argue with here: better separation of concerns, better testing, better consistency across versions, better IDE tools, and easier translation into other target formats. I like it.

One small note: I wanted to see what the AST looks like, so I built it and ran the CQL playground. In the process of doing that, I found that the CQL-to-ELM UI README needs to be updated. It currently says to browse to http://localhost:3000/cql-to-elm-ui, but that should be http://localhost:3000/clinical_quality_language/playground now.

Hi @cmoesel, sorry the README got outdated after we deployed the UI to cqframework.org. Now it's fixed.

There's not much to argue with here... I like it.

Is that "I like the idea" or "I like the AST implementation?"

There's not much to argue with here... I like it.

Is that "I like the idea" or "I like the AST implementation?"

I like the idea. I haven't looked at the implementation closely enough to comment on that. Is there any documentation that lists all of the possible AST nodes and their relationships?

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@cmoesel - Docs updated. A few key ones:

Src/java/cql/README.md

Src/java/docs/DECISIONS.md

Src/java/cql/AST.md

I don't disagree with the AST approach here, there are some good arguments there, but I disagree with the premise of number 3, where are you seeing that "CQL has begun to evolve independently of ELM"? I don't think that's true at all, Every change that has been proposed in CQL 2.0 has a corresponding change in the ELM representation. I don't think it changes the thrust of the arguments, but I want to clarify the statement.

@brynrhodes - I don't see that in the AST docs anywhere? Was there some particular statement that needed to be clarified?

It's not in the docs so far as I know, I was referring to the comment thread above (#1640 (comment))

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud