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

Ensure local Ids for all ELM Elements #1303

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
65cc18f
Scaffolding for testing missing local ids
JPercival Dec 2, 2023
ffba970
WIP for localId validation
JPercival Dec 5, 2023
b89a9dd
WIP
JPercival Dec 5, 2023
7b1ff7f
WIP
JPercival Dec 6, 2023
299899d
Cleaning up tests
JPercival Dec 6, 2023
70aca54
Remove whitespace changes
JPercival Dec 6, 2023
788cb43
More whitespace cleanup
JPercival Dec 6, 2023
7f22014
Remove more unintended text changes
JPercival Dec 6, 2023
2778376
One more unwanted change
JPercival Dec 6, 2023
9996710
Bug fixes for the ELM visitor, fixed incorrect tests
JPercival Dec 7, 2023
a8e6da9
fix tag tests
JPercival Dec 7, 2023
0bb0fc4
Hack around compiler depending on tags
JPercival Dec 7, 2023
9e48dc0
Cleanup
JPercival Dec 7, 2023
3c863eb
More cleanup
JPercival Dec 7, 2023
87188b6
More cleanup
JPercival Dec 7, 2023
762ec6d
More cleanup
JPercival Dec 7, 2023
c68c52c
Reset ELM base visitor
JPercival Dec 11, 2023
fb5864c
Added test to ensure all elements of a random ELM graph are visited o…
JPercival Dec 12, 2023
06b42c0
Bit of clean-up and renaming for clarity
JPercival Dec 12, 2023
ffacc2f
More cleanup
JPercival Dec 12, 2023
7d8bae3
Architecture tests
JPercival Dec 13, 2023
9c65eb4
Working Random ELM tests and design tests
JPercival Dec 13, 2023
b1abfb2
bug fixes
JPercival Dec 18, 2023
b95d3ea
Add spotless, prep for master merge
JPercival Dec 21, 2023
772853f
Merge remote-tracking branch 'origin/master' into bug-elm-visitor
JPercival Dec 21, 2023
9f001a2
Fixed bugs in datarequirements and base visitor
JPercival Dec 22, 2023
18970ac
Remove max report age restriction
JPercival Dec 22, 2023
6f001b5
Fix codecov one more time!
JPercival Dec 22, 2023
65ca219
format with spotless to prep for merge
JPercival Dec 22, 2023
c56187a
Merge branch 'bug-elm-visitor' into bug-localId-tests
JPercival Dec 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Src/java/.vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@
"fhirpath",
"hamcrest",
"Inferencing",
"opdef",
"Instancio",
"Objenesis",
"qicore",
"Randomizer",
"testng",
"tngtech",
"trackback"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ dependencies {
testImplementation 'org.testng:testng:7.4.0'
testImplementation 'org.hamcrest:hamcrest-all:1.3'
testImplementation 'uk.co.datumedge:hamcrest-json:0.2'
testImplementation 'junit:junit:4.12'
testImplementation 'junit:junit:4.13.2'
testImplementation 'org.slf4j:slf4j-simple:1.7.36'

// These are JAXB dependencies excluded because the libraries need to work
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ dependencies {

ext.xjc = [
destDir: "${buildDir}/generated/sources/$name/main/java",
args: '-disableXmlSecurity -Xfluent-api -Xequals -XhashCode -XtoString'
args: '-disableXmlSecurity -Xfluent-api -Xequals -XhashCode -XtoString -Xsetters -Xsetters-mode=direct'
]


Expand Down
1 change: 1 addition & 0 deletions Src/java/cql-to-elm/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ dependencies {
testImplementation project(':qdm')
testImplementation 'com.github.reinert:jjschema:1.16'
testImplementation 'com.tngtech.archunit:archunit:1.2.1'
testImplementation 'org.skyscreamer:jsonassert:1.5.1'
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
package org.cqframework.cql.cql2elm;

import static org.cqframework.cql.cql2elm.CqlCompilerOptions.Options.EnableAnnotations;
import static org.cqframework.cql.cql2elm.CqlCompilerOptions.Options.EnableLocators;
import static org.cqframework.cql.cql2elm.CqlCompilerOptions.Options.EnableResultTypes;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
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.ElmEditor;
import org.cqframework.cql.cql2elm.model.CompiledLibrary;
import org.cqframework.cql.cql2elm.preprocessor.CqlPreprocessorVisitor;
import org.cqframework.cql.cql2elm.preprocessor.CqlPreprocessor;
import org.cqframework.cql.elm.IdObjectFactory;
import org.cqframework.cql.elm.tracking.TrackBack;
import org.cqframework.cql.gen.cqlLexer;
import org.cqframework.cql.gen.cqlParser;
Expand Down Expand Up @@ -190,34 +199,45 @@ public Library run(CharStream is) {
warnings = new ArrayList<>();
messages = new ArrayList<>();

LibraryBuilder builder = new LibraryBuilder(namespaceInfo, libraryManager);
builder.setCompilerOptions(libraryManager.getCqlCompilerOptions());
Cql2ElmVisitor visitor = new Cql2ElmVisitor(builder);
builder.setVisitor(visitor);
visitor.setTranslatorOptions(libraryManager.getCqlCompilerOptions());
var options = libraryManager.getCqlCompilerOptions().getOptions();

CqlCompiler.CqlErrorListener errorListener =
new CqlCompiler.CqlErrorListener(builder, visitor.isDetailedErrorsEnabled());
LibraryBuilder builder = new LibraryBuilder(namespaceInfo, libraryManager, new IdObjectFactory());
CqlCompiler.CqlErrorListener errorListener = new CqlCompiler.CqlErrorListener(
builder, options.contains(CqlCompilerOptions.Options.EnableDetailedErrors));

// Phase 1: Lexing
cqlLexer lexer = new cqlLexer(is);
lexer.removeErrorListeners();
lexer.addErrorListener(errorListener);
CommonTokenStream tokens = new CommonTokenStream(lexer);

// Phase 2: Parsing (the lexer is actually streaming, so Phase 1 and 2 happen together)
cqlParser parser = new cqlParser(tokens);
parser.setBuildParseTree(true);

parser.removeErrorListeners(); // Clear the default console listener
parser.addErrorListener(errorListener);
ParseTree tree = parser.library();

CqlPreprocessorVisitor preprocessor = new CqlPreprocessorVisitor(builder, tokens);
// Phase 3: preprocess the parse tree (generates the LibraryInfo with
// header information for definitions)
CqlPreprocessor preprocessor = new CqlPreprocessor(builder, tokens);
preprocessor.visit(tree);

visitor.setTokenStream(tokens);
visitor.setLibraryInfo(preprocessor.getLibraryInfo());

// Phase 4: generate the ELM (the ELM is generated with full type information that can be used
// for validation, optimization, rewriting, debugging, etc.)
ElmGenerator visitor = new ElmGenerator(builder, tokens, preprocessor.getLibraryInfo());
visitResult = visitor.visit(tree);
library = builder.getLibrary();

// 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 = coalesceAll(
nullIfFalse(options.contains(EnableAnnotations), ElmEdit.REMOVE_ANNOTATION),
nullIfFalse(options.contains(EnableResultTypes), ElmEdit.REMOVE_RESULT_TYPE),
nullIfFalse(options.contains(EnableLocators), ElmEdit.REMOVE_LOCATOR));

new ElmEditor(edits).edit(library);

compiledLibrary = builder.getCompiledLibrary();
retrieves = visitor.getRetrieves();
exceptions.addAll(builder.getExceptions());
Expand All @@ -227,4 +247,12 @@ public Library run(CharStream is) {

return library;
}

private static <T> T nullIfFalse(boolean b, T t) {
return !b ? t : null;
}

private static List<ElmEdit> coalesceAll(ElmEdit... ts) {
return Arrays.stream(ts).filter(t -> t != null).collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.TokenStream;
import org.antlr.v4.runtime.tree.ParseTree;
import org.antlr.v4.runtime.tree.TerminalNode;
import org.cqframework.cql.cql2elm.model.*;
Expand All @@ -17,23 +18,14 @@
import org.cqframework.cql.gen.cqlParser;
import org.hl7.cql.model.*;
import org.hl7.elm.r1.*;
import org.hl7.elm.r1.Element;
import org.hl7.elm.r1.Interval;
import org.hl7.elm_modelinfo.r1.ModelInfo;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class Cql2ElmVisitor extends CqlPreprocessorElmCommonVisitor {
private static final Logger logger = LoggerFactory.getLogger(Cql2ElmVisitor.class);
public class ElmGenerator extends CqlPreprocessorElmCommonVisitor {
private static final Logger logger = LoggerFactory.getLogger(ElmGenerator.class);
private final SystemMethodResolver systemMethodResolver;

public void setLibraryInfo(LibraryInfo libraryInfo) {
if (libraryInfo == null) {
throw new IllegalArgumentException("libraryInfo is null");
}
this.libraryInfo = libraryInfo;
}

private final Set<String> definedExpressionDefinitions = new HashSet<>();
private final Stack<ExpressionDefinitionInfo> forwards = new Stack<>();
private final Map<cqlParser.FunctionDefinitionContext, FunctionHeader> functionHeaders = new HashMap<>();
Expand All @@ -45,13 +37,9 @@ public void setLibraryInfo(LibraryInfo libraryInfo) {
private final List<Expression> expressions = new ArrayList<>();
private final Map<String, Element> contextDefinitions = new HashMap<>();

public Cql2ElmVisitor(LibraryBuilder libraryBuilder) {
super(libraryBuilder);

if (libraryBuilder == null) {
throw new IllegalArgumentException("libraryBuilder is null");
}

public ElmGenerator(LibraryBuilder libraryBuilder, TokenStream tokenStream, LibraryInfo libraryInfo) {
super(libraryBuilder, tokenStream);
this.libraryInfo = Objects.requireNonNull(libraryInfo, "libraryInfo required");
this.systemMethodResolver = new SystemMethodResolver(this, libraryBuilder);
}

Expand Down Expand Up @@ -1624,10 +1612,6 @@ public Expression visitEqualityExpression(cqlParser.EqualityExpressionContext ct

libraryBuilder.resolveBinaryCall("System", "Equivalent", equivalent);

if (isAnnotationEnabled()) {
equivalent.setLocalId(Integer.toString(getNextLocalId()));
}

if (!"~".equals(parseString(ctx.getChild(1)))) {
track(equivalent, ctx);
Not not = of.createNot().withOperand(equivalent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
/**
* Created by Bryn on 12/29/2016.
*/
public class LibraryBuilder implements ModelResolver {
public class LibraryBuilder {
public static enum SignatureLevel {
/*
Indicates signatures will never be included in operator invocations
Expand All @@ -42,19 +42,17 @@ public static enum SignatureLevel {
All
}

public LibraryBuilder(LibraryManager libraryManager) {
this(null, libraryManager);
public LibraryBuilder(LibraryManager libraryManager, ObjectFactory objectFactory) {
this(null, libraryManager, objectFactory);
}

public LibraryBuilder(NamespaceInfo namespaceInfo, LibraryManager libraryManager) {
if (libraryManager == null) {
throw new IllegalArgumentException("libraryManager is null");
}
public LibraryBuilder(NamespaceInfo namespaceInfo, LibraryManager libraryManager, ObjectFactory objectFactory) {
this.libraryManager = Objects.requireNonNull(libraryManager);
this.of = Objects.requireNonNull(objectFactory);

this.namespaceInfo = namespaceInfo; // Note: allowed to be null, implies global namespace
this.modelManager = libraryManager.getModelManager();
this.libraryManager = libraryManager;
this.typeBuilder = new TypeBuilder(of, this);
this.typeBuilder = new TypeBuilder(of, this.modelManager);

this.library = of.createLibrary()
.withSchemaIdentifier(of.createVersionedIdentifier()
Expand All @@ -63,8 +61,13 @@ public LibraryBuilder(NamespaceInfo namespaceInfo, LibraryManager libraryManager

this.cqlToElmInfo = af.createCqlToElmInfo();
this.cqlToElmInfo.setTranslatorVersion(LibraryBuilder.class.getPackage().getImplementationVersion());

this.library.getAnnotation().add(this.cqlToElmInfo);

this.options = Objects.requireNonNull(
libraryManager.getCqlCompilerOptions(), "libraryManager compilerOptions can not be null.");

this.setCompilerOptions(this.options);
compiledLibrary = new CompiledLibrary();
compiledLibrary.setLibrary(library);
}
Expand Down Expand Up @@ -97,6 +100,14 @@ public List<CqlCompilerException> getExceptions() {
return exceptions;
}

public ObjectFactory getObjectFactory() {
return of;
}

public LibraryManager getLibraryManager() {
return libraryManager;
}

private final Map<String, Model> models = new LinkedHashMap<>();

private final Map<String, ResultWithPossibleError<NamedTypeSpecifier>> nameTypeSpecifiers = new HashMap<>();
Expand All @@ -108,10 +119,10 @@ public List<CqlCompilerException> getExceptions() {
private final Deque<HidingIdentifierContext> hidingIdentifiersContexts = new ArrayDeque<>();
private int literalContext = 0;
private int typeSpecifierContext = 0;
private NamespaceInfo namespaceInfo = null;
private ModelManager modelManager = null;
private final NamespaceInfo namespaceInfo;
private final ModelManager modelManager;
private Model defaultModel = null;
private LibraryManager libraryManager = null;
private final LibraryManager libraryManager;
private Library library = null;

public Library getLibrary() {
Expand All @@ -130,24 +141,18 @@ public ConversionMap getConversionMap() {
return conversionMap;
}

private final ObjectFactory of = new ObjectFactory();
private final ObjectFactory of;
private final org.hl7.cql_annotations.r1.ObjectFactory af = new org.hl7.cql_annotations.r1.ObjectFactory();
private boolean listTraversal = true;
private CqlCompilerOptions options;
private CqlToElmInfo cqlToElmInfo = null;
private TypeBuilder typeBuilder = null;
private Cql2ElmVisitor visitor = null;
private final CqlCompilerOptions options;
private final CqlToElmInfo cqlToElmInfo;
private final TypeBuilder typeBuilder;

public void enableListTraversal() {
listTraversal = true;
}

public void setCompilerOptions(CqlCompilerOptions options) {
if (options == null) {
throw new IllegalArgumentException("Options cannot be null");
}

this.options = options;
private void setCompilerOptions(CqlCompilerOptions options) {
if (options.getOptions().contains(CqlCompilerOptions.Options.DisableListTraversal)) {
this.listTraversal = false;
}
Expand All @@ -168,10 +173,6 @@ public void setCompilerOptions(CqlCompilerOptions options) {
this.cqlToElmInfo.setSignatureLevel(options.getSignatureLevel().name());
}

public void setVisitor(Cql2ElmVisitor visitor) {
this.visitor = visitor;
}

private String compatibilityLevel = null;

public boolean isCompatibilityLevel3() {
Expand Down Expand Up @@ -964,11 +965,6 @@ public Expression resolveUnion(Expression left, Expression right) {
// TODO: Take advantage of nary unions
BinaryWrapper wrapper = normalizeListTypes(left, right);
Union union = of.createUnion().withOperand(wrapper.left, wrapper.right);

if (visitor != null && visitor.isAnnotationEnabled()) {
union.setLocalId(Integer.toString(visitor.getNextLocalId()));
}

resolveNaryCall("System", "Union", union);
return union;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import org.cqframework.cql.cql2elm.model.QueryContext;
import org.cqframework.cql.gen.cqlParser;
Expand All @@ -13,21 +14,14 @@
* Created by Bryn on 12/27/2016.
*/
public class SystemMethodResolver {
private final ObjectFactory of = new ObjectFactory();
private final Cql2ElmVisitor visitor;
private final ObjectFactory of;
private final ElmGenerator visitor;
private final LibraryBuilder builder;

public SystemMethodResolver(Cql2ElmVisitor visitor, LibraryBuilder builder) {
if (visitor == null) {
throw new IllegalArgumentException("visitor is null");
}

if (builder == null) {
throw new IllegalArgumentException("builder is null");
}

this.visitor = visitor;
this.builder = builder;
public SystemMethodResolver(ElmGenerator visitor, LibraryBuilder builder) {
this.visitor = Objects.requireNonNull(visitor, "visitor required");
this.builder = Objects.requireNonNull(builder, "builder required");
this.of = Objects.requireNonNull(builder.getObjectFactory(), "builder must have an object factory");
}

private List<Expression> getParams(Expression target, cqlParser.ParamListContext ctx) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class TypeBuilder {
private ObjectFactory of;
private ModelResolver mr;

public class InternalModelResolver implements ModelResolver {
public static class InternalModelResolver implements ModelResolver {
private ModelManager modelManager;

public InternalModelResolver(ModelManager modelManager) {
Expand All @@ -33,9 +33,8 @@ public TypeBuilder(ObjectFactory of, ModelResolver mr) {
this.mr = mr;
}

public TypeBuilder(ModelManager modelManager) {
this.of = new ObjectFactory();
this.mr = new InternalModelResolver(modelManager);
public TypeBuilder(ObjectFactory of, ModelManager modelManager) {
this(of, new InternalModelResolver(modelManager));
}

public QName dataTypeToQName(DataType type) {
Expand Down
Loading
Loading