From 56c9f8a1c9b7f0f66c4171c20cde35ded891fa7c Mon Sep 17 00:00:00 2001 From: Andrey Listopadov Date: Wed, 11 Feb 2026 14:41:40 +0300 Subject: [PATCH] disallow double quotes in fhirpath --- package.json | 2 +- src/analyzer.ts | 12 ++++++++++- src/errors.ts | 3 ++- src/lexer.ts | 13 ++++++++++-- src/parser.ts | 10 ++++++--- test/analyzer.test.ts | 8 +++---- test/analyzer/empty-propagation.test.ts | 4 ++-- test/analyzer/type-checking.test.ts | 28 ++++++++++++++++++++++++- test/lexer.test.ts | 10 ++++----- 9 files changed, 70 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index d8e15c3..7395dac 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@atomic-ehr/fhirpath", - "version": "0.1.1", + "version": "0.1.2", "description": "A TypeScript implementation of FHIRPath", "type": "module", "main": "./dist/index.node.js", diff --git a/src/analyzer.ts b/src/analyzer.ts index 2a5c962..58310f0 100644 --- a/src/analyzer.ts +++ b/src/analyzer.ts @@ -785,10 +785,20 @@ export class Analyzer { */ private analyzeLiteral(node: LiteralNode, context: AnalysisContext): InternalAnalysisResult { let type: TypeInfo; + const diagnostics: Diagnostic[] = []; switch (node.valueType) { case 'string': type = { type: 'String', singleton: true }; + if (node.raw?.startsWith('"')) { + diagnostics.push({ + range: node.range, + message: 'Double-quoted strings are not valid in FHIRPath, use single quotes instead', + severity: DiagnosticSeverity.Warning, + code: ErrorCodes.DOUBLE_QUOTED_STRING, + source: 'fhirpath', + }); + } break; case 'number': // Number without decimal point is integer @@ -814,7 +824,7 @@ export class Analyzer { type = { type: 'Any', singleton: true }; } - return { type, diagnostics: [] }; + return { type, diagnostics }; } private analyzeTemporalLiteral(node: TemporalLiteralNode, context: AnalysisContext): InternalAnalysisResult { diff --git a/src/errors.ts b/src/errors.ts index 385d7f9..bebeda2 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -269,5 +269,6 @@ export enum ErrorCodes { UNSUPPORTED_TEMPORAL_UNIT_FOR_TYPE = 'FP6011', // Static analysis warnings (7000-7999) - UNREACHABLE_CODE = 'FP7001' + UNREACHABLE_CODE = 'FP7001', + DOUBLE_QUOTED_STRING = 'FP7002' } diff --git a/src/lexer.ts b/src/lexer.ts index 294b459..56ca98d 100644 --- a/src/lexer.ts +++ b/src/lexer.ts @@ -24,6 +24,7 @@ export enum TokenType { TIME = 5, QUANTITY = 6, // Quantity literals like 5 'mg' DATE = 7, // Date literals like @2020-01-01 + DOUBLE_QUOTED_STRING = 8, // "..." - not in spec but lexed for diagnostics // Operators (all symbol operators consolidated) OPERATOR = 10, // +, -, *, /, <, >, <=, >=, =, !=, ~, !~, |, & @@ -246,8 +247,7 @@ export class Lexer { return this.readString("'"); case '"': - // Not in spec but often supported - return this.readString('"'); + return this.readDoubleQuotedString(); case '`': return this.readDelimitedIdentifier(); @@ -364,6 +364,15 @@ export class Lexer { const value = this.input.substring(start, this.position); return this.createToken(TokenType.STRING, value, start, this.position, startLine, startColumn); } + + private readDoubleQuotedString(): Token { + const start = this.position; + const startLine = this.line; + const startColumn = this.column; + this.scanQuoted('"', 'Unterminated string'); + const value = this.input.substring(start, this.position); + return this.createToken(TokenType.DOUBLE_QUOTED_STRING, value, start, this.position, startLine, startColumn); + } private readNumber(): Token { const start = this.position; diff --git a/src/parser.ts b/src/parser.ts index f677de2..c5ed712 100644 --- a/src/parser.ts +++ b/src/parser.ts @@ -454,7 +454,7 @@ export class Parser { // Check if next token is a string (quantity unit) const nextToken = this.peek(); - if (nextToken.type === TokenType.STRING) { + if (nextToken.type === TokenType.STRING || nextToken.type === TokenType.DOUBLE_QUOTED_STRING) { this.advance(); const unit = this.parseStringValue(nextToken.value); return this.createQuantityNode(numberValue, unit, token, nextToken); @@ -477,10 +477,14 @@ export class Parser { return this.createLiteralNode(numberValue, isDecimal ? 'decimal' : 'number', token); } - if (token.type === TokenType.STRING) { + if (token.type === TokenType.STRING || token.type === TokenType.DOUBLE_QUOTED_STRING) { this.current++; // inline advance() const value = this.parseStringValue(token.value); - return this.createLiteralNode(value, 'string', token); + const node = this.createLiteralNode(value, 'string', token); + if (token.type === TokenType.DOUBLE_QUOTED_STRING) { + node.raw = token.value; + } + return node; } if (token.type === TokenType.IDENTIFIER && (token.value === 'true' || token.value === 'false')) { diff --git a/test/analyzer.test.ts b/test/analyzer.test.ts index c4c5666..ef7f9a2 100644 --- a/test/analyzer.test.ts +++ b/test/analyzer.test.ts @@ -57,7 +57,7 @@ describe("Analyzer", () => { describe("functions", () => { it("should not report errors for valid functions", async () => { - const result = await analyze('name.where(use = "official")'); + const result = await analyze("name.where(use = 'official')"); expect(result.diagnostics).toEqual([]); }); @@ -93,7 +93,7 @@ describe("Analyzer", () => { describe("complex expressions", () => { it("should analyze nested expressions", async () => { - const result = await analyze('name.where(use = "official").given'); + const result = await analyze("name.where(use = 'official').given"); expect(result.diagnostics).toEqual([]); }); @@ -447,7 +447,7 @@ describe("Analyzer", () => { } const result = await analyze( - 'Patient.contact.where(relationship.coding.code = "family").name.given.first()', + "Patient.contact.where(relationship.coding.code = 'family').name.given.first()", { modelProvider, }, @@ -533,7 +533,7 @@ describe("Analyzer", () => { } const result = await analyze( - 'Patient.extension.where(url = "http://example.org/ext").value', + "Patient.extension.where(url = 'http://example.org/ext').value", { modelProvider }, ); diff --git a/test/analyzer/empty-propagation.test.ts b/test/analyzer/empty-propagation.test.ts index 4bdfc98..8927db5 100644 --- a/test/analyzer/empty-propagation.test.ts +++ b/test/analyzer/empty-propagation.test.ts @@ -151,10 +151,10 @@ describe('Empty Collection Propagation', () => { }); it('should produce warnings for empty arguments where specific type expected', async () => { - const ast = parse('"test".substring({})'); + const ast = parse("'test'.substring({})"); const analyzer = new Analyzer(); const result = await analyzer.analyze(ast.ast); - + // Should have a warning about type mismatch const warnings = result.diagnostics.filter(d => d.severity === 2); // 2 = warning expect(warnings.length).toBeGreaterThan(0); diff --git a/test/analyzer/type-checking.test.ts b/test/analyzer/type-checking.test.ts index 0f89778..4545ff2 100644 --- a/test/analyzer/type-checking.test.ts +++ b/test/analyzer/type-checking.test.ts @@ -6,11 +6,37 @@ import { ErrorCodes } from "../../src/index.node"; describe('Type Checking', () => { describe('Basic type inference', () => { it('should infer literal types', async () => { - const result = await analyze('"hello"'); + const result = await analyze("'hello'"); expect(result.ast.typeInfo).toEqual({ type: 'String', singleton: true }); expect(result.diagnostics).toHaveLength(0); }); + it('should warn on double-quoted strings', async () => { + const result = await analyze('"hello"'); + expect(result.ast.typeInfo).toEqual({ type: 'String', singleton: true }); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0]).toMatchObject({ + severity: DiagnosticSeverity.Warning, + code: ErrorCodes.DOUBLE_QUOTED_STRING, + }); + }); + + it('should warn on double-quoted strings in function arguments', async () => { + const result = await analyze('name.where(use = "official")'); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0]).toMatchObject({ + severity: DiagnosticSeverity.Warning, + code: ErrorCodes.DOUBLE_QUOTED_STRING, + message: 'Double-quoted strings are not valid in FHIRPath, use single quotes instead', + }); + }); + + it('should warn on each double-quoted string separately', async () => { + const result = await analyze('"a" + "b"'); + expect(result.diagnostics).toHaveLength(2); + expect(result.diagnostics.every(d => d.code === ErrorCodes.DOUBLE_QUOTED_STRING)).toBe(true); + }); + it('should infer numeric types', async () => { const intResult = await analyze('42'); expect(intResult.ast.typeInfo).toEqual({ type: 'Integer', singleton: true }); diff --git a/test/lexer.test.ts b/test/lexer.test.ts index 95e7af8..8b74d43 100644 --- a/test/lexer.test.ts +++ b/test/lexer.test.ts @@ -200,7 +200,7 @@ describe("New Simplified Lexer", () => { it("should tokenize strings with double quotes", async () => { const tokens = tokenize('"hello" "world"'); - expect(getToken(tokens, 0).type).toBe(TokenType.STRING); + expect(getToken(tokens, 0).type).toBe(TokenType.DOUBLE_QUOTED_STRING); expect(getToken(tokens, 0).value).toBe('"hello"'); }); @@ -450,7 +450,7 @@ describe("New Simplified Lexer", () => { }); it("should tokenize expressions with mixed operators", async () => { - const result = tokenTypesAndValues('age >= 18 and status = "active"'); + const result = tokenTypesAndValues("age >= 18 and status = 'active'"); expect(result).toEqual([ "ID:age", "OP:>=", @@ -464,7 +464,7 @@ describe("New Simplified Lexer", () => { }); it("should tokenize function calls with arguments", async () => { - const result = tokenTypesAndValues('where(use = "official").given'); + const result = tokenTypesAndValues("where(use = 'official').given"); expect(result).toEqual([ "ID:where", "LPAREN", @@ -615,7 +615,7 @@ describe("New Simplified Lexer", () => { describe("Real-world Examples", () => { it("should tokenize FHIR resource paths", async () => { const result = tokenTypesAndValues( - 'Patient.identifier.where(system = "http://example.org").value', + "Patient.identifier.where(system = 'http://example.org').value", ); expect(result).toEqual([ "ID:Patient", @@ -636,7 +636,7 @@ describe("New Simplified Lexer", () => { it("should tokenize complex filter expressions", async () => { const result = tokenTypesAndValues( - 'Observation.where(code.coding.exists(system = "LOINC" and code = "1234-5"))', + "Observation.where(code.coding.exists(system = 'LOINC' and code = '1234-5'))", ); expect(result).toEqual([ "ID:Observation",