Apply private access modifier (cqframework#1239)

* add test for private access modifier * add impl for private access modifier * rectify test function name * cover private access for parameter, codesystem, valueset, code, concept, function, expression * identify possible bug * enable tests after the merged fix
Path-Check · Oct 18, 2023 · cc89145 · cc89145
1 parent a47cfc8
commit cc89145
Show file tree

Hide file tree

Showing 5 changed files with 137 additions and 9 deletions.
diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java
@@ -1,5 +1,6 @@
 package org.cqframework.cql.cql2elm;
 
+import org.apache.commons.lang3.StringUtils;
 import org.cqframework.cql.cql2elm.model.*;
 import org.cqframework.cql.cql2elm.model.invocation.*;
 import org.cqframework.cql.elm.tracking.TrackBack;
@@ -1274,20 +1275,27 @@ public OperatorResolution resolveCall(CallContext callContext) {
                     }
                 }
                 */
-
-                if (result != null) {
-                    checkAccessLevel(result.getOperator().getLibraryName(), result.getOperator().getName(),
-                            result.getOperator().getAccessLevel());
-                }
             }
         }
         else {
             result = resolveLibrary(callContext.getLibraryName()).resolveCall(callContext, conversionMap);
         }
 
+        if (result != null) {
+            checkAccessLevel(result.getOperator().getLibraryName(), result.getOperator().getName(),
+                    result.getOperator().getAccessLevel());
+        }
+
         return result;
     }
 
+    private boolean isInterFunctionAccess(String f1, String f2) {
+        if(StringUtils.isNoneBlank(f1) && StringUtils.isNoneBlank(f2)) {
+            return !f1.equalsIgnoreCase(f2);
+        }
+        return false;
+    }
+
     public void checkOperator(CallContext callContext, OperatorResolution resolution) {
         if (resolution == null) {
             // ERROR:
@@ -1307,9 +1315,10 @@ public void checkOperator(CallContext callContext, OperatorResolution resolution
     }
 
     public void checkAccessLevel(String libraryName, String objectName, AccessModifier accessModifier) {
-        if (accessModifier == AccessModifier.PRIVATE) {
+        if (accessModifier == AccessModifier.PRIVATE &&
+                isInterFunctionAccess(this.library.getIdentifier().getId(), libraryName)) {
             // ERROR:
-            throw new IllegalArgumentException(String.format("Object %s in library %s is marked private and cannot be referenced from another library.", objectName, libraryName));
+            throw new CqlSemanticException(String.format("Identifier %s in library %s is marked private and cannot be referenced from another library.", objectName, libraryName));
         }
     }
 
@@ -2732,7 +2741,7 @@ public Expression resolveAccessor(Expression left, String memberIdentifier) {
             }
 
             if (element instanceof CodeSystemDef) {
-                checkAccessLevel(libraryName, memberIdentifier, ((CodeSystemDef)element).getAccessLevel());
+                checkAccessLevel(libraryName, memberIdentifier, ((CodeSystemDef) element).getAccessLevel());
                 CodeSystemRef result = of.createCodeSystemRef()
                         .withLibraryName(libraryName)
                         .withName(memberIdentifier);

diff --git a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/LibraryTests.java b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/LibraryTests.java
@@ -12,11 +12,16 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.cqframework.cql.cql2elm.LibraryBuilder.SignatureLevel;
 import org.hl7.cql_annotations.r1.CqlToElmError;
 import org.hl7.elm.r1.*;
-import org.testng.annotations.*;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
 
 public class LibraryTests {
 
@@ -142,6 +147,31 @@ public void testInvalidLibraryReferences() {
         }
     }
 
+    @Test
+    public void testPrivateAccessModifierReferencing() throws IOException {
+        CqlTranslator translator = TestUtils.createTranslatorFromStream("LibraryTests/AccessModifierReferencing.cql");
+        assertThat(translator.getErrors().size(), is(not(0)));
+
+        Set<String> errors = translator.getErrors().stream()
+                .map(CqlCompilerException::getMessage)
+                .collect(Collectors.toSet());
+
+        assertTrue(errors.contains("Identifier ICD-10:2014 in library Base is marked private and cannot be referenced from another library."));
+        assertTrue(errors.contains("Identifier f1 in library AccessModifierBase is marked private and cannot be referenced from another library."));
+        assertTrue(errors.contains("Identifier PrivateExpression in library Base is marked private and cannot be referenced from another library."));
+        assertTrue(errors.contains("Identifier Test Parameter in library Base is marked private and cannot be referenced from another library."));
+        assertTrue(errors.contains("Identifier Female Administrative Sex in library Base is marked private and cannot be referenced from another library."));
+        assertTrue(errors.contains("Identifier XYZ Code in library Base is marked private and cannot be referenced from another library."));
+        assertTrue(errors.contains("Identifier XYZ Concept in library Base is marked private and cannot be referenced from another library."));
+
+    }
+
+    @Test
+    public void testPrivateAccessModifierNonReferencing() throws IOException {
+        CqlTranslator translator = TestUtils.createTranslatorFromStream("LibraryTests/AccessModifierNonReferencing.cql");
+        assertThat(translator.getErrors().size(), is(0));
+    }
+
     @Test
     public void testInvalidLibraryReference() {
         CqlTranslator translator = null;

diff --git a/...to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierBase.cql b/...to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierBase.cql
@@ -0,0 +1,19 @@
+library AccessModifierBase
+
+private codesystem "ICD-10:2014": 'ICD-10' version '2014'
+
+private codesystem "SNOMED-CT:2020": 'http://snomed.info/sct' version '2020'
+private codesystem "ICD-9CM:2020": '2.16.840.1.113883.6.103' version '2020'
+
+private code "XYZ Code": 'XYZ' from "SNOMED-CT:2020" display 'XYZ Code'
+private code "ABC Code": 'ABC' from "ICD-9CM:2020" display 'ABC Code'
+
+private concept "XYZ Concept": { "XYZ Code", "ABC Code" } display 'XYZ Concept'
+
+private valueset "Female Administrative Sex": '2.16.840.1.113883.3.560.100.2'
+
+private parameter "Test Parameter" Integer
+
+define private function f1(arg String): arg
+
+define private PrivateExpression: Tuple { Id : '12345', Name : 'John Doe' }
diff --git a/.../test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierNonReferencing.cql b/.../test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierNonReferencing.cql
@@ -0,0 +1,44 @@
+library AccessModifierNonReferencing
+
+//********private member definition begin**********
+private codesystem "ICD-10:2014": 'ICD-10' version '2014'
+
+private codesystem "SNOMED-CT:2020": 'http://snomed.info/sct' version '2020'
+private codesystem "ICD-9CM:2020": '2.16.840.1.113883.6.103' version '2020'
+
+private code "XYZ Code": 'XYZ' from "SNOMED-CT:2020" display 'XYZ Code'
+private code "ABC Code": 'ABC' from "ICD-9CM:2020" display 'ABC Code'
+
+private concept "XYZ Concept": { "XYZ Code", "ABC Code" } display 'XYZ Concept'
+
+private valueset "Female Administrative Sex": '2.16.840.1.113883.3.560.100.2'
+//private member access for codesystem
+valueset "Chlamydia Screening": '2.16.840.1.113883.3.464.1003.110.12.1052'
+    codesystems { "ICD-10:2014" }
+
+private parameter "Test Parameter" Integer
+
+define private function f1(arg String): arg
+
+define private PrivateExpression: Tuple { Id : '12345', Name : 'John Doe' }
+
+//********private member definition end**********
+
+//private member access for function
+define FunctionTestOuterPrivate: f1('hello')
+
+//private member access for expressionDef
+define ReferenceExpression: PrivateExpression.Id
+
+//private member access for parameter
+define "Test Definition": "Test Parameter"
+
+//private member access for valueset
+define GenderExpression: 'Female' in "Female Administrative Sex"
+
+//private member access for code
+define CodeRef: "XYZ Code"
+
+//private member access for concept
+define ConceptRef: "XYZ Concept"
+
diff --git a/...src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierReferencing.cql b/...src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierReferencing.cql
@@ -0,0 +1,26 @@
+library AccessModifierReferencing
+
+include AccessModifierBase called Base
+
+//private member access for codesystem
+valueset "Chlamydia Screening": '2.16.840.1.113883.3.464.1003.110.12.1052'
+    codesystems { Base."ICD-10:2014" }
+
+//private member access for function
+define FunctionTestOuterPrivate: Base.f1('hello')
+
+//private member access for expressionDef
+define ReferenceExpression: Base.PrivateExpression.Id
+
+//private member access for parameter
+define "Test Definition": Base."Test Parameter"
+
+//private member access for valueset
+define GenderExpression: 'Female' in Base."Female Administrative Sex"
+
+//private member access for code
+define CodeRef: Base."XYZ Code"
+
+//private member access for concept
+define ConceptRef: Base."XYZ Concept"
+