Skip to content

Commit 68fb2e9

Browse files
authored
Merge pull request #267 from effigies/feat/message-interpolation
feat: Add issue message formatting
2 parents 09daea1 + 6b5ee40 commit 68fb2e9

File tree

6 files changed

+216
-75
lines changed

6 files changed

+216
-75
lines changed

.github/workflows/deno_tests.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ jobs:
8383
touch tests/data/bids-examples/${DS}/.SKIP_VALIDATION
8484
done
8585
if: github.ref_name != 'dev' && github.base_ref != 'dev'
86-
- run: deno test --node-modules-dir=auto $PERMS --coverage=cov/ src/
86+
- run: deno test --parallel --node-modules-dir=auto $PERMS --coverage=cov/ src/
8787
- name: Collect coverage
8888
run: deno coverage cov/ --lcov --output=coverage.lcov
8989
if: ${{ always() }}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<!--
2+
A new scriv changelog fragment.
3+
4+
Uncomment the section that is right (remove the HTML comment wrapper).
5+
For top level release notes, leave all the headers commented out.
6+
-->
7+
8+
### Added
9+
10+
- Support issues messages that access validation context variables.
11+
12+
<!--
13+
### Changed
14+
15+
- A bullet item for the Changed category.
16+
17+
-->
18+
<!--
19+
### Fixed
20+
21+
- A bullet item for the Fixed category.
22+
23+
-->
24+
<!--
25+
### Deprecated
26+
27+
- A bullet item for the Deprecated category.
28+
29+
-->
30+
<!--
31+
### Removed
32+
33+
- A bullet item for the Removed category.
34+
35+
-->
36+
<!--
37+
### Security
38+
39+
- A bullet item for the Security category.
40+
41+
-->
42+
<!--
43+
### Infrastructure
44+
45+
- A bullet item for the Infrastructure category.
46+
47+
-->

src/schema/applyRules.test.ts

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// @ts-nocheck
22
import { assert, assertEquals } from '@std/assert'
3-
import { applyRules, evalCheck, evalConstructor } from './applyRules.ts'
3+
import { applyRules, evalCheck } from './applyRules.ts'
44
import { DatasetIssues } from '../issues/datasetIssues.ts'
55
import { expressionFunctions } from './expressionLanguage.ts'
66

@@ -107,48 +107,3 @@ Deno.test(
107107
assertEquals(context.dataset.issues.get({ code: 'CHECK_ERROR' }).length, 1)
108108
},
109109
)
110-
111-
Deno.test('evalConstructor test', async (t) => {
112-
const match = expressionFunctions.match
113-
await t.step('check veridical reconstruction of match expressions', () => {
114-
// match() functions frequently contain escapes in regex patterns
115-
// We receive these from the schema as written in YAML, so we need to ensure
116-
// that they are correctly reconstructed in the evalConstructor() function
117-
//
118-
// The goal is to avoid schema authors needing to double-escape their regex patterns
119-
// and other implementations to account for the double-escaping
120-
let pattern = String.raw`^\.nii(\.gz)?$`
121-
122-
// Check both a literal and a variable pattern produce the same result
123-
for (const check of ['match(extension, pattern)', `match(extension, '${pattern}')`]) {
124-
const niftiCheck = evalConstructor(check)
125-
for (
126-
const [extension, expected] of [
127-
['.nii', true],
128-
['.nii.gz', true],
129-
['.tsv', false],
130-
[',nii,gz', false], // Check that . is not treated as a wildcard
131-
]
132-
) {
133-
assert(match(extension, pattern) === expected)
134-
// Pass in a context object to provide any needed variables
135-
assert(niftiCheck({ match, extension, pattern }) === expected)
136-
}
137-
}
138-
139-
pattern = String.raw`\S`
140-
for (const check of ['match(json.Name, pattern)', `match(json.Name, '${pattern}')`]) {
141-
const nonEmptyCheck = evalConstructor(check)
142-
for (
143-
const [Name, expected] of [
144-
['test', true],
145-
['', false],
146-
[' ', false],
147-
]
148-
) {
149-
assert(match(Name, pattern) === expected)
150-
assert(nonEmptyCheck({ match, json: { Name }, pattern }) === expected)
151-
}
152-
}
153-
})
154-
})

src/schema/applyRules.ts

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import type { GenericRule, GenericSchema, SchemaFields, SchemaTypeLike } from '../types/schema.ts'
22
import type { Severity } from '../types/issues.ts'
33
import type { BIDSContext } from './context.ts'
4-
import { expressionFunctions } from './expressionLanguage.ts'
4+
import { contextFunction, formatter, prepareContext } from './expressionLanguage.ts'
55
import { logger } from '../utils/logger.ts'
6-
import { memoize } from '../utils/memoize.ts'
76
import { compile } from '../validators/json.ts'
87
import type { DefinedError } from '@ajv'
98
import {
@@ -29,6 +28,35 @@ export function applyRules(
2928
context: BIDSContext,
3029
rootSchema?: GenericSchema,
3130
schemaPath?: string,
31+
): Promise<void> {
32+
_applyRules(schema, prepareContext(context), rootSchema, schemaPath)
33+
return Promise.resolve()
34+
}
35+
36+
/** Evaluate a single expression in a BIDSContext.
37+
*/
38+
export function evalCheck(src: string, context: BIDSContext) {
39+
return _evalCheck(src, prepareContext(context))
40+
}
41+
42+
/**
43+
* Classic rules interpreted like selectors. Examples in specification:
44+
* schema/rules/checks/*
45+
*/
46+
export function evalRuleChecks(
47+
rule: GenericRule,
48+
context: BIDSContext,
49+
schema: GenericSchema,
50+
schemaPath: string,
51+
): boolean {
52+
return _evalRuleChecks(rule, prepareContext(context), schema, schemaPath)
53+
}
54+
55+
function _applyRules(
56+
schema: GenericSchema,
57+
context: BIDSContext,
58+
rootSchema?: GenericSchema,
59+
schemaPath?: string,
3260
) {
3361
if (!rootSchema) {
3462
rootSchema = schema
@@ -45,9 +73,6 @@ export function applyRules(
4573
schemaPath = ''
4674
}
4775
}
48-
Object.assign(context, expressionFunctions)
49-
// @ts-expect-error
50-
context.exists.bind(context)
5176
for (const key in schema) {
5277
if (!(schema[key].constructor === Object)) {
5378
continue
@@ -60,29 +85,20 @@ export function applyRules(
6085
`${schemaPath}.${key}`,
6186
)
6287
} else if (schema[key].constructor === Object) {
63-
applyRules(
88+
_applyRules(
6489
schema[key] as GenericSchema,
6590
context,
6691
rootSchema,
6792
`${schemaPath}.${key}`,
6893
)
6994
}
7095
}
71-
return Promise.resolve()
7296
}
7397

74-
export const evalConstructor = (src: string): Function =>
75-
new Function('context', `with (context) { return ${src.replace(/\\/g, '\\\\')} }`)
76-
const safeHas = () => true
77-
const safeGet = (target: any, prop: any) => prop === Symbol.unscopables ? undefined : target[prop]
78-
79-
const memoizedEvalConstructor = memoize(evalConstructor)
80-
81-
export function evalCheck(src: string, context: BIDSContext) {
82-
const test = memoizedEvalConstructor(src)
83-
const safeContext = new Proxy(context, { has: safeHas, get: safeGet })
98+
function _evalCheck(src: string, context: BIDSContext) {
99+
const test = contextFunction(src)
84100
try {
85-
return test(safeContext)
101+
return test(context)
86102
} catch (error) {
87103
logger.debug(error)
88104
return null
@@ -103,7 +119,7 @@ const evalMap: Record<
103119
schemaPath: string,
104120
) => boolean | void
105121
> = {
106-
checks: evalRuleChecks,
122+
checks: _evalRuleChecks,
107123
// @ts-expect-error
108124
columns: evalColumns,
109125
// @ts-expect-error
@@ -139,27 +155,24 @@ function evalRule(
139155
}
140156

141157
function mapEvalCheck(statements: string[], context: BIDSContext): boolean {
142-
return statements.every((x) => evalCheck(x, context))
158+
return statements.every((x) => _evalCheck(x, context))
143159
}
144160

145-
/**
146-
* Classic rules interpreted like selectors. Examples in specification:
147-
* schema/rules/checks/*
148-
*/
149-
export function evalRuleChecks(
161+
function _evalRuleChecks(
150162
rule: GenericRule,
151163
context: BIDSContext,
152164
schema: GenericSchema,
153165
schemaPath: string,
154166
): boolean {
155167
if (rule.checks && !mapEvalCheck(rule.checks, context)) {
156168
if (rule.issue?.code && rule.issue?.message) {
169+
const format = formatter(rule.issue.message)
157170
context.dataset.issues.add({
158171
code: rule.issue.code,
159172
location: context.path,
160173
rule: schemaPath,
161174
severity: rule.issue.level as Severity,
162-
}, rule.issue.message)
175+
}, format(context))
163176
} else {
164177
context.dataset.issues.add(
165178
{ code: 'CHECK_ERROR', location: context.path, rule: schemaPath },

src/schema/expressionLanguage.test.ts

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { assert } from '@std/assert'
2-
import { expressionFunctions } from './expressionLanguage.ts'
1+
import { assert, assertEquals } from '@std/assert'
2+
import { contextFunction, expressionFunctions, formatter, prepareContext } from './expressionLanguage.ts'
33
import { dataFile, rootFileTree } from './fixtures.test.ts'
44
import { BIDSContext } from './context.ts'
55
import type { DatasetIssues } from '../issues/datasetIssues.ts'
@@ -235,3 +235,82 @@ Deno.test('test expression functions', async (t) => {
235235
assert(allequal([1, 2, 1, 2], [1, 2, 1]) === false)
236236
})
237237
})
238+
239+
Deno.test('contextFunction test', async (t) => {
240+
const match = expressionFunctions.match
241+
await t.step('check veridical reconstruction of match expressions', () => {
242+
// match() functions frequently contain escapes in regex patterns
243+
// We receive these from the schema as written in YAML, so we need to ensure
244+
// that they are correctly reconstructed in the contextFunction() function
245+
//
246+
// The goal is to avoid schema authors needing to double-escape their regex patterns
247+
// and other implementations to account for the double-escaping
248+
let pattern = String.raw`^\.nii(\.gz)?$`
249+
250+
// Check both a literal and a variable pattern produce the same result
251+
for (const check of ['match(extension, pattern)', `match(extension, '${pattern}')`]) {
252+
const niftiCheck = contextFunction(check)
253+
for (
254+
const { extension, expected } of [
255+
{ extension: '.nii', expected: true },
256+
{ extension: '.nii.gz', expected: true },
257+
{ extension: '.tsv', expected: false },
258+
{ extension: ',nii,gz', expected: false }, // Check that . is not treated as a wildcard
259+
]
260+
) {
261+
assert(match(extension, pattern) === expected)
262+
// Pass in a context object to provide any needed variables
263+
assert(niftiCheck({ match, extension, pattern } as unknown as BIDSContext) === expected)
264+
}
265+
}
266+
267+
pattern = String.raw`\S`
268+
for (const check of ['match(json.Name, pattern)', `match(json.Name, '${pattern}')`]) {
269+
const nonEmptyCheck = contextFunction(check)
270+
for (
271+
const { Name, expected } of [
272+
{ Name: 'test', expected: true },
273+
{ Name: '', expected: false },
274+
{ Name: ' ', expected: false },
275+
]
276+
) {
277+
assert(match(Name, pattern) === expected)
278+
assert(
279+
nonEmptyCheck({ match, json: { Name }, pattern } as unknown as BIDSContext) === expected,
280+
)
281+
}
282+
}
283+
})
284+
})
285+
286+
Deno.test('formatter test', async (t) => {
287+
await t.step('simple strings', () => {
288+
const context = {} as BIDSContext
289+
for (
290+
const str of [
291+
'simple string',
292+
'string with `backticks` and \\back\\slashes',
293+
]
294+
) {
295+
assertEquals(formatter(str)(context), str)
296+
}
297+
})
298+
await t.step('format strings', () => {
299+
const context = prepareContext(
300+
{a: 'stringa', b: 'stringb', c: 3, d: {e: 4}, f: [0, 1, 2], g: [1, 2, 3]} as unknown as BIDSContext
301+
)
302+
for (const [str, expected] of [
303+
['{a}', 'stringa'],
304+
['`{a}`', '`stringa`'], // Backticks are preserved
305+
['`````{a}`````', '`````stringa`````'],
306+
['{a} and {b} and {c}', 'stringa and stringb and 3'],
307+
['{a}\\n{d.e}', 'stringa\\n4'], // Backslashes are preserved
308+
['{intersects(f, g)}', '1,2'], // expressions are evaluated
309+
['{z}', 'undefined'],
310+
// Unsupported Pythonisms
311+
// ['{{a}}', '{a}'],
312+
]) {
313+
assertEquals(formatter(str)(context), expected)
314+
}
315+
})
316+
})

src/schema/expressionLanguage.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { BIDSContext } from './context.ts'
2+
import { memoize } from '../utils/memoize.ts'
23

34
function exists(this: BIDSContext, list: string[], rule: string = 'dataset'): number {
45
if (list == null) {
@@ -124,3 +125,49 @@ export const expressionFunctions = {
124125
return (a != null && b != null) && a.length === b.length && a.every((v, i) => v === b[i])
125126
},
126127
}
128+
129+
/**
130+
* Generate a function that evaluates an expression using the BIDS context.
131+
*/
132+
function _contextFunction(expr: string): (context: BIDSContext) => any {
133+
return new Function('context', `with (context) { return ${expr.replace(/\\/g, '\\\\')} }`) as (
134+
context: BIDSContext,
135+
) => any
136+
}
137+
138+
/**
139+
* Generate a function that evaluates an expression using the BIDS context.
140+
*/
141+
export const contextFunction = memoize(_contextFunction)
142+
143+
function _formatter(format: string): (context: BIDSContext) => string {
144+
if (!format.includes('{')) {
145+
return (context: BIDSContext) => format
146+
}
147+
const template = format.replace(/{/g, '${').replace(/\\/g, '\\\\').replace(/`/g, '\\`')
148+
return new Function('context', `with (context) { return \`${template}\` }`) as (
149+
context: BIDSContext,
150+
) => string
151+
}
152+
153+
/**
154+
* Generate a function that formats a string using the BIDS context.
155+
*
156+
* Strings are expected to use Python-style formatting,
157+
* e.g., "sub-{entities.sub}/ses-{entities.ses}".
158+
*/
159+
export const formatter = memoize(_formatter)
160+
161+
function safeContext(context: BIDSContext): BIDSContext {
162+
return new Proxy(context, {
163+
has: () => true,
164+
get: (target: any, prop: any) => prop === Symbol.unscopables ? undefined : target[prop],
165+
})
166+
}
167+
168+
export function prepareContext(context: BIDSContext): BIDSContext {
169+
Object.assign(context, expressionFunctions)
170+
// @ts-expect-error
171+
context.exists.bind(context)
172+
return safeContext(context)
173+
}

0 commit comments

Comments
 (0)