Repository: FHIR/sushi
Pull request #505 (2020-06-26 15:38:25)

Deprecate "units" keyword
The "units" keyword was an optional syntax that could be used on FixedValue and ValueSet rules. The keyword had some meaning, if it was used on a non-Quantity an error would be logged, but beyond that the keyword had no meaning and based on feedback given in https://jira.hl7.org/browse/FHIR-27226, we decide to remove the keyword. This PR does not fully remove `units`, but deprecates it so that if `units` is used, a warning is logged. The additional error checking that `units` used to provide is removed. 


Patch:
diff --git a/src/errors/InvalidUnitsError.ts b/src/errors/InvalidUnitsError.ts
deleted file mode 100644
index c6dc8f4..0000000
--- a/src/errors/InvalidUnitsError.ts
+++ /dev/null
@@ -1,5 +0,0 @@
-export class InvalidUnitsError extends Error {
-  constructor(public id: string) {
-    super(`Invalid use of "units" keyword on non-Quantity element: ${id}`);
-  }
-}
diff --git a/src/errors/index.ts b/src/errors/index.ts
index ea592cd..dfeef7b 100644
--- a/src/errors/index.ts
+++ b/src/errors/index.ts
@@ -34,7 +34,6 @@ export * from './CurrentPackageLoadError';
 export * from './InvalidUriError';
 export * from './MissingSnapshotError';
 export * from './SliceTypeRemovalError';
-export * from './InvalidUnitsError';
 export * from './MultipleStandardsStatusError';
 export * from './InvalidMappingError';
 export * from './InvalidFHIRIdError';
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index 93284ba..f38a8e4 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -101,7 +101,6 @@ export class InstanceExporter implements Fishable {
           rule.path,
           rule.fixedValue,
           this.fisher,
-          rule.units,
           inlineResourceTypes
         );
         // Record each valid rule in a map
diff --git a/src/export/StructureDefinitionExporter.ts b/src/export/StructureDefinitionExporter.ts
index dfe8d64..dacd5c8 100644
--- a/src/export/StructureDefinitionExporter.ts
+++ b/src/export/StructureDefinitionExporter.ts
@@ -136,12 +136,7 @@ export class StructureDefinitionExporter implements Fishable {
             element.constrainCardinality(rule.min, rule.max);
           } else if (rule instanceof FixedValueRule) {
             const replacedRule = replaceReferences(rule, this.tank, this);
-            element.fixValue(
-              replacedRule.fixedValue,
-              replacedRule.exactly,
-              replacedRule.units,
-              this
-            );
+            element.fixValue(replacedRule.fixedValue, replacedRule.exactly, this);
           } else if (rule instanceof FlagRule) {
             element.applyFlags(
               rule.mustSupport,
@@ -156,7 +151,7 @@ export class StructureDefinitionExporter implements Fishable {
             element.constrainType(rule, this, target);
           } else if (rule instanceof ValueSetRule) {
             const vsURI = this.fishForMetadata(rule.valueSet, Type.ValueSet)?.url ?? rule.valueSet;
-            element.bindToVS(vsURI, rule.strength as ElementDefinitionBindingStrength, rule.units);
+            element.bindToVS(vsURI, rule.strength as ElementDefinitionBindingStrength);
           } else if (rule instanceof ContainsRule) {
             const isExtension = element.type?.length === 1 && element.type[0].code === 'Extension';
             if (isExtension) {
diff --git a/src/fhirtypes/ElementDefinition.ts b/src/fhirtypes/ElementDefinition.ts
index d6f18cf..da93ff1 100644
--- a/src/fhirtypes/ElementDefinition.ts
+++ b/src/fhirtypes/ElementDefinition.ts
@@ -23,7 +23,6 @@ import {
   NarrowingRootCardinalityError,
   SliceTypeRemovalError,
   InvalidUriError,
-  InvalidUnitsError,
   FixedToPatternError,
   MultipleStandardsStatusError,
   InvalidMappingError,
@@ -1045,13 +1044,11 @@ export class ElementDefinition {
    * @see {@link http://hl7.org/fhir/R4/terminologies.html#strength}
    * @param {string} vsURI - the value set URI to bind
    * @param {string} strength - the strength of the binding (e.g., 'required')
-   * @param {boolean} units - True if the units keyword is used on this rule
    * @throws {BindingStrengthError} when the binding can't be applied because it is looser than the existing binding
    * @throws {CodedTypeNotFoundError} - when the binding can't be applied because the element is the wrong type
    * @throws {InvalidUriError} when the value set uri is not valid
-   * @throws {InvalidUnitsError} when the "units" keyword is used on a non-Quantity type
    */
-  bindToVS(vsURI: string, strength: ElementDefinitionBindingStrength, units = false): void {
+  bindToVS(vsURI: string, strength: ElementDefinitionBindingStrength): void {
     // Check if this is a valid type to be bound against
     const validTypes = this.findTypesByCode(
       'code',
@@ -1101,24 +1098,17 @@ export class ElementDefinition {
       strength,
       valueSet: vsURI
     };
-
-    // Units error should not stop binding, but must still be logged
-    if (units && !validTypes.find(t => t.code === 'Quantity')) {
-      throw new InvalidUnitsError(this.id);
-    }
   }
 
   /**
    * Fixes a value to an ElementDefinition
    * @param {FixedValueType} value - The value to fix
    * @param {exactly} boolean - True if if fixed[x] should be used, otherwise pattern[x] is used
-   * @param {boolean} units - True if the units keyword is used on this rule
    * @throws {NoSingleTypeError} when the ElementDefinition does not have a single type
    * @throws {ValueAlreadyFixedError} when the value is already fixed to a different value
    * @throws {MismatchedTypeError} when the value does not match the type of the ElementDefinition
-   * @throws {InvalidUnitsError} when the "units" keyword is used on a non-Quantity type
    */
-  fixValue(value: FixedValueType, exactly = false, units = false, fisher?: Fishable): void {
+  fixValue(value: FixedValueType, exactly = false, fisher?: Fishable): void {
     let type: string;
     if (value instanceof FshCode) {
       type = 'Code';
@@ -1210,12 +1200,6 @@ export class ElementDefinition {
         this.constrainCardinality(1, '');
       }
     });
-
-    // Units error should not stop fixing value, but must still be logged
-    const types = this.findTypesByCode('Quantity');
-    if (units && types.length === 0) {
-      throw new InvalidUnitsError(this.id);
-    }
   }
 
   /**
diff --git a/src/fhirtypes/StructureDefinition.ts b/src/fhirtypes/StructureDefinition.ts
index fd29466..7709c3b 100644
--- a/src/fhirtypes/StructureDefinition.ts
+++ b/src/fhirtypes/StructureDefinition.ts
@@ -433,7 +433,6 @@ export class StructureDefinition {
    * @param {string} path - The path to the ElementDefinition to fix
    * @param {any} value - The value to fix; use null to validate just the path when you know the value is valid
    * @param {Fishable} fisher - A fishable implementation for finding definitions and metadata
-   * @param {boolean} units - If the value uses the units keyword
    * @param {inlineResourceTypes} - Types that will be used to replace Resource elements
    * @throws {CannotResolvePathError} when the path cannot be resolved to an element
    * @throws {InvalidResourceTypeError} when setting resourceType to an invalid value
@@ -443,7 +442,6 @@ export class StructureDefinition {
     path: string,
     value: any,
     fisher: Fishable,
-    units = false,
     inlineResourceTypes: string[] = []
   ): { fixedValue: any; pathParts: PathPart[] } {
     const pathParts = this.parseFSHPath(path);
@@ -556,7 +554,6 @@ export class StructureDefinition {
               inlinePath,
               value,
               fisher,
-              units,
               inlineResourceTypes.slice(i + 1)
             );
             return {
@@ -585,7 +582,7 @@ export class StructureDefinition {
       // fixValue will throw if it fails, but skip the check if value is null
       if (value != null) {
         // exactly must be true so that we always test fixing with the more strict fixed[x] approach
-        clone.fixValue(value, true, units, fisher);
+        clone.fixValue(value, true, fisher);
       }
       // If there is a fixedValue or patternValue, find it and return it
       const key = Object.keys(clone).find(k => k.startsWith('pattern') || k.startsWith('fixed'));
diff --git a/src/fshtypes/rules/FixedValueRule.ts b/src/fshtypes/rules/FixedValueRule.ts
index fe1619c..c323288 100644
--- a/src/fshtypes/rules/FixedValueRule.ts
+++ b/src/fshtypes/rules/FixedValueRule.ts
@@ -15,7 +15,6 @@ export type FixedValueType =
 export class FixedValueRule extends Rule {
   fixedValue: FixedValueType;
   exactly: boolean;
-  units: boolean;
   isInstance: boolean;
 
   constructor(path: string) {
diff --git a/src/fshtypes/rules/ValueSetRule.ts b/src/fshtypes/rules/ValueSetRule.ts
index 30e8b5a..48db819 100644
--- a/src/fshtypes/rules/ValueSetRule.ts
+++ b/src/fshtypes/rules/ValueSetRule.ts
@@ -3,7 +3,6 @@ import { Rule } from './Rule';
 export class ValueSetRule extends Rule {
   valueSet: string;
   strength: string;
-  units: boolean;
 
   constructor(path: string) {
     super(path);
diff --git a/src/import/FSHImporter.ts b/src/import/FSHImporter.ts
index ff88aa6..05156ad 100644
--- a/src/import/FSHImporter.ts
+++ b/src/import/FSHImporter.ts
@@ -1051,7 +1051,12 @@ export class FSHImporter extends FSHVisitor {
       .withFile(this.currentFile);
     vsRule.valueSet = this.aliasAwareValue(ctx.SEQUENCE());
     vsRule.strength = ctx.strength() ? this.visitStrength(ctx.strength()) : 'required';
-    vsRule.units = ctx.KW_UNITS() != null;
+    if (ctx.KW_UNITS()) {
+      logger.warn(
+        'The "units" keyword is deprecated and has no effect. Support will be removed entirely in a future release.',
+        vsRule.sourceInfo
+      );
+    }
     return vsRule;
   }
 
@@ -1072,7 +1077,12 @@ export class FSHImporter extends FSHVisitor {
       .withFile(this.currentFile);
     fixedValueRule.fixedValue = this.visitValue(ctx.value());
     fixedValueRule.exactly = ctx.KW_EXACTLY() != null;
-    fixedValueRule.units = ctx.KW_UNITS() != null;
+    if (ctx.KW_UNITS()) {
+      logger.warn(
+        'The "units" keyword is deprecated and has no effect. Support will be removed entirely in a future release.',
+        fixedValueRule.sourceInfo
+      );
+    }
     fixedValueRule.isInstance =
       ctx.value().SEQUENCE() != null && !this.allAliases.has(ctx.value().SEQUENCE().getText());
     return fixedValueRule;


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index 957f1cb..590e1c3 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -1467,21 +1467,6 @@ describe('InstanceExporter', () => {
       });
     });
 
-    // Units keyword
-    it('should log an error when fixing to a non-Quantity with the units keyword', () => {
-      const barRule = new FixedValueRule('gender')
-        .withFile('PatientInstance.fsh')
-        .withLocation([1, 2, 3, 4]);
-      barRule.fixedValue = new FshCode('mycode', 'http:/mysystem.com');
-      barRule.units = true;
-      patientInstance.rules.push(barRule);
-      const exported = exportInstance(patientInstance);
-      expect(exported.gender).toBeUndefined();
-      expect(loggerSpy.getLastMessage('error')).toMatch(
-        /units.*Patient.gender.*File: PatientInstance.fsh.*Line: 1 - 3\D*/s
-      );
-    });
-
     // Validating required elements
     it('should log an error when a required element is not present', () => {
       const cardRule = new CardRule('active');
diff --git a/test/export/StructureDefinitionExporter.test.ts b/test/export/StructureDefinitionExporter.test.ts
index 533a9cf..e5869d2 100644
--- a/test/export/StructureDefinitionExporter.test.ts
+++ b/test/export/StructureDefinitionExporter.test.ts
@@ -920,27 +920,6 @@ describe('StructureDefinitionExporter', () => {
     expect(loggerSpy.getLastMessage()).toMatch(/File: Strict\.fsh.*Line: 9\D*/s);
   });
 
-  it('should not apply a ValueSetRule on a non-Quantity with units keyword', () => {
-    const profile = new Profile('Foo');
-    profile.parent = 'Observation';
-
-    const rule = new ValueSetRule('code').withFile('Fixed.fsh').withLocation([4, 18, 4, 28]);
-    rule.valueSet = 'http://system.com';
-    rule.strength = 'required';
-    rule.units = true;
-    profile.rules.push(rule);
-
-    exporter.exportStructDef(profile);
-    const sd = pkg.profiles[0];
-    const fixedCode = sd.findElement('Observation.code');
-
-    expect(fixedCode.binding.valueSet).toBe('http://system.com'); // Still bound
-    expect(fixedCode.binding.strength).toBe('required');
-    expect(loggerSpy.getLastMessage()).toMatch(
-      /units.*Observation.code.*File: Fixed\.fsh.*Line: 4\D*/s
-    );
-  });
-
   // Only Rule
   it('should apply a correct OnlyRule on a non-reference choice', () => {
     const profile = new Profile('Foo');
@@ -1685,26 +1664,6 @@ describe('StructureDefinitionExporter', () => {
     expect(loggerSpy.getLastMessage()).toMatch(/File: Fixed\.fsh.*Line: 4\D*/s);
   });
 
-  it('should not apply a FixedValueRule on a non-Quantity with units keyword', () => {
-    const profile = new Profile('Foo');
-    profile.parent = 'Observation';
-
-    const rule = new FixedValueRule('code').withFile('Fixed.fsh').withLocation([4, 18, 4, 28]);
-    rule.fixedValue = new FshCode('mycode', 'http://mysystem.com');
-    rule.units = true;
-    profile.rules.push(rule);
-
-    exporter.exportStructDef(profile);
-    const sd = pkg.profiles[0];
-    const fixedCode = sd.findElement('Observation.code');
-    expect(fixedCode.patternCodeableConcept).toEqual({
-      coding: [{ system: 'http://mysystem.com', code: 'mycode' }]
-    }); // Code still set
-    expect(loggerSpy.getLastMessage()).toMatch(
-      /units.*Observation.code.*File: Fixed\.fsh.*Line: 4\D*/s
-    );
-  });
-
   // Contains Rule
   it('should apply a ContainsRule on an element with defined slicing', () => {
     const profile = new Profile('Foo');
diff --git a/test/fhirtypes/ElementDefinition.bindToVS.test.ts b/test/fhirtypes/ElementDefinition.bindToVS.test.ts
index f6c6830..99f865b 100644
--- a/test/fhirtypes/ElementDefinition.bindToVS.test.ts
+++ b/test/fhirtypes/ElementDefinition.bindToVS.test.ts
@@ -74,12 +74,6 @@ describe('ElementDefinition', () => {
       expect(concept.binding.strength).toBe('required');
     });
 
-    it('should bind a value set with units on a Quantity', () => {
-      const quantity = observation.findElementByPath('valueQuantity', fisher);
-      quantity.bindToVS('http://valueset.org', 'required', true);
-      expect(quantity.binding.valueSet).toBe('http://valueset.org');
-    });
-
     it('should throw CodedTypeNotFoundError when binding to an unsupported type', () => {
       const instant = observation.elements.find(e => e.id === 'Observation.issued');
       const clone = cloneDeep(instant);
@@ -97,17 +91,6 @@ describe('ElementDefinition', () => {
       }).toThrow(/notAUri/);
     });
 
-    it('should throw InvalidUnitsError when using the units keyword on a non-Quantity', () => {
-      const category = observation.elements.find(e => e.id === 'Observation.category');
-      const clone = cloneDeep(category);
-      expect(() => {
-        clone.bindToVS('http://valueset.org', 'required', true);
-      }).toThrow(/units.*Observation.category/);
-      // Units error should not stop value from still being bound
-      expect(clone.binding.valueSet).toBe('http://valueset.org');
-      expect(clone.binding.strength).toBe('required');
-    });
-
     it('should only allow required to be rebound with required', () => {
       const status = observation.elements.find(e => e.id === 'Observation.status');
       expect(status.binding.strength).toBe('required');
diff --git a/test/fhirtypes/ElementDefinition.fixValue.test.ts b/test/fhirtypes/ElementDefinition.fixValue.test.ts
index ea0a819..6e19019 100644
--- a/test/fhirtypes/ElementDefinition.fixValue.test.ts
+++ b/test/fhirtypes/ElementDefinition.fixValue.test.ts
@@ -72,17 +72,6 @@ describe('ElementDefinition', () => {
       );
     });
 
-    it('should throw InvalidUnitsError when using the units keyword on a non-Quantity', () => {
-      const code = medication.elements.find(e => e.id === 'Medication.code');
-      expect(() => {
-        code.fixValue(new FshCode('mycode', 'https://code.com'), false, true);
-      }).toThrow(/units.*Medication.code/);
-      // Units error should not stop value from still being fixed
-      expect(code.patternCodeableConcept).toEqual({
-        coding: [{ code: 'mycode', system: 'https://code.com' }]
-      });
-    });
-
     it('should ensure that minimum cardinality is 1 when fixing a value mentioned in a parent slice discriminator', () => {
       const cat = medicationRequest.elements.find(e => e.id === 'MedicationRequest.category');
       cat.slicing = { discriminator: [{ type: 'value', path: 'coding.code' }], rules: 'open' };
diff --git a/test/fhirtypes/StructureDefinition.test.ts b/test/fhirtypes/StructureDefinition.test.ts
index d2ff060..583bede 100644
--- a/test/fhirtypes/StructureDefinition.test.ts
+++ b/test/fhirtypes/StructureDefinition.test.ts
@@ -1329,7 +1329,6 @@ describe('StructureDefinition', () => {
           'contained[0].gender',
           gender,
           fisher,
-          false,
           ['Patient']
         );
         expect(fixedValue).toBe('F');
@@ -1344,13 +1343,12 @@ describe('StructureDefinition', () => {
         const {
           fixedValue,
           pathParts
-        } = respRate.validateValueAtPath(
-          'contained[0].entry[0].resource.gender',
-          gender,
-          fisher,
-          false,
-          ['Bundle', null, 'Patient', null]
-        );
+        } = respRate.validateValueAtPath('contained[0].entry[0].resource.gender', gender, fisher, [
+          'Bundle',
+          null,
+          'Patient',
+          null
+        ]);
         expect(fixedValue).toBe('F');
         expect(pathParts).toEqual([
           { base: 'contained', brackets: ['0'] },
@@ -1369,7 +1367,6 @@ describe('StructureDefinition', () => {
           'contained[0].entry[0].resource.entry[0].resource.gender',
           gender,
           fisher,
-          false,
           ['Bundle', null, 'Bundle', null, 'Patient', null]
         );
         expect(fixedValue).toBe('F');
@@ -1388,7 +1385,7 @@ describe('StructureDefinition', () => {
         const {
           fixedValue,
           pathParts
-        } = respRate.validateValueAtPath('contained[0].valueQuantity.unit', unit, fisher, false, [
+        } = respRate.validateValueAtPath('contained[0].valueQuantity.unit', unit, fisher, [
           'http://hl7.org/fhir/StructureDefinition/resprate',
           null,
           null
@@ -1404,20 +1401,19 @@ describe('StructureDefinition', () => {
       it('should not allow overriding a Resource constrained to Patient with a non-Patient path', () => {
         const system = 'http://hello.com';
         expect(() =>
-          respRate.validateValueAtPath('contained[0].system', system, fisher, false, ['Patient'])
+          respRate.validateValueAtPath('contained[0].system', system, fisher, ['Patient'])
         ).toThrow('The element or path you referenced does not exist: contained[0].system');
       });
 
       it('should not allow overriding a Resource constrained to Patient with a non-Patient path inside a Resource', () => {
         const system = 'http://hello.com';
         expect(() =>
-          respRate.validateValueAtPath(
-            'contained[0].entry[0].resource.system',
-            system,
-            fisher,
-            false,
-            ['Bundle', null, 'Patient', null]
-          )
+          respRate.validateValueAtPath('contained[0].entry[0].resource.system', system, fisher, [
+            'Bundle',
+            null,
+            'Patient',
+            null
+          ])
         ).toThrow(
           'The element or path you referenced does not exist: contained[0].entry[0].resource.system'
         );
@@ -1426,7 +1422,7 @@ describe('StructureDefinition', () => {
       it('should not allow overriding a Patient with an Observation', () => {
         const method = new FshCode('man', 'http://method.com');
         expect(() =>
-          respRate.validateValueAtPath('contained[PatientsOnly].method', method, fisher, false, [
+          respRate.validateValueAtPath('contained[PatientsOnly].method', method, fisher, [
             'Observation'
           ])
         ).toThrow(
@@ -1437,9 +1433,7 @@ describe('StructureDefinition', () => {
       it('should not allow overriding a Resource constrained to a non-FHIR Resource', () => {
         const system = 'http://hello.com';
         expect(() =>
-          respRate.validateValueAtPath('contained[0].system', system, fisher, false, [
-            'CodeableConcept'
-          ])
+          respRate.validateValueAtPath('contained[0].system', system, fisher, ['CodeableConcept'])
         ).toThrow('The element or path you referenced does not exist: contained[0].system');
       });
 
diff --git a/test/fshtypes/rules/FixedValueRule.test.ts b/test/fshtypes/rules/FixedValueRule.test.ts
index 7db49e3..bdaf212 100644
--- a/test/fshtypes/rules/FixedValueRule.test.ts
+++ b/test/fshtypes/rules/FixedValueRule.test.ts
@@ -8,7 +8,6 @@ describe('FixedValueRule', () => {
       expect(c.path).toBe('component.code');
       expect(c.fixedValue).toBeUndefined();
       expect(c.exactly).toBeUndefined();
-      expect(c.units).toBeUndefined();
       expect(c.isInstance).toBeUndefined();
     });
   });
diff --git a/test/fshtypes/rules/ValueSetRule.test.ts b/test/fshtypes/rules/ValueSetRule.test.ts
index a8e3993..7b53bed 100644
--- a/test/fshtypes/rules/ValueSetRule.test.ts
+++ b/test/fshtypes/rules/ValueSetRule.test.ts
@@ -8,7 +8,6 @@ describe('ValueSetRule', () => {
       expect(c.path).toBe('component.code');
       expect(c.valueSet).toBeUndefined();
       expect(c.strength).toBeUndefined();
-      expect(c.units).toBeUndefined();
     });
   });
 });
diff --git a/test/import/FSHImporter.Instance.test.ts b/test/import/FSHImporter.Instance.test.ts
index 8687ba5..16fc18d 100644
--- a/test/import/FSHImporter.Instance.test.ts
+++ b/test/import/FSHImporter.Instance.test.ts
@@ -222,7 +222,7 @@ describe('FSHImporter', () => {
         expect(instance.title).toBe('Georgio Manos');
         expect(instance.description).toBe('An example of a fictional patient named Georgio Manos');
         expect(instance.rules.length).toBe(1);
-        assertFixedValueRule(instance.rules[0], 'contained[0]', 'SomeInstance', false, false, true);
+        assertFixedValueRule(instance.rules[0], 'contained[0]', 'SomeInstance', false, true);
       });
     });
 
diff --git a/test/import/FSHImporter.Profile.test.ts b/test/import/FSHImporter.Profile.test.ts
index 7ac9577..20d8c0a 100644
--- a/test/import/FSHImporter.Profile.test.ts
+++ b/test/import/FSHImporter.Profile.test.ts
@@ -814,23 +814,24 @@ describe('FSHImporter', () => {
         );
       });
 
-      it('should parse value set rules on Quantity with units keyword', () => {
+      it('should ignore the units keyword and log a warning when parsing value set rules on Quantity', () => {
         const input = `
-
         Profile: ObservationProfile
         Parent: Observation
         * valueQuantity units from http://unitsofmeasure.org
         `;
 
-        const result = importSingleText(input);
+        const result = importSingleText(input, 'UselessQuant.fsh');
         const profile = result.profiles.get('ObservationProfile');
         expect(profile.rules).toHaveLength(1);
         assertValueSetRule(
           profile.rules[0],
           'valueQuantity',
           'http://unitsofmeasure.org',
-          'required',
-          true
+          'required'
+        );
+        expect(loggerSpy.getLastMessage('warn')).toMatch(
+          /The "units" keyword is deprecated and has no effect.*File: UselessQuant\.fsh.*Line: 4\D*/s
         );
       });
     });
@@ -992,21 +993,23 @@ describe('FSHImporter', () => {
         assertFixedValueRule(profile.rules[0], 'valueCodeableConcept', expectedCode, true);
       });
 
-      it('should parse fixed value FSHCode rule with units on Quantity', () => {
+      it('should ignore the units keyword and log a warning when parsing a fixed value FSHCode rule with units on Quantity', () => {
         const input = `
-
         Profile: ObservationProfile
         Parent: Observation
         * valueQuantity units = http://unitsofmeasure.org#cGy
         `;
 
-        const result = importSingleText(input);
+        const result = importSingleText(input, 'UselessUnits.fsh');
         const profile = result.profiles.get('ObservationProfile');
         expect(profile.rules).toHaveLength(1);
         const expectedCode = new FshCode('cGy', 'http://unitsofmeasure.org')
-          .withLocation([5, 33, 5, 61])
-          .withFile('');
-        assertFixedValueRule(profile.rules[0], 'valueQuantity', expectedCode, false, true);
+          .withLocation([4, 33, 4, 61])
+          .withFile('UselessUnits.fsh');
+        assertFixedValueRule(profile.rules[0], 'valueQuantity', expectedCode);
+        expect(loggerSpy.getLastMessage('warn')).toMatch(
+          /The "units" keyword is deprecated and has no effect.*File: UselessUnits\.fsh.*Line: 4\D*/s
+        );
       });
 
       it('should parse fixed value Quantity rule', () => {
diff --git a/test/testhelpers/asserts.ts b/test/testhelpers/asserts.ts
index 8bcbe6f..97d0dc2 100644
--- a/test/testhelpers/asserts.ts
+++ b/test/testhelpers/asserts.ts
@@ -51,15 +51,13 @@ export function assertValueSetRule(
   rule: Rule,
   path: string,
   valueSet: string,
-  strength: string,
-  units = false
+  strength: string
 ): void {
   expect(rule).toBeInstanceOf(ValueSetRule);
   const valueSetRule = rule as ValueSetRule;
   expect(valueSetRule.path).toBe(path);
   expect(valueSetRule.valueSet).toBe(valueSet);
   expect(valueSetRule.strength).toBe(strength);
-  expect(valueSetRule.units).toBe(units);
 }
 
 export function assertFixedValueRule(
@@ -67,7 +65,6 @@ export function assertFixedValueRule(
   path: string,
   value: FixedValueType,
   exactly = false,
-  units = false,
   isInstance = false
 ): void {
   expect(rule).toBeInstanceOf(FixedValueRule);
@@ -75,7 +72,6 @@ export function assertFixedValueRule(
   expect(fixedValueRule.path).toBe(path);
   expect(fixedValueRule.fixedValue).toEqual(value);
   expect(fixedValueRule.exactly).toBe(exactly);
-  expect(fixedValueRule.units).toBe(units);
   expect(fixedValueRule.isInstance).toEqual(isInstance);
 }