From 83b5f1ed53f9d26ab26ada9e9975f7bdb2434430 Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Fri, 16 Aug 2024 15:39:03 +1200 Subject: [PATCH 1/4] Remove choice type specifier type if empty. --- .../cqframework/cql/cql2elm/CqlCompiler.java | 8 ++- .../cqframework/cql/cql2elm/elm/ElmEdit.java | 41 +------------ .../cql/cql2elm/elm/ElmEditEnum.java | 58 +++++++++++++++++++ .../cql/cql2elm/elm/ElmEditor.java | 20 +++---- .../cql/cql2elm/elm/ElmEditTest.java | 32 ++++++++++ .../cql/cql2elm/elm/ElmEditorTest.java | 32 ++++++++++ 6 files changed, 138 insertions(+), 53 deletions(-) create mode 100644 Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditEnum.java create mode 100644 Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditTest.java create mode 100644 Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditorTest.java diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/CqlCompiler.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/CqlCompiler.java index 9b4862e5a..dbbdc0c0c 100644 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/CqlCompiler.java +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/CqlCompiler.java @@ -17,6 +17,7 @@ import org.antlr.v4.runtime.*; import org.antlr.v4.runtime.tree.ParseTree; import org.cqframework.cql.cql2elm.elm.ElmEdit; +import org.cqframework.cql.cql2elm.elm.ElmEditEnum; import org.cqframework.cql.cql2elm.elm.ElmEditor; import org.cqframework.cql.cql2elm.model.CompiledLibrary; import org.cqframework.cql.cql2elm.preprocessor.CqlPreprocessor; @@ -232,9 +233,10 @@ public Library run(CharStream is) { // Phase 5: ELM optimization/reduction (this is where result types, annotations, etc. are removed // and there will probably be a lot of other optimizations that happen here in the future) var edits = allNonNull( - !options.contains(EnableAnnotations) ? ElmEdit.REMOVE_ANNOTATION : null, - !options.contains(EnableResultTypes) ? ElmEdit.REMOVE_RESULT_TYPE : null, - !options.contains(EnableLocators) ? ElmEdit.REMOVE_LOCATOR : null); + !options.contains(EnableAnnotations) ? ElmEditEnum.REMOVE_ANNOTATION : null, + !options.contains(EnableResultTypes) ? ElmEditEnum.REMOVE_RESULT_TYPE : null, + !options.contains(EnableLocators) ? ElmEditEnum.REMOVE_LOCATOR : null, + ElmEditEnum.REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY); new ElmEditor(edits).edit(library); diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java index 912b97f16..8458df11b 100644 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java @@ -1,44 +1,7 @@ package org.cqframework.cql.cql2elm.elm; -import org.hl7.cql_annotations.r1.Annotation; import org.hl7.elm.r1.Element; -public enum ElmEdit { - REMOVE_LOCATOR { - @Override - public void edit(Element element) { - element.setLocator(null); - } - }, - REMOVE_ANNOTATION { - @Override - public void edit(Element element) { - element.setLocalId(null); - if (element.getAnnotation() != null) { - for (int i = 0; i < element.getAnnotation().size(); i++) { - var x = element.getAnnotation().get(i); - if (x instanceof Annotation) { - var a = (Annotation) x; - // TODO: Remove narrative but _not_ tags - // Tags are necessary for `allowFluent` compiler resolution - // to work correctly - a.setS(null); - if (a.getT().isEmpty()) { - element.getAnnotation().remove(i); - i--; - } - } - } - } - } - }, - REMOVE_RESULT_TYPE { - @Override - public void edit(Element element) { - element.setResultTypeName(null); - element.setResultTypeSpecifier(null); - } - }; - - public abstract void edit(Element element); +public interface ElmEdit { + void edit(Element element); } diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditEnum.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditEnum.java new file mode 100644 index 000000000..1ccf21ec6 --- /dev/null +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditEnum.java @@ -0,0 +1,58 @@ +package org.cqframework.cql.cql2elm.elm; + +import org.hl7.cql_annotations.r1.Annotation; +import org.hl7.elm.r1.ChoiceTypeSpecifier; +import org.hl7.elm.r1.Element; + +public enum ElmEditEnum implements ElmEdit { + REMOVE_LOCATOR { + public void edit(Element element) { + element.setLocator(null); + } + }, + REMOVE_ANNOTATION { + public void edit(Element element) { + element.setLocalId(null); + if (element.getAnnotation() != null) { + for (int i = 0; i < element.getAnnotation().size(); i++) { + var x = element.getAnnotation().get(i); + if (x instanceof Annotation) { + var a = (Annotation) x; + // TODO: Remove narrative but _not_ tags + // Tags are necessary for `allowFluent` compiler resolution + // to work correctly + a.setS(null); + if (a.getT().isEmpty()) { + element.getAnnotation().remove(i); + i--; + } + } + } + } + } + }, + REMOVE_RESULT_TYPE { + public void edit(Element element) { + element.setResultTypeName(null); + element.setResultTypeSpecifier(null); + } + }, + REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY { + // The ChoiceTypeSpecifier ELM node has a deprecated `type` element which, if not null, clashes with the + // `"type" : "ChoiceTypeSpecifier"` field in the JSON serialization of the node. This does not happen in the XML + // serialization which nests tags inside . + // Because the `type` element is deprecated, it is not normally populated by the compiler anymore and + // stays null in the ChoiceTypeSpecifier instance. It is however set to an empty list if you just call + // ChoiceTypeSpecifier.getType() (which we do during the ELM optimization stage in the compiler), so + // this edit is needed to "protect" the downstream JSON serialization if it can be done without data loss. + + public void edit(Element element) { + if (element instanceof ChoiceTypeSpecifier) { + var choice = (ChoiceTypeSpecifier) element; + if (choice.getType().isEmpty()) { + choice.setType(null); + } + } + } + }; +} diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java index 28dd8e23d..12893dbdc 100644 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java @@ -11,26 +11,24 @@ public class ElmEditor { private final List edits; - private final FunctionalElmVisitor> visitor; + private final FunctionalElmVisitor visitor; public ElmEditor(List edits) { this.edits = Objects.requireNonNull(edits); - this.visitor = Visitors.from(ElmEditor::applyEdits); + this.visitor = Visitors.from((elm, context) -> elm, this::applyEdits); } public void edit(Library library) { - this.visitor.visitLibrary(library, edits); + this.visitor.visitLibrary(library, null); } - protected static Void applyEdits(Trackable trackable, List edits) { - if (!(trackable instanceof Element)) { - return null; + protected Trackable applyEdits(Trackable aggregate, Trackable nextResult) { + if (nextResult instanceof Element) { + for (ElmEdit edit : edits) { + edit.edit((Element) nextResult); + } } - for (ElmEdit edit : edits) { - edit.edit((Element) trackable); - } - - return null; + return aggregate; } } diff --git a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditTest.java b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditTest.java new file mode 100644 index 000000000..d3211900c --- /dev/null +++ b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditTest.java @@ -0,0 +1,32 @@ +package org.cqframework.cql.cql2elm.elm; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.List; +import org.hl7.elm.r1.ChoiceTypeSpecifier; +import org.hl7.elm.r1.NamedTypeSpecifier; +import org.hl7.elm.r1.TypeSpecifier; +import org.junit.jupiter.api.Test; + +class ElmEditTest { + + @Test + void removeChoiceTypeSpecifierTypeIfEmpty() { + var extChoiceTypeSpecifier = new ExtChoiceTypeSpecifier(); + + extChoiceTypeSpecifier.setType(List.of()); + ElmEditEnum.REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY.edit(extChoiceTypeSpecifier); + assertNull(extChoiceTypeSpecifier.getType()); + + var typeSpecifiers = List.of((TypeSpecifier) new NamedTypeSpecifier()); + extChoiceTypeSpecifier.setType(typeSpecifiers); + ElmEditEnum.REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY.edit(extChoiceTypeSpecifier); + assertSame(typeSpecifiers, extChoiceTypeSpecifier.getType()); + } + + private static class ExtChoiceTypeSpecifier extends ChoiceTypeSpecifier { + public List getType() { + return type; + } + } +} diff --git a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditorTest.java b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditorTest.java new file mode 100644 index 000000000..36005a2c8 --- /dev/null +++ b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditorTest.java @@ -0,0 +1,32 @@ +package org.cqframework.cql.cql2elm.elm; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.List; +import org.hl7.elm.r1.ChoiceTypeSpecifier; +import org.hl7.elm.r1.Element; +import org.junit.jupiter.api.Test; + +class ElmEditorTest { + boolean editCalled = false; + + @Test + void applyEdits() { + editCalled = false; + var editCounter = new ElmEdit() { + public void edit(Element element) { + editCalled = true; + } + }; + var edits = List.of((ElmEdit) editCounter); + ElmEditor editor = new ElmEditor(edits); + + editCalled = false; + editor.applyEdits(null, null); + assertFalse(editCalled); + + editCalled = false; + editor.applyEdits(null, new ChoiceTypeSpecifier()); + assertTrue(editCalled); + } +} From cc94269246887146fb05d6ea3509b7d341d68964 Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Fri, 16 Aug 2024 16:42:54 +1200 Subject: [PATCH 2/4] Make sure edit is applied to the library itself. Allow direct ELM construction in tests. --- .../cql/cql2elm/elm/ElmEditor.java | 15 ++++++- .../cql/cql2elm/ArchitectureTest.java | 5 ++- .../cql/cql2elm/elm/ElmEditorTest.java | 41 +++++++++++-------- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java index 12893dbdc..25c6a51d5 100644 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java @@ -15,14 +15,17 @@ public class ElmEditor { public ElmEditor(List edits) { this.edits = Objects.requireNonNull(edits); - this.visitor = Visitors.from((elm, context) -> elm, this::applyEdits); + this.visitor = Visitors.from((elm, context) -> elm, this::aggregateResults); } public void edit(Library library) { this.visitor.visitLibrary(library, null); + + // This is needed because aggregateResults is not called for the library itself. + this.applyEdits(library); } - protected Trackable applyEdits(Trackable aggregate, Trackable nextResult) { + protected Trackable aggregateResults(Trackable aggregate, Trackable nextResult) { if (nextResult instanceof Element) { for (ElmEdit edit : edits) { edit.edit((Element) nextResult); @@ -31,4 +34,12 @@ protected Trackable applyEdits(Trackable aggregate, Trackable nextResult) { return aggregate; } + + protected void applyEdits(Trackable trackable) { + if (trackable instanceof Element) { + for (ElmEdit edit : edits) { + edit.edit((Element) trackable); + } + } + } } diff --git a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/ArchitectureTest.java b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/ArchitectureTest.java index 3671456e1..6b8195e77 100644 --- a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/ArchitectureTest.java +++ b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/ArchitectureTest.java @@ -4,6 +4,7 @@ import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ClassFileImporter; +import com.tngtech.archunit.core.importer.ImportOption; import org.cqframework.cql.cql2elm.model.LibraryRef; import org.cqframework.cql.elm.IdObjectFactory; import org.hl7.elm.r1.Element; @@ -14,7 +15,9 @@ class ArchitectureTest { @Test void ensureNoDirectElmConstruction() { - JavaClasses importedClasses = new ClassFileImporter().importPackages("org.cqframework.cql"); + JavaClasses importedClasses = new ClassFileImporter() + .withImportOption(new ImportOption.DoNotIncludeTests()) + .importPackages("org.cqframework.cql"); constructors() .that() diff --git a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditorTest.java b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditorTest.java index 36005a2c8..6209127ae 100644 --- a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditorTest.java +++ b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditorTest.java @@ -1,32 +1,37 @@ package org.cqframework.cql.cql2elm.elm; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; import java.util.List; import org.hl7.elm.r1.ChoiceTypeSpecifier; -import org.hl7.elm.r1.Element; +import org.hl7.elm.r1.Library; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class ElmEditorTest { - boolean editCalled = false; + private int editCount = 0; + private final ElmEditor editor = new ElmEditor(List.of(element -> editCount++)); + + @BeforeEach + void beforeEach() { + editCount = 0; + } @Test - void applyEdits() { - editCalled = false; - var editCounter = new ElmEdit() { - public void edit(Element element) { - editCalled = true; - } - }; - var edits = List.of((ElmEdit) editCounter); - ElmEditor editor = new ElmEditor(edits); + void edit() { + editor.edit(new Library()); + assertEquals(editCount, 1); + } - editCalled = false; - editor.applyEdits(null, null); - assertFalse(editCalled); + @Test + void applyEdits1() { + editor.applyEdits(null); + assertEquals(editCount, 0); + } - editCalled = false; - editor.applyEdits(null, new ChoiceTypeSpecifier()); - assertTrue(editCalled); + @Test + void applyEdits2() { + editor.applyEdits(new ChoiceTypeSpecifier()); + assertEquals(editCount, 1); } } From 821d6dccc5e3123ddf9276bb673530638c93f648 Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Fri, 16 Aug 2024 17:05:12 +1200 Subject: [PATCH 3/4] Tidy everything up --- .../cqframework/cql/cql2elm/CqlCompiler.java | 12 ++-- .../cqframework/cql/cql2elm/elm/ElmEdit.java | 55 +++++++++++++++++- .../cql/cql2elm/elm/ElmEditEnum.java | 58 ------------------- .../cql/cql2elm/elm/ElmEditor.java | 15 ++--- .../cqframework/cql/cql2elm/elm/IElmEdit.java | 7 +++ .../cql/cql2elm/elm/ElmEditTest.java | 4 +- 6 files changed, 73 insertions(+), 78 deletions(-) delete mode 100644 Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditEnum.java create mode 100644 Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/IElmEdit.java diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/CqlCompiler.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/CqlCompiler.java index dbbdc0c0c..a99fbbac6 100644 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/CqlCompiler.java +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/CqlCompiler.java @@ -17,8 +17,8 @@ import org.antlr.v4.runtime.*; import org.antlr.v4.runtime.tree.ParseTree; import org.cqframework.cql.cql2elm.elm.ElmEdit; -import org.cqframework.cql.cql2elm.elm.ElmEditEnum; import org.cqframework.cql.cql2elm.elm.ElmEditor; +import org.cqframework.cql.cql2elm.elm.IElmEdit; import org.cqframework.cql.cql2elm.model.CompiledLibrary; import org.cqframework.cql.cql2elm.preprocessor.CqlPreprocessor; import org.cqframework.cql.elm.IdObjectFactory; @@ -233,10 +233,10 @@ public Library run(CharStream is) { // Phase 5: ELM optimization/reduction (this is where result types, annotations, etc. are removed // and there will probably be a lot of other optimizations that happen here in the future) var edits = allNonNull( - !options.contains(EnableAnnotations) ? ElmEditEnum.REMOVE_ANNOTATION : null, - !options.contains(EnableResultTypes) ? ElmEditEnum.REMOVE_RESULT_TYPE : null, - !options.contains(EnableLocators) ? ElmEditEnum.REMOVE_LOCATOR : null, - ElmEditEnum.REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY); + !options.contains(EnableAnnotations) ? ElmEdit.REMOVE_ANNOTATION : null, + !options.contains(EnableResultTypes) ? ElmEdit.REMOVE_RESULT_TYPE : null, + !options.contains(EnableLocators) ? ElmEdit.REMOVE_LOCATOR : null, + ElmEdit.REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY); new ElmEditor(edits).edit(library); @@ -250,7 +250,7 @@ public Library run(CharStream is) { return library; } - private List allNonNull(ElmEdit... ts) { + private List allNonNull(IElmEdit... ts) { return Arrays.stream(ts).filter(x -> x != null).collect(Collectors.toList()); } } diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java index 8458df11b..28d7617b5 100644 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java @@ -1,7 +1,58 @@ package org.cqframework.cql.cql2elm.elm; +import org.hl7.cql_annotations.r1.Annotation; +import org.hl7.elm.r1.ChoiceTypeSpecifier; import org.hl7.elm.r1.Element; -public interface ElmEdit { - void edit(Element element); +public enum ElmEdit implements IElmEdit { + REMOVE_LOCATOR { + public void edit(Element element) { + element.setLocator(null); + } + }, + REMOVE_ANNOTATION { + public void edit(Element element) { + element.setLocalId(null); + if (element.getAnnotation() != null) { + for (int i = 0; i < element.getAnnotation().size(); i++) { + var x = element.getAnnotation().get(i); + if (x instanceof Annotation) { + var a = (Annotation) x; + // TODO: Remove narrative but _not_ tags + // Tags are necessary for `allowFluent` compiler resolution + // to work correctly + a.setS(null); + if (a.getT().isEmpty()) { + element.getAnnotation().remove(i); + i--; + } + } + } + } + } + }, + REMOVE_RESULT_TYPE { + public void edit(Element element) { + element.setResultTypeName(null); + element.setResultTypeSpecifier(null); + } + }, + REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY { + // The ChoiceTypeSpecifier ELM node has a deprecated `type` element which, if not null, clashes with the + // `"type" : "ChoiceTypeSpecifier"` field in the JSON serialization of the node. This does not happen in the XML + // serialization which nests tags inside . + // Because the `type` element is deprecated, it is not normally populated by the compiler anymore and + // stays null in the ChoiceTypeSpecifier instance. It is however set to an empty list if you just call + // ChoiceTypeSpecifier.getType() (which we do during the ELM optimization stage in the compiler), so + // this edit is needed to "protect" the downstream JSON serialization if it can be done without data loss. + + public void edit(Element element) { + if (element instanceof ChoiceTypeSpecifier) { + var choice = (ChoiceTypeSpecifier) element; + if (choice.getType().isEmpty()) { + choice.setType(null); + } + } + } + }; } diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditEnum.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditEnum.java deleted file mode 100644 index 1ccf21ec6..000000000 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditEnum.java +++ /dev/null @@ -1,58 +0,0 @@ -package org.cqframework.cql.cql2elm.elm; - -import org.hl7.cql_annotations.r1.Annotation; -import org.hl7.elm.r1.ChoiceTypeSpecifier; -import org.hl7.elm.r1.Element; - -public enum ElmEditEnum implements ElmEdit { - REMOVE_LOCATOR { - public void edit(Element element) { - element.setLocator(null); - } - }, - REMOVE_ANNOTATION { - public void edit(Element element) { - element.setLocalId(null); - if (element.getAnnotation() != null) { - for (int i = 0; i < element.getAnnotation().size(); i++) { - var x = element.getAnnotation().get(i); - if (x instanceof Annotation) { - var a = (Annotation) x; - // TODO: Remove narrative but _not_ tags - // Tags are necessary for `allowFluent` compiler resolution - // to work correctly - a.setS(null); - if (a.getT().isEmpty()) { - element.getAnnotation().remove(i); - i--; - } - } - } - } - } - }, - REMOVE_RESULT_TYPE { - public void edit(Element element) { - element.setResultTypeName(null); - element.setResultTypeSpecifier(null); - } - }, - REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY { - // The ChoiceTypeSpecifier ELM node has a deprecated `type` element which, if not null, clashes with the - // `"type" : "ChoiceTypeSpecifier"` field in the JSON serialization of the node. This does not happen in the XML - // serialization which nests tags inside . - // Because the `type` element is deprecated, it is not normally populated by the compiler anymore and - // stays null in the ChoiceTypeSpecifier instance. It is however set to an empty list if you just call - // ChoiceTypeSpecifier.getType() (which we do during the ELM optimization stage in the compiler), so - // this edit is needed to "protect" the downstream JSON serialization if it can be done without data loss. - - public void edit(Element element) { - if (element instanceof ChoiceTypeSpecifier) { - var choice = (ChoiceTypeSpecifier) element; - if (choice.getType().isEmpty()) { - choice.setType(null); - } - } - } - }; -} diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java index 25c6a51d5..4ecd00113 100644 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEditor.java @@ -10,10 +10,10 @@ public class ElmEditor { - private final List edits; + private final List edits; private final FunctionalElmVisitor visitor; - public ElmEditor(List edits) { + public ElmEditor(List edits) { this.edits = Objects.requireNonNull(edits); this.visitor = Visitors.from((elm, context) -> elm, this::aggregateResults); } @@ -21,23 +21,18 @@ public ElmEditor(List edits) { public void edit(Library library) { this.visitor.visitLibrary(library, null); - // This is needed because aggregateResults is not called for the library itself. + // This is needed because aggregateResults is not called on the library itself. this.applyEdits(library); } protected Trackable aggregateResults(Trackable aggregate, Trackable nextResult) { - if (nextResult instanceof Element) { - for (ElmEdit edit : edits) { - edit.edit((Element) nextResult); - } - } - + applyEdits(nextResult); return aggregate; } protected void applyEdits(Trackable trackable) { if (trackable instanceof Element) { - for (ElmEdit edit : edits) { + for (IElmEdit edit : edits) { edit.edit((Element) trackable); } } diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/IElmEdit.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/IElmEdit.java new file mode 100644 index 000000000..ee6ba3d5d --- /dev/null +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/IElmEdit.java @@ -0,0 +1,7 @@ +package org.cqframework.cql.cql2elm.elm; + +import org.hl7.elm.r1.Element; + +public interface IElmEdit { + void edit(Element element); +} diff --git a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditTest.java b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditTest.java index d3211900c..b86a8b878 100644 --- a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditTest.java +++ b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/elm/ElmEditTest.java @@ -15,12 +15,12 @@ void removeChoiceTypeSpecifierTypeIfEmpty() { var extChoiceTypeSpecifier = new ExtChoiceTypeSpecifier(); extChoiceTypeSpecifier.setType(List.of()); - ElmEditEnum.REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY.edit(extChoiceTypeSpecifier); + ElmEdit.REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY.edit(extChoiceTypeSpecifier); assertNull(extChoiceTypeSpecifier.getType()); var typeSpecifiers = List.of((TypeSpecifier) new NamedTypeSpecifier()); extChoiceTypeSpecifier.setType(typeSpecifiers); - ElmEditEnum.REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY.edit(extChoiceTypeSpecifier); + ElmEdit.REMOVE_CHOICE_TYPE_SPECIFIER_TYPE_IF_EMPTY.edit(extChoiceTypeSpecifier); assertSame(typeSpecifiers, extChoiceTypeSpecifier.getType()); } From cbb4ff960b832655b5f4755c4524460a4393267a Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Fri, 16 Aug 2024 17:09:25 +1200 Subject: [PATCH 4/4] Tidy everything up --- .../main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java index 28d7617b5..81212314d 100644 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/elm/ElmEdit.java @@ -6,11 +6,13 @@ public enum ElmEdit implements IElmEdit { REMOVE_LOCATOR { + @Override public void edit(Element element) { element.setLocator(null); } }, REMOVE_ANNOTATION { + @Override public void edit(Element element) { element.setLocalId(null); if (element.getAnnotation() != null) { @@ -32,6 +34,7 @@ public void edit(Element element) { } }, REMOVE_RESULT_TYPE { + @Override public void edit(Element element) { element.setResultTypeName(null); element.setResultTypeSpecifier(null); @@ -46,6 +49,7 @@ public void edit(Element element) { // ChoiceTypeSpecifier.getType() (which we do during the ELM optimization stage in the compiler), so // this edit is needed to "protect" the downstream JSON serialization if it can be done without data loss. + @Override public void edit(Element element) { if (element instanceof ChoiceTypeSpecifier) { var choice = (ChoiceTypeSpecifier) element;