diff --git a/packages/xforms-engine/src/expression/DependentExpression.ts b/packages/xforms-engine/src/expression/DependentExpression.ts index e343a256..bdf710d2 100644 --- a/packages/xforms-engine/src/expression/DependentExpression.ts +++ b/packages/xforms-engine/src/expression/DependentExpression.ts @@ -38,11 +38,6 @@ interface DependentExpressionOptions { */ readonly ignoreContextReference?: boolean; - /** - * @default true - */ - readonly ignoreNullExpressions?: boolean; - readonly semanticDependencies?: SemanticDependencyOptions; } @@ -61,7 +56,6 @@ export class DependentExpression { const { ignoreContextReference = false, - ignoreNullExpressions = true, semanticDependencies = { parentContext: false, translations: false, @@ -71,7 +65,6 @@ export class DependentExpression { const dependencyReferences = new Set( resolveDependencyNodesets(context.reference, expression, { ignoreReferenceToContextPath: ignoreContextReference, - simulateNullKeyword: ignoreNullExpressions, }) ); diff --git a/packages/xforms-engine/src/parse/xpath/dependency-analysis.ts b/packages/xforms-engine/src/parse/xpath/dependency-analysis.ts index aedab94f..02372e73 100644 --- a/packages/xforms-engine/src/parse/xpath/dependency-analysis.ts +++ b/packages/xforms-engine/src/parse/xpath/dependency-analysis.ts @@ -29,29 +29,10 @@ interface PathResolutionOptions { * cycle analysis. */ readonly ignoreReferenceToContextPath?: boolean; - - /** - * Treats the literal sub-expression `null` as if it were a keyword. This is a - * deviation from XPath (where `null` would be a RelativeLocationPath - * referencing a child node by that name). However, in real-world usage, some - * forms evidently use `null` as a keyword representing a null and/or blank - * value. - * - * @default true - * - * @todo Some equivalent of this flag has been present, with the same default, - * since very early Web Forms exploratory work. At time of writing, it has - * never been overridden in any equivalent context. This has not been a source - * of any known bug/unexpected/unusual behavior. Howevever, it would probably - * be more robust to override the default whenever a form definition includes - * a node literally named ``. - */ - readonly simulateNullKeyword?: boolean; } const defaultPathResolutionOptions: Required = { ignoreReferenceToContextPath: false, - simulateNullKeyword: true, }; /** @@ -75,19 +56,14 @@ export const resolveDependencyNodesets = ( expression: string, options: PathResolutionOptions = {} ): readonly string[] => { - const { ignoreReferenceToContextPath, simulateNullKeyword } = { + const { ignoreReferenceToContextPath } = { ...defaultPathResolutionOptions, ...options, }; const contextNode = getPathExpressionNode(contextNodeset ?? '"not a path, null fallback"'); const expressionRootNode = expressionParser.parse(expression).rootNode; - - let subExpressions = findLocationPathSubExpressionNodes(expressionRootNode); - - if (simulateNullKeyword) { - subExpressions = subExpressions.filter((subExpression) => subExpression.text !== 'null'); - } + const subExpressions = findLocationPathSubExpressionNodes(expressionRootNode); const resolvedPathLists = subExpressions.flatMap((subExpression) => { const pathNodes = resolvePath(contextNode, subExpression); diff --git a/packages/xforms-engine/test/parse/xpath/dependency-analysis.test.ts b/packages/xforms-engine/test/parse/xpath/dependency-analysis.test.ts index fe23a567..b568f640 100644 --- a/packages/xforms-engine/test/parse/xpath/dependency-analysis.test.ts +++ b/packages/xforms-engine/test/parse/xpath/dependency-analysis.test.ts @@ -312,6 +312,16 @@ describe('Dependency analysis', () => { expression: '//. | //./foo | //.. | //../foo', expected: ['//.', '//foo', '//..'], }, + + // This case exists to test removal of such a special case. + // + // Discussion here: https://github.com/getodk/web-forms/pull/166#discussion_r1680168253 + { + description: 'null is not a keyword, nor special cased', + contextNodeset: '/data', + expression: 'count(null) > count(foo/null)', + expected: ['/data/null', '/data/foo/null'], + }, ])('$description', ({ contextNodeset, expression, expected }) => { it(`resolves nodeset dependencies in expression ${JSON.stringify(expression)}, with context ${JSON.stringify(contextNodeset)}, producing nodesets: ${JSON.stringify(expected)}`, () => { const actual = resolveDependencyNodesets(contextNodeset, expression);