Merge pull request #2104 from effigies/feat/warn-undefined-columns

feat(tsv): Warn on undefined columns

Author: rwblair

bids-validator/src/issues/list.ts

@@ -96,6 +96,14 @@ export const bidsIssues: IssueDefinitionRecord = {
     severity: 'error',
     reason: 'A TSV file has extra columns which are not allowed for its file type',
   },
+  TSV_ADDITIONAL_COLUMNS_MUST_DEFINE: {
+    severity: 'error',
+    reason: 'Additional TSV columns must be defined in the associated JSON sidecar for this file type',
+  },
+  TSV_ADDITIONAL_COLUMNS_UNDEFINED: {
+    severity: 'warning',
+    reason: 'A TSV file has extra columns which are not defined in its associated JSON sidecar',
+  },
   TSV_INDEX_VALUE_NOT_UNIQUE: {
     severity: 'error',
     reason:

bids-validator/src/schema/tables.test.ts

@@ -149,7 +149,7 @@ Deno.test('tables eval* tests', async (t) => {
     )
   })
 
-  await t.step('verify additional columns', () => {
+  await t.step('verify not allowed additional columns', () => {
     const context = {
       path: '/sub-01/sub-01_scans.tsv',
       extension: '.tsv',
@@ -171,4 +171,35 @@ Deno.test('tables eval* tests', async (t) => {
       1,
     )
   })
+  await t.step('verify allowed and allowed_if_defined additional columns', () => {
+    const context = {
+      path: '/sub-01/sub-01_scans.tsv',
+      extension: '.tsv',
+      sidecar: {'extra': {'description': 'a fun and whimsical extra column'}},
+      columns: {
+        onset: ['1', '2', 'n/a'],
+        strain_rrid: ['RRID:SCR_012345', 'RRID:SCR_012345', 'n/a'],
+        extra: [1, 2, 3]
+      },
+      dataset: { issues: new DatasetIssues() },
+    }
+    const rule = schemaDefs.rules.tabular_data.made_up.MadeUp
+    rule.additional_columns = "allowed_if_defined"
+    evalAdditionalColumns(rule, context, schema, 'rules.tabular_data.made_up.MadeUp')
+    assertEquals(context.dataset.issues.size, 0)
+
+    context['sidecar'] = {}
+    evalAdditionalColumns(rule, context, schema, 'rules.tabular_data.made_up.MadeUp')
+    assertEquals(
+      context.dataset.issues.get({ code: 'TSV_ADDITIONAL_COLUMNS_MUST_DEFINE' }).length,
+      1,
+    )
+
+    rule.additional_columns = "allowed"
+    evalAdditionalColumns(rule, context, schema, 'rules.tabular_data.made_up.MadeUp')
+    assertEquals(
+      context.dataset.issues.get({ code: 'TSV_ADDITIONAL_COLUMNS_UNDEFINED' }).length,
+      1,
+    )
+  })
 })

bids-validator/src/schema/tables.ts

@@ -238,25 +238,32 @@ export function evalAdditionalColumns(
 ): void {
   if (context.extension !== '.tsv') return
   const headers = Object.keys(context?.columns)
-  // hard coding allowed here feels bad
-  if (!(rule.additional_columns === 'allowed') && rule.columns) {
+  if (rule.columns) {
     const ruleHeadersNames = Object.keys(rule.columns).map(
       // @ts-expect-error
       (x) => schema.objects.columns[x].name,
     )
     let extraCols = headers.filter(
       (header) => !ruleHeadersNames.includes(header),
     )
-    if (rule.additional_columns === 'allowed_if_defined') {
+
+    if (rule.additional_columns?.startsWith('allowed')) {
       extraCols = extraCols.filter((header) => !(header in context.sidecar))
     }
+    const code = (
+	rule.additional_columns === 'allowed'
+	? 'TSV_ADDITIONAL_COLUMNS_UNDEFINED'
+	: rule.additional_columns === 'allowed_if_defined'
+	? 'TSV_ADDITIONAL_COLUMNS_MUST_DEFINE'
+	: 'TSV_ADDITIONAL_COLUMNS_NOT_ALLOWED'
+    )
+    const issue = {
+      code,
+      location: context.path,
+      rule: schemaPath,
+    }
     for (const col of extraCols) {
-      context.dataset.issues.add({
-        code: 'TSV_ADDITIONAL_COLUMNS_NOT_ALLOWED',
-        subCode: col,
-        location: context.path,
-        rule: schemaPath,
-      })
+      context.dataset.issues.add({ ...issue, subCode: col })
     }
   }
 }