Repository: FHIR/sushi (ts)

Issue:
Setting an extension on a StructureDefinition
We're trying to set the value of an extension on the underlying StructureDefinition for a profile we're defining by using the "escape syntax" (^):
```
Profile: NewOnsetOfT2DMDataElementCPG3
Parent: Observation
* ^extension[inferenceExpression].valueExpression.language = #text/cql
* ^extension[inferenceExpression].valueExpression.expression =
  "exists([Condition in \"T2DMConditionVS\"] c where c.onset is within last 2 months)"
* code = ExampleCdsCS#t2dm
```

Which builds fine, but it seems to be treating the `.valueExpression.language` part as constraints, rather than instance assignments...
```
"extension": [
    {
      "_sliceName": "inferenceExpression",
      "valueExpression": {
        "language": {
          "fixedValue": "text/cql",
          "_primitive": true
        },
        "expression": {
          "fixedValue": "exists([Condition in \"T2DMConditionVS\"] c where c.onset is within last 2 months)",
          "_primitive": true
        }
      }
    }
  ]
```


Pull request description:
Clean up StructureDefinition.extension during export
Fixes #412 and completes [CIMPL-384](https://standardhealthrecord.atlassian.net/browse/CIMPL-384)

When rules are applied to a StructureDefinition's `extension`, they should look the same way that they would as if those rules were on an instance: that is, they should look like regular values, and not include the extra properties that get added during processing.

Because cleaning is now being applied to StructureDefinitions as well as Instances, this PR moves a lot of the cleaning-up functions to the shared `common.ts`.

Patch:
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index 6568ef7..fd038d9 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -5,14 +5,14 @@ import { logger, Fishable, Type, Metadata } from '../utils';
 import {
   setPropertyOnInstance,
   replaceReferences,
-  replaceField,
+  cleanResource,
   splitOnPathPeriods,
   applyMixinRules,
-  getAllImpliedPaths
+  setImpliedPropertiesOnInstance
 } from '../fhirtypes/common';
 import { InstanceOfNotDefinedError } from '../errors/InstanceOfNotDefinedError';
 import { Package } from '.';
-import { isEmpty, cloneDeep } from 'lodash';
+import { cloneDeep } from 'lodash';
 
 export class InstanceExporter implements Fishable {
   constructor(
@@ -110,63 +110,8 @@ export class InstanceExporter implements Fishable {
       }
     });
 
-    // Record the values implied by the structure definition in sdRuleMap
-    const sdRuleMap: Map<string, any> = new Map();
     const paths = ['', ...rules.map(rule => rule.path)];
-    paths.forEach(path => {
-      const nonNumericPath = path.replace(/\[[-+]?\d+\]/g, '');
-      const element = instanceOfStructureDefinition.findElementByPath(nonNumericPath, this.fisher);
-      if (element) {
-        // If an element is pointed to by a path, that means we must fix values on its parents, its own
-        // fixable descendents, and fixable descenendents of its parents. A fixable descendent is a 1..n direct child
-        // or a fixable descendent of such a child
-        const parents = element.getAllParents();
-        const associatedElements = [...element.getFixableDescendents(), ...parents];
-        parents.map(p => p.getFixableDescendents()).forEach(pd => associatedElements.push(...pd));
-
-        for (const associatedEl of associatedElements) {
-          const fixedValueKey = Object.keys(associatedEl).find(
-            k => k.startsWith('fixed') || k.startsWith('pattern')
-          );
-          const foundFixedValue = cloneDeep(associatedEl[fixedValueKey as keyof ElementDefinition]);
-          if (foundFixedValue) {
-            // Find how much the two paths overlap, for example, a.b.c, and a.b.d overlap for a.b
-            let overlapIdx = 0;
-            const elParts = element.id.split('.');
-            const assocElParts = associatedEl.id.split('.');
-            for (
-              ;
-              overlapIdx < elParts.length && elParts[overlapIdx] === assocElParts[overlapIdx];
-              overlapIdx++
-            );
-            // We must keep the relevant portion of the beginning of path to preserve sliceNames
-            // and combine this with portion of the associatedEl's path that is not overlapped
-            const pathStart = splitOnPathPeriods(path).slice(0, overlapIdx - 1);
-            const pathEnd = associatedEl
-              .diffId()
-              .split('.')
-              .slice(overlapIdx)
-              // Replace FHIR slicing with FSH slicing, a:b.c:d -> a[b].c[d]
-              .map(r => r.replace(/(\S):(\S+)/, (match, c1, c2) => `${c1}[${c2}]`));
-            const finalPath = [...pathStart, ...pathEnd].join('.');
-            const impliedPaths = getAllImpliedPaths(
-              associatedEl,
-              // Implied paths are found via the index free path
-              finalPath.replace(/\[[-+]?\d+\]/g, '')
-            );
-            [finalPath, ...impliedPaths].forEach(ip => sdRuleMap.set(ip, foundFixedValue));
-          }
-        }
-      }
-    });
-    sdRuleMap.forEach((value, path) => {
-      const { pathParts } = instanceOfStructureDefinition.validateValueAtPath(
-        path,
-        null,
-        this.fisher
-      );
-      setPropertyOnInstance(instanceDef, pathParts, value);
-    });
+    setImpliedPropertiesOnInstance(instanceDef, instanceOfStructureDefinition, paths, this.fisher);
     ruleMap.forEach(rule => setPropertyOnInstance(instanceDef, rule.pathParts, rule.fixedValue));
     return instanceDef;
   }
@@ -246,32 +191,6 @@ export class InstanceExporter implements Fishable {
     this.validateRequiredChildElements(instanceDef, elements[0], fshDefinition);
   }
 
-  /**
-   * Cleans up temporary properties that were added to the InstanceDefinition during processing
-   * @param {InstanceDefinition} instanceDef - The InstanceDefinition to clean
-   */
-  private cleanInstance(instanceDef: InstanceDefinition): void {
-    // Remove all _sliceName fields
-    replaceField(
-      instanceDef,
-      (o, p) => p === '_sliceName',
-      (o, p) => delete o[p]
-    );
-    // Change any {} to null
-    replaceField(
-      instanceDef,
-      (o, p) => typeof o[p] === 'object' && o[p] !== null && isEmpty(o[p]),
-      (o, p) => (o[p] = null)
-    );
-
-    // Change back any primitives that have been converted into objects by setPropertyOnInstance
-    replaceField(
-      instanceDef,
-      (o, p) => typeof o[p] === 'object' && o[p] !== null && o[p]._primitive,
-      (o, p) => (o[p] = o[p].fixedValue)
-    );
-  }
-
   fishForFHIR(item: string) {
     let result = this.fisher.fishForFHIR(item, Type.Instance);
     if (result == null) {
@@ -344,7 +263,7 @@ export class InstanceExporter implements Fishable {
       instanceOfStructureDefinition.elements,
       fshDefinition
     );
-    this.cleanInstance(instanceDef);
+    cleanResource(instanceDef);
     return instanceDef;
   }
 
diff --git a/src/export/StructureDefinitionExporter.ts b/src/export/StructureDefinitionExporter.ts
index 628bdbf..5d0dd4c 100644
--- a/src/export/StructureDefinitionExporter.ts
+++ b/src/export/StructureDefinitionExporter.ts
@@ -25,7 +25,12 @@ import {
   ObeysRule
 } from '../fshtypes/rules';
 import { logger, Type, Fishable, Metadata, MasterFisher } from '../utils';
-import { replaceReferences, splitOnPathPeriods, applyMixinRules } from '../fhirtypes/common';
+import {
+  replaceReferences,
+  splitOnPathPeriods,
+  applyMixinRules,
+  cleanResource
+} from '../fhirtypes/common';
 import { Package } from './Package';
 
 /**
@@ -476,6 +481,7 @@ export class StructureDefinitionExporter implements Fishable {
     this.preprocessStructureDefinition(fshDefinition, structDef.type === 'Extension');
 
     this.setRules(structDef, fshDefinition);
+    cleanResource(structDef, (prop: string) => prop == 'elements' || prop.indexOf('_') == 0);
     structDef.inProgress = false;
 
     return structDef;
diff --git a/src/fhirtypes/common.ts b/src/fhirtypes/common.ts
index 9dfb5b5..186d9f0 100644
--- a/src/fhirtypes/common.ts
+++ b/src/fhirtypes/common.ts
@@ -21,6 +21,7 @@ import { Type, Fishable } from '../utils/Fishable';
 import cloneDeep = require('lodash/cloneDeep');
 import { logger } from '../utils';
 import { FHIRId, idRegex } from './primitiveTypes';
+import { isEmpty } from 'lodash';
 
 export function splitOnPathPeriods(path: string): string[] {
   return path.split(/\.(?![^\[]*\])/g); // match a period that isn't within square brackets
@@ -41,9 +42,70 @@ export function setPropertyOnDefinitionInstance(
 ): void {
   const instanceSD = instance.getOwnStructureDefinition(fisher);
   const { fixedValue, pathParts } = instanceSD.validateValueAtPath(path, value, fisher);
+  setImpliedPropertiesOnInstance(instance, instanceSD, [path], fisher);
   setPropertyOnInstance(instance, pathParts, fixedValue);
 }
 
+export function setImpliedPropertiesOnInstance(
+  instanceDef: StructureDefinition | ElementDefinition | InstanceDefinition,
+  instanceOfStructureDefinition: StructureDefinition,
+  paths: string[],
+  fisher: Fishable
+) {
+  // Record the values implied by the structure definition in sdRuleMap
+  const sdRuleMap: Map<string, any> = new Map();
+  paths.forEach(path => {
+    const nonNumericPath = path.replace(/\[[-+]?\d+\]/g, '');
+    const element = instanceOfStructureDefinition.findElementByPath(nonNumericPath, fisher);
+    if (element) {
+      // If an element is pointed to by a path, that means we must fix values on its parents, its own
+      // fixable descendents, and fixable descenendents of its parents. A fixable descendent is a 1..n direct child
+      // or a fixable descendent of such a child
+      const parents = element.getAllParents();
+      const associatedElements = [...element.getFixableDescendents(), ...parents];
+      parents.map(p => p.getFixableDescendents()).forEach(pd => associatedElements.push(...pd));
+
+      for (const associatedEl of associatedElements) {
+        const fixedValueKey = Object.keys(associatedEl).find(
+          k => k.startsWith('fixed') || k.startsWith('pattern')
+        );
+        const foundFixedValue = cloneDeep(associatedEl[fixedValueKey as keyof ElementDefinition]);
+        if (foundFixedValue) {
+          // Find how much the two paths overlap, for example, a.b.c, and a.b.d overlap for a.b
+          let overlapIdx = 0;
+          const elParts = element.id.split('.');
+          const assocElParts = associatedEl.id.split('.');
+          for (
+            ;
+            overlapIdx < elParts.length && elParts[overlapIdx] === assocElParts[overlapIdx];
+            overlapIdx++
+          );
+          // We must keep the relevant portion of the beginning of path to preserve sliceNames
+          // and combine this with portion of the associatedEl's path that is not overlapped
+          const pathStart = splitOnPathPeriods(path).slice(0, overlapIdx - 1);
+          const pathEnd = associatedEl
+            .diffId()
+            .split('.')
+            .slice(overlapIdx)
+            // Replace FHIR slicing with FSH slicing, a:b.c:d -> a[b].c[d]
+            .map(r => r.replace(/(\S):(\S+)/, (match, c1, c2) => `${c1}[${c2}]`));
+          const finalPath = [...pathStart, ...pathEnd].join('.');
+          const impliedPaths = getAllImpliedPaths(
+            associatedEl,
+            // Implied paths are found via the index free path
+            finalPath.replace(/\[[-+]?\d+\]/g, '')
+          );
+          [finalPath, ...impliedPaths].forEach(ip => sdRuleMap.set(ip, foundFixedValue));
+        }
+      }
+    }
+  });
+  sdRuleMap.forEach((value, path) => {
+    const { pathParts } = instanceOfStructureDefinition.validateValueAtPath(path, null, fisher);
+    setPropertyOnInstance(instanceDef, pathParts, value);
+  });
+}
+
 export function setPropertyOnInstance(
   instance: StructureDefinition | ElementDefinition | InstanceDefinition | ValueSet | CodeSystem,
   pathParts: PathPart[],
@@ -69,7 +131,7 @@ export function setPropertyOnInstance(
           if (typeof fixedValue !== 'object') {
             // When a fixedValue is a primitive but also a slice, we convert to an object so that
             // the sliceName field can be tracked on the object. The _primitive field marks the object
-            // to later be converted back to a primitive by replaceField in cleanInstance
+            // to later be converted back to a primitive by replaceField in cleanResource
             fixedValue = { fixedValue, _primitive: true };
           }
           const sliceIndices: number[] = [];
@@ -204,21 +266,56 @@ export function getSliceName(pathPart: PathPart): string {
  * @param { {[key: string]: any} } object - The object to replace fields on
  * @param {(object: { [key: string]: any }, prop: string) => boolean} matchFn - The function to match with
  * @param {(object: { [key: string]: any }, prop: string) => void} replaceFn - The function to replace with
+ * @param {string => boolean} skipFn - A function that returns true if a property should not be traversed
  */
 export function replaceField(
   object: { [key: string]: any },
   matchFn: (object: { [key: string]: any }, prop: string) => boolean,
-  replaceFn: (object: { [key: string]: any }, prop: string) => void
+  replaceFn: (object: { [key: string]: any }, prop: string) => void,
+  skipFn: (prop: string) => boolean
 ): void {
   for (const prop in object) {
     if (matchFn(object, prop)) {
       replaceFn(object, prop);
-    } else if (typeof object[prop] === 'object') {
-      replaceField(object[prop], matchFn, replaceFn);
+    } else if (typeof object[prop] === 'object' && !skipFn(prop)) {
+      replaceField(object[prop], matchFn, replaceFn, skipFn);
     }
   }
 }
 
+/**
+ * Cleans up temporary properties that were added to the resource definition during processing
+ * @param {StructureDefinition | InstanceDefinition} resourceDef - The resource definition to clean
+ * @param {string => boolean} skipFn - A function that returns true if a property should not be traversed
+ */
+export function cleanResource(
+  resourceDef: StructureDefinition | InstanceDefinition,
+  skipFn: (prop: string) => boolean = () => false
+): void {
+  // Remove all _sliceName fields
+  replaceField(
+    resourceDef,
+    (o, p) => p === '_sliceName',
+    (o, p) => delete o[p],
+    skipFn
+  );
+  // Change any {} to null
+  replaceField(
+    resourceDef,
+    (o, p) => typeof o[p] === 'object' && o[p] !== null && isEmpty(o[p]),
+    (o, p) => (o[p] = null),
+    skipFn
+  );
+
+  // Change back any primitives that have been converted into objects by setPropertyOnInstance
+  replaceField(
+    resourceDef,
+    (o, p) => typeof o[p] === 'object' && o[p] !== null && o[p]._primitive,
+    (o, p) => (o[p] = o[p].fixedValue),
+    skipFn
+  );
+}
+
 /**
  * Adds Mixin rules onto a Profile, Extension, or Instance
  * @param {Profile | Extension | Instance} fshDefinition - The definition to apply mixin rules on
@@ -280,7 +377,7 @@ export function getAllImpliedPaths(element: ElementDefinition, path: string): st
     } else {
       // If min >= 1, the parent or its parents my have implied paths, recursively find those
       parentPaths.push(
-        ...this.getAllImpliedPaths(parent, splitOnPathPeriods(path).slice(0, -1).join('.'))
+        ...getAllImpliedPaths(parent, splitOnPathPeriods(path).slice(0, -1).join('.'))
       );
     }
   } else {


Test patch:
diff --git a/test/export/StructureDefinitionExporter.test.ts b/test/export/StructureDefinitionExporter.test.ts
index d125509..9fda2ce 100644
--- a/test/export/StructureDefinitionExporter.test.ts
+++ b/test/export/StructureDefinitionExporter.test.ts
@@ -2221,6 +2221,20 @@ describe('StructureDefinitionExporter', () => {
     expect(sd.description).toBe('foo');
   });
 
+  it('should apply a CaretValueRule on the child of a primitive element without a path', () => {
+    const profile = new Profile('SpecialUrlId');
+    profile.parent = 'Observation';
+
+    const rule = new CaretValueRule('');
+    rule.caretPath = 'url.id';
+    rule.value = 'my-id';
+    profile.rules.push(rule);
+
+    exporter.exportStructDef(profile);
+    const sd = pkg.profiles[0];
+    expect(sd).toHaveProperty('_url', { id: 'my-id' });
+  });
+
   it('should not apply an invalid CaretValueRule on an element without a path', () => {
     const profile = new Profile('Foo');
     profile.parent = 'Observation';
@@ -2237,6 +2251,30 @@ describe('StructureDefinitionExporter', () => {
     expect(loggerSpy.getLastMessage()).toMatch(/File: InvalidValue\.fsh.*Line: 6\D*/s);
   });
 
+  it('should apply a CaretValueRule on an extension element without a path', () => {
+    // Extension: SpecialExtension
+    const extension = new Extension('SpecialExtension');
+    doc.extensions.set(extension.name, extension);
+    // Profile: HasSpecialExtension
+    // Parent: Observation
+    // * ^extension[SpecialExtension].valueString = "This is the special extension on the structure definition."
+    const profile = new Profile('HasSpecialExtension');
+    profile.parent = 'Observation';
+    const caretValueRule = new CaretValueRule('');
+    caretValueRule.caretPath = 'extension[SpecialExtension].valueString';
+    caretValueRule.value = 'This is the special extension on the structure definition.';
+    profile.rules.push(caretValueRule);
+
+    exporter.exportStructDef(profile);
+    const sd = pkg.profiles[0];
+    const extensionElement = sd.extension[0];
+    expect(extensionElement).toBeDefined();
+    expect(extensionElement).toEqual({
+      url: 'http://example.com/StructureDefinition/SpecialExtension',
+      valueString: 'This is the special extension on the structure definition.'
+    });
+  });
+
   // ObeysRule
   it('should apply an ObeysRule at the specified path', () => {
     const profile = new Profile('Foo');
diff --git a/test/testhelpers/testdefs/package/StructureDefinition-uri.json b/test/testhelpers/testdefs/package/StructureDefinition-uri.json
new file mode 100644
index 0000000..82e4d15
--- /dev/null
+++ b/test/testhelpers/testdefs/package/StructureDefinition-uri.json
@@ -0,0 +1,1 @@
+{"resourceType":"StructureDefinition","id":"uri","meta":{"lastUpdated":"2019-11-01T09:29:23.356+11:00"},"extension":[{"url":"http://hl7.org/fhir/StructureDefinition/structuredefinition-standards-status","valueCode":"normative"},{"url":"http://hl7.org/fhir/StructureDefinition/structuredefinition-normative-version","valueCode":"4.0.0"}],"url":"http://hl7.org/fhir/StructureDefinition/uri","version":"4.0.1","name":"uri","status":"active","date":"2019-11-01T09:29:23+11:00","publisher":"HL7 FHIR Standard","contact":[{"telecom":[{"system":"url","value":"http://hl7.org/fhir"}]}],"description":"Base StructureDefinition for uri Type: String of characters used to identify a name or a resource","fhirVersion":"4.0.1","kind":"primitive-type","abstract":false,"type":"uri","baseDefinition":"http://hl7.org/fhir/StructureDefinition/Element","derivation":"specialization","snapshot":{"element":[{"id":"uri","path":"uri","short":"Primitive Type uri","definition":"String of characters used to identify a name or a resource","comment":"see http://en.wikipedia.org/wiki/Uniform_resource_identifier","min":0,"max":"*","base":{"path":"uri","min":0,"max":"*"},"constraint":[{"key":"ele-1","severity":"error","human":"All FHIR elements must have a @value or children","expression":"hasValue() or (children().count() > id.count())","xpath":"@value|f:*|h:div","source":"http://hl7.org/fhir/StructureDefinition/Element"}],"isModifier":false,"isSummary":false},{"id":"uri.id","path":"uri.id","representation":["xmlAttr"],"short":"xml:id (or equivalent in JSON)","definition":"unique id for the element within a resource (for internal references)","min":0,"max":"1","base":{"path":"Element.id","min":0,"max":"1"},"type":[{"extension":[{"url":"http://hl7.org/fhir/StructureDefinition/structuredefinition-fhir-type","valueUrl":"string"}],"code":"http://hl7.org/fhirpath/System.String"}],"isModifier":false,"isSummary":false},{"id":"uri.extension","path":"uri.extension","short":"Additional content defined by implementations","definition":"May be used to represent additional information that is not part of the basic definition of the resource. To make the use of extensions safe and manageable, there is a strict set of governance  applied to the definition and use of extensions. Though any implementer can define an extension, there is a set of requirements that SHALL be met as part of the definition of the extension.","comment":"There can be no stigma associated with the use of extensions by any application, project, or standard - regardless of the institution or jurisdiction that uses or defines the extensions.  The use of extensions is what allows the FHIR specification to retain a core level of simplicity for everyone.","alias":["extensions","user content"],"min":0,"max":"*","base":{"path":"Element.extension","min":0,"max":"*"},"type":[{"code":"Extension"}],"constraint":[{"key":"ele-1","severity":"error","human":"All FHIR elements must have a @value or children","expression":"hasValue() or (children().count() > id.count())","xpath":"@value|f:*|h:div","source":"http://hl7.org/fhir/StructureDefinition/Element"},{"key":"ext-1","severity":"error","human":"Must have either extensions or value[x], not both","expression":"extension.exists() != value.exists()","xpath":"exists(f:extension)!=exists(f:*[starts-with(local-name(.), 'value')])","source":"http://hl7.org/fhir/StructureDefinition/Extension"}],"isModifier":false,"isSummary":false},{"id":"uri.value","path":"uri.value","representation":["xmlAttr"],"short":"Primitive value for uri","definition":"The actual value","min":0,"max":"1","base":{"path":"uri.value","min":0,"max":"1"},"type":[{"extension":[{"url":"http://hl7.org/fhir/StructureDefinition/structuredefinition-fhir-type","valueUrl":"uri"},{"url":"http://hl7.org/fhir/StructureDefinition/regex","valueString":"\\S*"}],"code":"http://hl7.org/fhirpath/System.String"}],"isModifier":false,"isSummary":false}]},"differential":{"element":[{"id":"uri","path":"uri","short":"Primitive Type uri","definition":"String of characters used to identify a name or a resource","comment":"see http://en.wikipedia.org/wiki/Uniform_resource_identifier","min":0,"max":"*"},{"id":"uri.value","path":"uri.value","representation":["xmlAttr"],"short":"Primitive value for uri","definition":"Primitive value for uri","min":0,"max":"1","type":[{"extension":[{"url":"http://hl7.org/fhir/StructureDefinition/structuredefinition-fhir-type","valueUrl":"uri"},{"url":"http://hl7.org/fhir/StructureDefinition/regex","valueString":"\\S*"}],"code":"http://hl7.org/fhirpath/System.String"}]}]}}
\ No newline at end of file