Release 3.6.0, HAPI 6.10.2, fixes for build warnings (#1318)

* Prep for release 3.6.0, fix some build warnings

* Fixup formatting

* Fixes for identifier hiding

* Fixes for Android not having all the 11 APIs

Src/java/.vscode/settings.json

@@ -9,18 +9,23 @@
  "bools",
  "Bryn",
  "checkstyle",
+ "Codeable",
+ "Codesystem",
  "cqframework",
  "datumedge",
  "fhir",
  "fhirpath",
  "hamcrest",
  "Inferencing",
  "Instancio",
+ "JAXB",
+ "modelinfo",
  "Objenesis",
  "opdef",
  "opencds",
  "qicore",
  "Randomizer",
+ "redeclaration",
  "testng",
  "tngtech",
  "trackback",

Src/java/buildSrc/src/main/groovy/cql.sca-conventions.gradle

@@ -27,6 +27,7 @@ tasks.withType(JavaCompile).configureEach {
 
 tasks {
  compileTestJava {
+ // TODO: Talk to the team about warnings in tests
  options.errorprone.disableAllWarnings = true
  options.errorprone.disableWarningsInGeneratedCode = true
  }

Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/HidingIdentifierContext.java → Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/IdentifierContext.java

@@ -7,11 +7,11 @@
 /**
  * Simple POJO using for identifier hider that maintains the identifier and Trackable type of the construct being evaluated.
  */
-public class HidingIdentifierContext {
+public class IdentifierContext {
  private final String identifier;
  private final Class<? extends Trackable> elementSubclass;
 
- public HidingIdentifierContext(String identifier, Class<? extends Trackable> elementSubclass) {
+ public IdentifierContext(String identifier, Class<? extends Trackable> elementSubclass) {
  this.identifier = identifier;
  this.elementSubclass = elementSubclass;
  }
@@ -32,7 +32,7 @@ public boolean equals(Object other) {
  if (other == null || getClass() != other.getClass()) {
  return false;
  }
- HidingIdentifierContext that = (HidingIdentifierContext) other;
+ IdentifierContext that = (IdentifierContext) other;
  return Objects.equals(identifier, that.identifier) && Objects.equals(elementSubclass, that.elementSubclass);
  }
 
@@ -43,7 +43,7 @@ public int hashCode() {
 
  @Override
  public String toString() {
- return new StringJoiner(", ", HidingIdentifierContext.class.getSimpleName() + "[", "]")
+ return new StringJoiner(", ", IdentifierContext.class.getSimpleName() + "[", "]")
  .add("identifier='" + identifier + "'")
  .add("elementSubclass=" + elementSubclass)
  .toString();

Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java

@@ -20,7 +20,7 @@
  * Created by Bryn on 12/29/2016.
  */
 public class LibraryBuilder {
- public static enum SignatureLevel {
+ public enum SignatureLevel {
  /*
  Indicates signatures will never be included in operator invocations
  */
@@ -116,7 +116,8 @@ public LibraryManager getLibraryManager() {
  private final Stack<String> expressionContext = new Stack<>();
  private final ExpressionDefinitionContextStack expressionDefinitions = new ExpressionDefinitionContextStack();
  private final Stack<FunctionDef> functionDefs = new Stack<>();
- private final Deque<HidingIdentifierContext> hidingIdentifiersContexts = new ArrayDeque<>();
+ private final Deque<IdentifierContext> globalIdentifiers = new ArrayDeque<>();
+ private final Stack<Deque<IdentifierContext>> localIdentifierStack = new Stack<>();
  private int literalContext = 0;
  private int typeSpecifierContext = 0;
  private final NamespaceInfo namespaceInfo;
@@ -3127,6 +3128,11 @@ private DataType getExpressionDefResultType(ExpressionDef expressionDef) {
  currentExpressionContext(), expressionDef.getContext()));
  }
 
+ public enum IdentifierScope {
+ GLOBAL,
+ LOCAL
+ }
+
  /**
  * Add an identifier to the deque to indicate that we are considering it for consideration for identifier hiding and
  * adding a compiler warning if this is the case.
@@ -3140,46 +3146,93 @@ private DataType getExpressionDefResultType(ExpressionDef expressionDef) {
  * <p/>
  * Also, special case function overloads so that only a single overloaded function name is taken into account.
  *
+ * Default scope is {@link IdentifierScope#LOCAL}
+ *
  * @param identifier The identifier belonging to the parameter, expression, function, alias, etc, to be evaluated.
  * @param trackable The construct trackable, for example {@link ExpressionRef}.
  */
- void pushIdentifierForHiding(String identifier, Trackable trackable) {
- final boolean hasRelevantMatch = hidingIdentifiersContexts.stream()
- .filter(innerContext -> innerContext.getIdentifier().equals(identifier))
- .peek(matchedContext -> {
- // If we are passing an overloaded function definition, do not issue a warning
- if (trackable instanceof FunctionDef
- && FunctionDef.class == matchedContext.getTrackableSubclass()) {
- return;
- }
+ void pushIdentifier(String identifier, Trackable trackable) {
+ pushIdentifier(identifier, trackable, IdentifierScope.LOCAL);
+ }
 
- reportWarning(
- resolveWarningMessage(matchedContext.getIdentifier(), identifier, trackable), trackable);
- })
- .findAny()
- .isPresent();
+ /**
+ * Add an identifier to the deque to indicate that we are considering it for consideration for identifier hiding and
+ * adding a compiler warning if this is the case.
+ * <p/>
+ * For example, if an alias within an expression body has the same name as a parameter, execution would have
+ * added the parameter identifier and the next execution would consider an alias with the same name, thus resulting
+ * in a warning.
+ * <p/>
+ * Exact case matching as well as case-insensitive matching are considered. If known, the type of the structure
+ * in question will be considered in crafting the warning message, as per the {@link Element} parameter.
+ * <p/>
+ * Also, special case function overloads so that only a single overloaded function name is taken into account.
+ *
+ * @param identifier The identifier belonging to the parameter, expression, function, alias, etc, to be evaluated.
+ * @param trackable The construct trackable, for example {@link ExpressionRef}.
+ * @param scope the scope of the current identifier
+ */
+ void pushIdentifier(String identifier, Trackable trackable, IdentifierScope scope) {
+ var localMatch = !localIdentifierStack.isEmpty()
+ ? findMatchingIdentifierContext(localIdentifierStack.peek(), identifier)
+ : Optional.<IdentifierContext>empty();
+ var globalMatch = findMatchingIdentifierContext(globalIdentifiers, identifier);
+
+ if (globalMatch.isPresent() || localMatch.isPresent()) {
+ var matchedContext = globalMatch.isPresent() ? globalMatch.get() : localMatch.get();
+
+ boolean matchedOnFunctionOverloads =
+ matchedContext.getTrackableSubclass().equals(FunctionDef.class) && trackable instanceof FunctionDef;
+
+ if (!matchedOnFunctionOverloads) {
+ reportWarning(resolveWarningMessage(matchedContext.getIdentifier(), identifier, trackable), trackable);
+ }
+ }
 
- // We will only add function definitions once
- if (!(trackable instanceof FunctionDef) || !hasRelevantMatch) {
- if (shouldPushHidingContext(trackable)) {
-  final Class<? extends Trackable> trackableOrNull = trackable != null ? trackable.getClass() : null;
- // Sometimes the underlying Trackable doesn't resolve in the calling code
-  hidingIdentifiersContexts.push(
-  new HidingIdentifierContext(identifier, trackable != null ? trackable.getClass() : null));
+ if (shouldAddIdentifierContext(trackable)) {
+  final Class<? extends Trackable> trackableOrNull = trackable != null ? trackable.getClass() : null;
+ // Sometimes the underlying Trackable doesn't resolve in the calling code
+ if (scope == IdentifierScope.GLOBAL) {
+ globalIdentifiers.push(new IdentifierContext(identifier, trackableOrNull));
+ } else {
+ localIdentifierStack.peek().push(new IdentifierContext(identifier, trackableOrNull));
  }
  }
  }
 
+ private Optional<IdentifierContext> findMatchingIdentifierContext(
+ Collection<IdentifierContext> identifierContext, String identifier) {
+ return identifierContext.stream()
+ .filter(innerContext -> innerContext.getIdentifier().equals(identifier))
+ .findFirst();
+ }
+
  /**
  * Pop the last resolved identifier off the deque. This is needed in case of a context in which an identifier
- * falls out of scope, for an example, an alias within an expression or function body
+ * falls out of scope, for an example, an alias within an expression or function body.
  */
- void popIdentifierForHiding() {
- hidingIdentifiersContexts.pop();
+ void popIdentifier() {
+ popIdentifier(IdentifierScope.LOCAL);
+ }
+
+ void popIdentifier(IdentifierScope scope) {
+ if (scope == IdentifierScope.GLOBAL) {
+ globalIdentifiers.pop();
+ } else {
+ localIdentifierStack.peek().pop();
+ }
+ }
+
+ void pushIdentifierScope() {
+ localIdentifierStack.push(new ArrayDeque<>());
+ }
+
+ void popIdentifierScope() {
+ localIdentifierStack.pop();
  }
 
  // TODO: consider other structures that should only trigger a readonly check of identifier hiding
- private boolean shouldPushHidingContext(Trackable trackable) {
+ private boolean shouldAddIdentifierContext(Trackable trackable) {
  return !(trackable instanceof Literal);
  }
 

Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/fhir/r4/BaseTest.java

@@ -5,12 +5,14 @@
 import static org.cqframework.cql.cql2elm.matchers.QuickDataType.quickDataType;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.assertEquals;
 
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 import org.cqframework.cql.cql2elm.CqlCompilerOptions;
 import org.cqframework.cql.cql2elm.CqlTranslator;
+import org.cqframework.cql.cql2elm.LibraryBuilder.SignatureLevel;
 import org.cqframework.cql.cql2elm.TestUtils;
 import org.cqframework.cql.cql2elm.model.CompiledLibrary;
 import org.hl7.elm.r1.*;
@@ -323,4 +325,11 @@ public void testRetrieveWithConcept() throws IOException {
  ToList toList = (ToList) retrieve.getCodes();
  assertThat(toList.getOperand(), instanceOf(CodeRef.class));
  }
+
+ @Test
+ public void testExm108IdentifierHiding() throws IOException {
+ var translator = TestUtils.runSemanticTest("fhir/r4/exm108/EXM108.cql", 0, SignatureLevel.All);
+ // Should only be one identifier being hid after fixes, "Warafin"
+ assertEquals(1, translator.getExceptions().size());
+ }
 }