🐛 Do not raise errors on non-accessible sub-trees (#2532)

Author: rowanc1

.changeset/smart-pillows-follow.md

@@ -0,0 +1,5 @@
+---
+'myst-parser': patch
+---
+
+Do not raise errors on unprocessed nodes

packages/myst-parser/src/directives.ts

@@ -3,18 +3,18 @@ import type {
   DirectiveData,
   DirectiveSpec,
   DirectiveContext,
-  ParseTypes,
   GenericParent,
 } from 'myst-common';
 import { RuleId, fileError, fileWarn } from 'myst-common';
 import { selectAll } from 'unist-util-select';
 import type { VFile } from 'vfile';
-import { contentFromNode } from './utils.js';
+import { contentFromNode, markChildrenAsProcessed } from './utils.js';
 import type { Directive } from 'myst-spec';
 import { parseOptions } from './inlineAttributes.js';
 
 type MystDirectiveNode = GenericNode & {
   name: string;
+  processed?: false;
 };
 /**
  * Apply directive `run()` methods to build directive ASTs.
@@ -47,12 +47,20 @@ export function applyDirectives(
       }
     });
   });
-  // Find all raw directive nodes
+  // Find all raw directive nodes, these will have a `processed` attribute set to `false`
   const nodes = selectAll('mystDirective[processed=false]', tree) as MystDirectiveNode[];
   nodes.forEach((node) => {
-    delete node.processed; // Indicate that the directive has been processed
     const { name } = node;
     const spec = specLookup[name];
+    if (spec && spec.body && spec.body.type !== 'myst') {
+      // Do not process the children of the directive that is not a myst block
+      // Doing so would raise errors (for example, if the parent directive is a code block)
+      // See https://github.com/jupyter-book/mystmd/issues/2530
+      markChildrenAsProcessed(node);
+    }
+    // Re-check if the directive has been processed
+    if (node.processed !== false) return;
+    delete node.processed; // Indicate that the directive has been processed
     if (!spec) {
       fileError(vfile, `unknown directive: ${name}`, { node, ruleId: RuleId.directiveKnown });
       // We probably want to do something better than just delete the children and

packages/myst-parser/src/roles.ts

@@ -3,11 +3,12 @@ import { RuleId, fileError, fileWarn } from 'myst-common';
 import type { Role } from 'myst-spec';
 import { selectAll } from 'unist-util-select';
 import type { VFile } from 'vfile';
-import { contentFromNode } from './utils.js';
+import { contentFromNode, markChildrenAsProcessed } from './utils.js';
 import { parseOptions } from './inlineAttributes.js';
 
 type MystRoleNode = GenericNode & {
   name: string;
+  processed?: false;
 };
 
 export function applyRoles(tree: GenericParent, specs: RoleSpec[], vfile: VFile) {
@@ -27,11 +28,20 @@ export function applyRoles(tree: GenericParent, specs: RoleSpec[], vfile: VFile)
       }
     });
   });
+  // Find all raw role nodes, these will have a `processed` attribute set to `false`
   const nodes = selectAll('mystRole[processed=false]', tree) as MystRoleNode[];
   nodes.forEach((node) => {
-    delete node.processed; // Indicate that the role has been processed
     const { name } = node;
     const spec = specLookup[name];
+    if (spec && spec.body && spec.body.type !== 'myst') {
+      // Do not process the children of the role that is not a myst block
+      // Doing so would raise errors (for example, if the parent role is a cite)
+      // See https://github.com/jupyter-book/mystmd/issues/2530
+      markChildrenAsProcessed(node);
+    }
+    // Re-check if the role has been processed
+    if (node.processed !== false) return;
+    delete node.processed; // Indicate that the role has been processed
     if (!spec) {
       fileError(vfile, `unknown role: ${name}`, { node, ruleId: RuleId.roleKnown });
       // We probably want to do something better than just delete the children

packages/myst-parser/src/utils.ts

@@ -1,5 +1,6 @@
 import { fileError, fileWarn, ParseTypesEnum } from 'myst-common';
 import type { GenericNode, ArgDefinition, RuleId } from 'myst-common';
+import { selectAll } from 'unist-util-select';
 import type { VFile } from 'vfile';
 
 export function contentFromNode(
@@ -59,3 +60,13 @@ export function contentFromNode(
     return !!value;
   }
 }
+
+export function markChildrenAsProcessed(node: GenericNode) {
+  const children = selectAll('mystDirective,mystRole', {
+    type: 'root',
+    children: node.children,
+  }) as GenericNode[];
+  children.forEach((child) => {
+    delete child.processed;
+  });
+}

packages/myst-parser/tests/directives/basicDirective.spec.ts

@@ -0,0 +1,20 @@
+import { describe, expect, test } from 'vitest';
+import { RuleId } from 'myst-common';
+import { mystParse } from '../../src';
+import { VFile } from 'vfile';
+
+describe('basic directive tests', () => {
+  test('Errors not raised for directives that are not processed', () => {
+    const vfile1 = new VFile();
+    mystParse('````{code}\n```{dontraiseerror}\n```\n````', { vfile: vfile1 }) as any;
+    expect(vfile1.messages).toEqual([]);
+    const vfile2 = new VFile();
+    mystParse('```{raiseerror}\nblah\n```', { vfile: vfile2 }) as any;
+    expect(vfile2.messages).toMatchObject([
+      {
+        message: 'unknown directive: raiseerror',
+        ruleId: RuleId.directiveKnown,
+      },
+    ]);
+  });
+});

packages/myst-parser/tests/myst.spec.ts

@@ -217,6 +217,6 @@ describe('Testing mdast --> html conversions', () => {
 
 if (skipped.length) {
   describe('Skipped Tests', () => {
-    test.skip.each(skipped)('%s', () => null);
+    test.skip.each(skipped)('%s', () => undefined);
   });
 }

packages/myst-parser/tests/roles/basicRole.spec.ts

@@ -1,7 +1,14 @@
 import { describe, expect, test } from 'vitest';
-import { normalizeLabel, type GenericNode, type RoleData, type RoleSpec } from 'myst-common';
+import {
+  normalizeLabel,
+  RuleId,
+  type GenericNode,
+  type RoleData,
+  type RoleSpec,
+} from 'myst-common';
 import { mystParse } from '../../src';
 import { position, positionFn } from '../position';
+import { VFile } from 'vfile';
 
 describe('role spec with options', () => {
   test('complex inline role with options', () => {
@@ -88,4 +95,17 @@ describe('role spec with options', () => {
       ],
     });
   });
+  test('Errors not raised for roles that are not processed', () => {
+    const vfile1 = new VFile();
+    mystParse('{cite}`` {noerror}`asdf` ``', { vfile: vfile1 }) as any;
+    expect(vfile1.messages).toEqual([]);
+    const vfile2 = new VFile();
+    mystParse('{error}`asdf`', { vfile: vfile2 }) as any;
+    expect(vfile2.messages).toMatchObject([
+      {
+        message: 'unknown role: error',
+        ruleId: RuleId.roleKnown,
+      },
+    ]);
+  });
 });