Warn on presence of deprecated metadata fields

[psadil]

"deprecated" is a valid requirement_level, according to the metaschema: https://github.com/bids-standard/bids-specification/blob/4235ae7a2a523b657f502b7b0c852093585d7614/src/metaschema.json#L554-L558

  "requirement_level": {
      "$comment": "Requirement levels that may apply to terms",
      "type": "string",
      "enum": ["required", "recommended", "optional", "deprecated"]
    }

So, with no "deprecated" option, the "severity" of a deprecated field is set to "undefined". The old block was skipped when the guard evaluated to false (as "undefined" would cause it to), meaning no warning was emitted

bids-validator/src/schema/applyRules.ts

Lines 208 to 230 in 024f687

	if (severity && severity !== 'ignore') {
	if (requirement.issue?.code && requirement.issue?.message) {
	context.dataset.issues.add({
	code: requirement.issue.code,
	subCode: keyName,
	location: context.path,
	severity,
	rule: schemaPath,
	issueMessage,
	}, requirement.issue.message)
	} else {
	const keyType = sidecarRule ? 'SIDECAR_KEY' : 'JSON_KEY'
	const level = severity === 'error' ? 'REQUIRED' : 'RECOMMENDED'
	context.dataset.issues.add({
	code: `${keyType}_${level}`,
	subCode: keyName,
	location: context.path,
	severity,
	rule: schemaPath,
	issueMessage,
	})
	}
	}

[It] seems the mapping from requirement_level to severity should be schema-driven, rather than hard-coded from a list of strings. But reverting that guard will also address the issue.

Note: The above was edited by @effigies to focus on the remaining issue, which is that deprecated fields fall through the logical cracks. The motivating bug was fixed in #436.

---

Original post

This is limited to the pre-release and not the latest stable (silly me for working with alpha).

Current alpha release (AcquisitionDuration is deprecated)

❯ deno run -A lib/bids-validator/src/bids-validator.ts --version
bids-validator 3.0.0-alpha.4-2-g6eaae4e9

❯ deno run -A lib/bids-validator/src/bids-validator.ts tests/data/bids-examples/ds003 --ignoreNiftiHeaders --json | jq | rg -B1  AcquisitionDuration 
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",
--
        "code": "SIDECAR_KEY_RECOMMENDED",
        "subCode": "AcquisitionDuration",

Current stable

❯ deno run -ERWN jsr:@bids/validator --version                  
bids-validator 2.4.1

❯ deno run -ERWN jsr:@bids/validator  tests/data/bids-examples/ds003 --ignoreNiftiHeaders --json | jq | rg AcquisitionDuration  | wc -l
       0

So, a deprecated field like AcquisitionDuration that is properly missing is being flagged as recommended.

I think the difference is from b6635d3, interacting with requirement_levels being hardcoded:

bids-validator/src/schema/applyRules.ts

Lines 296 to 301 in 6eaae4e

	const levelToSeverity: Record<string, Severity> = {
	recommended: 'warning',
	required: 'error',
	optional: 'ignore',
	prohibited: 'ignore',
	}

"deprecated" is a valid requirement_level, according to the metaschema: https://github.com/bids-standard/bids-specification/blob/4235ae7a2a523b657f502b7b0c852093585d7614/src/metaschema.json#L554-L558

  "requirement_level": {
      "$comment": "Requirement levels that may apply to terms",
      "type": "string",
      "enum": ["required", "recommended", "optional", "deprecated"]
    }

So, with no "deprecated" option, the "severity" of a deprecated field is set to "undefined". The old block was skipped when the guard evaluated to false (as "undefined" would cause it to), meaning no warning was emitted

bids-validator/src/schema/applyRules.ts

Lines 208 to 230 in 024f687

	if (severity && severity !== 'ignore') {
	if (requirement.issue?.code && requirement.issue?.message) {
	context.dataset.issues.add({
	code: requirement.issue.code,
	subCode: keyName,
	location: context.path,
	severity,
	rule: schemaPath,
	issueMessage,
	}, requirement.issue.message)
	} else {
	const keyType = sidecarRule ? 'SIDECAR_KEY' : 'JSON_KEY'
	const level = severity === 'error' ? 'REQUIRED' : 'RECOMMENDED'
	context.dataset.issues.add({
	code: `${keyType}_${level}`,
	subCode: keyName,
	location: context.path,
	severity,
	rule: schemaPath,
	issueMessage,
	})
	}
	}

but now the meaning of that block is swapped

bids-validator/src/schema/applyRules.ts

Lines 208 to 210 in 6eaae4e

	if (severity && (severity === 'ignore')) {
	continue
	}

And so there is a failure to enter into it.

At a deeper level, it seems the mapping from requirement_level to severity should be schema-driven, rather than hard-coded from a list of strings. But reverting that guard will also address the issue.

[effigies]

I believe the fix that preserves the rest of b6635d3 would be:

-      if (severity && (severity === 'ignore')) {
+      if (!severity || severity === 'ignore') {
         continue
       }

[effigies]

Updated the title and initial post to focus on the feature of warning on the presence of deprecated fields.