Improve schema validation for nodes and min/max (#2388)

* Better schema violations for underflow and overflow

* Rename child_required_under/overflow to child_min/max_invalid

* Remove tailing whitespace

* Add tests to cover more branches

* Insert missing comma

docs/reference/slate/schema.md

@@ -303,17 +303,34 @@ When supplying your own `normalize` property for a schema rule, it will be calle
 
 Raised when the `object` property of a child node is invalid.
 
-### `'child_required'`
+### `child_min_invalid`
 
 ```js
 {
   index: Number,
+  count: Number,
+  limit: Number,
   node: Node,
   rule: Object,
 }
 ```
 
-Raised when a child node was required but none was found.
+Raised when a child node repeats less than required by a rule's `min` property.
+
+### `child_max_invalid`
+
+```js
+{
+  index: Number,
+  count: Number,
+  limit: Number,
+  node: Node,
+  rule: Object,
+}
+```
+
+Raised when a child node repeats more than permitted by a rule's `max`
+property.
 
 ### `'child_type_invalid'`
 

examples/forced-layout/index.js

@@ -22,7 +22,7 @@ const schema = {
           const type = index === 0 ? 'title' : 'paragraph'
           return editor.setNodeByKey(child.key, type)
         }
-        case 'child_required': {
+        case 'child_min_invalid': {
           const block = Block.create(index === 0 ? 'title' : 'paragraph')
           return editor.insertNodeByKey(node.key, index, block)
         }

packages/slate/src/plugins/core.js

@@ -100,7 +100,7 @@ function CorePlugin(options = {}) {
         normalize: (editor, error) => {
           const { code, node } = error
 
-          if (code === 'child_required') {
+          if (code === 'child_min_invalid' && node.nodes.isEmpty()) {
             editor.insertNodeByKey(node.key, 0, Text.create())
           }
         },

packages/slate/src/plugins/schema.js

@@ -157,7 +157,7 @@ function SchemaPlugin(schema) {
  */
 
 function defaultNormalize(editor, error) {
-  const { code, node, child, next, previous, key, mark } = error
+  const { code, node, child, next, previous, key, mark, index } = error
 
   switch (code) {
     case 'child_object_invalid':
@@ -192,7 +192,7 @@ function defaultNormalize(editor, error) {
         : editor.removeNodeByKey(next.key)
     }
 
-    case 'child_required':
+    case 'child_min_invalid':
     case 'node_text_invalid':
     case 'parent_object_invalid':
     case 'parent_type_invalid': {
@@ -201,6 +201,10 @@ function defaultNormalize(editor, error) {
         : editor.removeNodeByKey(node.key)
     }
 
+    case 'child_max_invalid': {
+      return editor.removeNodeByKey(node.nodes.get(index).key)
+    }
+
     case 'node_data_invalid': {
       return node.data.get(key) === undefined && node.object !== 'document'
         ? editor.removeNodeByKey(node.key)
@@ -359,36 +363,42 @@ function validateNodes(node, rule, rules = []) {
 
   const children = node.nodes.toArray()
   const defs = rule.nodes != null ? rule.nodes.slice() : []
-  let offset
+  let count = 0
-  let min
+  let lastCount = 0
-  let index
+  let min = null
-  let def
+  let index = -1
-  let max
+  let def = null
-  let child
+  let max = null
-  let previous
+  let child = null
-  let next
+  let previous = null
+  let next = null
 
   function nextDef() {
-    offset = offset == null ? null : 0
+    if (defs.length === 0) return false
     def = defs.shift()
-    min = def && def.min
+    lastCount = count
-    max = def && def.max
+    count = 0
-    return !!def
+    min = def.min || null
+    max = def.max || null
+    return true
   }
 
   function nextChild() {
-    index = index == null ? 0 : index + 1
+    index += 1
-    offset = offset == null ? 0 : offset + 1
     previous = child
     child = children[index]
     next = children[index + 1]
-    if (max != null && offset == max) nextDef()
+    if (!child) return false
-    return !!child
+    lastCount = count
+    count += 1
+    return true
   }
 
-  function rewind() {
+  function rewind(pastZero = false) {
-    offset -= 1
+    if (index > 0 || pastZero) {
-    index -= 1
+      index -= 1
+      count = lastCount
+    }
   }
 
   if (rule.nodes != null) {
@@ -411,12 +421,71 @@ function validateNodes(node, rule, rules = []) {
       if (def.match) {
         const error = validateRules(child, def.match)
 
-        if (error && offset >= min && nextDef()) {
-          rewind()
-          continue
-        }
-
         if (error) {
+          if (max != null && count - 1 > max) {
+            // Since we want to report overflow on last matching child we don't
+            // immediately check for count > max, but instead do so once we find
+            // a child that doesn't match.
+            rewind()
+            return fail('child_max_invalid', {
+              rule,
+              node,
+              index,
+              count,
+              limit: max,
+            })
+          }
+
+          const lastMin = min
+
+          // If there are more groups after this one then child might actually
+          // be valid.
+          if (nextDef()) {
+            // We already have all children required for current group, so this
+            // error can safely be ignored.
+            if (lastCount - 1 >= lastMin) {
+              rewind(true)
+              continue
+            }
+
+            // Otherwise we know that current value is underflowing. There are
+            // three possible causes: there might just not be enough elements
+            // for current group, and current child is in fact the first of
+            // the next group; current group is underflowing, but there is also
+            // an invalid child before the next group; or current group is not
+            // underflowing but it appears so because there's an invalid child
+            // between its members.
+            if (validateRules(child, def.match) == null) {
+              // It's the first case, so we just report an underflow.
+              rewind()
+              return fail('child_min_invalid', {
+                rule,
+                node,
+                index,
+                count: lastCount - 1,
+                limit: lastMin,
+              })
+            }
+            // It's either the second or third case. If it's the second then
+            // we could report an underflow, but presence of an invalid child
+            // is arguably more important, so we report it first. It also lets
+            // us avoid checking for which case exactly is it.
+
+            error.rule = rule
+            error.node = node
+            error.child = child
+            error.index = index
+            error.code = error.code.replace('node_', 'child_')
+            return error
+          }
+
+          // Otherwise either we exhausted the last group, in which case it's
+          // an unknown child, ...
+          if (max != null && count > max) {
+            return fail('child_unknown', { rule, node, child, index })
+          }
+
+          // ... or it's an invalid child for the last group.
           error.rule = rule
           error.node = node
           error.child = child
@@ -428,14 +497,30 @@ function validateNodes(node, rule, rules = []) {
     }
   }
 
-  if (rule.nodes != null) {
+  if (max != null && count > max) {
-    while (min != null) {
+    // Since we want to report overflow on last matching child we don't
-      if (offset < min) {
+    // immediately check for count > max, but do so after processing all nodes.
-        return fail('child_required', { rule, node, index })
+    return fail('child_max_invalid', {
-      }
+      rule,
+      node,
+      index: index - 1,
+      count,
+      limit: max,
+    })
+  }
 
-      nextDef()
+  if (rule.nodes != null) {
-    }
+    do {
+      if (count < min) {
+        return fail('child_min_invalid', {
+          rule,
+          node,
+          index,
+          count,
+          limit: min,
+        })
+      }
+    } while (nextDef())
   }
 }
 

packages/slate/test/schema/custom/child-max-invalid-at-end-default.js

@@ -0,0 +1,44 @@
+/** @jsx h */
+
+import h from '../../helpers/h'
+
+export const schema = {
+  blocks: {
+    paragraph: {},
+    quote: {
+      nodes: [
+        {
+          match: [{ type: 'paragraph' }],
+          max: 1,
+        },
+      ],
+    },
+  },
+}
+
+export const input = (
+  <value>
+    <document>
+      <quote>
+        <paragraph>
+          <text />
+        </paragraph>
+        <paragraph>
+          <text />
+        </paragraph>
+      </quote>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <quote>
+        <paragraph>
+          <text />
+        </paragraph>
+      </quote>
+    </document>
+  </value>
+)

packages/slate/test/schema/custom/child-max-invalid-custom.js

@@ -0,0 +1,52 @@
+/** @jsx h */
+
+import h from '../../helpers/h'
+
+export const schema = {
+  blocks: {
+    paragraph: {},
+    quote: {
+      nodes: [
+        {
+          match: [{ type: 'title' }],
+          max: 1,
+        },
+        {
+          match: [{ type: 'paragraph' }],
+        },
+      ],
+      normalize: (editor, { code, node, index }) => {
+        if (code == 'child_max_invalid') {
+          editor.mergeNodeByKey(node.nodes.get(index).key)
+        }
+      },
+    },
+  },
+}
+
+export const input = (
+  <value>
+    <document>
+      <quote>
+        <block type="title">One</block>
+        <block type="title">Two</block>
+        <paragraph>
+          <text />
+        </paragraph>
+      </quote>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <quote>
+        <block type="title">OneTwo</block>
+        <paragraph>
+          <text />
+        </paragraph>
+      </quote>
+    </document>
+  </value>
+)

packages/slate/test/schema/custom/child-max-invalid-default.js

@@ -0,0 +1,47 @@
+/** @jsx h */
+
+import h from '../../helpers/h'
+
+export const schema = {
+  blocks: {
+    paragraph: {},
+    quote: {
+      nodes: [
+        {
+          match: [{ type: 'title' }],
+          max: 1,
+        },
+        {
+          match: [{ type: 'paragraph' }],
+        },
+      ],
+    },
+  },
+}
+
+export const input = (
+  <value>
+    <document>
+      <quote>
+        <block type="title">One</block>
+        <block type="title">Two</block>
+        <paragraph>
+          <text />
+        </paragraph>
+      </quote>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <quote>
+        <block type="title">One</block>
+        <paragraph>
+          <text />
+        </paragraph>
+      </quote>
+    </document>
+  </value>
+)

packages/slate/test/schema/custom/child-min-invalid-at-end-custom.js

@@ -13,7 +13,7 @@ export const schema = {
         },
       ],
       normalize: (editor, { code, node, index }) => {
-        if (code == 'child_required') {
+        if (code == 'child_min_invalid') {
           editor.insertNodeByKey(node.key, index, {
             object: 'block',
             type: 'paragraph',

packages/slate/test/schema/custom/child-min-invalid-custom.js

@@ -0,0 +1,56 @@
+/** @jsx h */
+
+import h from '../../helpers/h'
+
+export const schema = {
+  blocks: {
+    paragraph: {},
+    title: {},
+    quote: {
+      nodes: [
+        {
+          match: [{ type: 'title' }],
+          min: 1,
+        },
+        {
+          match: [{ type: 'paragraph' }],
+        },
+      ],
+      normalize: (editor, { code, node, index }) => {
+        if (code == 'child_min_invalid' && index == 0) {
+          editor.insertNodeByKey(node.key, index, {
+            object: 'block',
+            type: 'title',
+          })
+        }
+      },
+    },
+  },
+}
+
+export const input = (
+  <value>
+    <document>
+      <quote>
+        <paragraph>
+          <text />
+        </paragraph>
+      </quote>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <quote>
+        <block type="title">
+          <text />
+        </block>
+        <paragraph>
+          <text />
+        </paragraph>
+      </quote>
+    </document>
+  </value>
+)

packages/slate/test/schema/custom/child-min-invalid-default.js

@@ -0,0 +1,39 @@
+/** @jsx h */
+
+import h from '../../helpers/h'
+
+export const schema = {
+  blocks: {
+    paragraph: {},
+    title: {},
+    quote: {
+      nodes: [
+        {
+          match: [{ type: 'title' }],
+          min: 1,
+        },
+        {
+          match: [{ type: 'paragraph' }],
+        },
+      ],
+    },
+  },
+}
+
+export const input = (
+  <value>
+    <document>
+      <quote>
+        <paragraph>
+          <text />
+        </paragraph>
+      </quote>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document />
+  </value>
+)

packages/slate/test/schema/custom/child-min-invalid-with-invalid-default.js

@@ -0,0 +1,59 @@
+/** @jsx h */
+
+import h from '../../helpers/h'
+
+export const schema = {
+  blocks: {
+    paragraph: {},
+    quote: {
+      nodes: [
+        {
+          match: [{ type: 'paragraph' }],
+          min: 2,
+        },
+        {
+          match: [{ type: 'final' }],
+        },
+      ],
+    },
+  },
+}
+
+export const input = (
+  <value>
+    <document>
+      <quote>
+        <paragraph>
+          <text />
+        </paragraph>
+        <block type="invalid">
+          <text />
+        </block>
+        <paragraph>
+          <text />
+        </paragraph>
+        <block type="final">
+          <text />
+        </block>
+      </quote>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <quote>
+        <paragraph>
+          <text />
+        </paragraph>
+        <paragraph>
+          <text />
+        </paragraph>
+        <block type="final">
+          <text />
+        </block>
+      </quote>
+    </document>
+  </value>
+)

packages/slate/test/schema/custom/child-unknown-custom.js

@@ -33,7 +33,7 @@ export const input = (
     <document>
       <quote>
         <paragraph>one</paragraph>
-        <paragraph>two</paragraph>
+        <block type="title">two</block>
       </quote>
     </document>
   </value>

packages/slate/test/schema/custom/child-unknown-default.js

@@ -21,7 +21,7 @@ export const input = (
     <document>
       <quote>
         <paragraph>one</paragraph>
-        <paragraph>two</paragraph>
+        <block type="title">two</block>
       </quote>
     </document>
   </value>