Added better tests and error handling

Author: crutchcorn

all.js

@@ -1,41 +1,67 @@
 'use strict';
 
-const find = require('unist-util-find');
+const is = require('unist-util-is');
 
-function replaceBetween(parent, start, end, func) {
+function replaceAllBetween(parent, start, end, func) {
   if (!parent || !parent.type || !parent.children) {
     throw new Error('Expected parent node');
   }
 
   const {children} = parent;
 
-  const index = check(start);
-  const length = check(end);
+  const isStart = node => is(node, start);
+  const isEnd = node => is(node, end);
 
-  const replaced = children.splice(index, length - index + 1);
+  /**
+   * @type {Array<{start: number, end: number}>}
+   */
+  const ranges = children.reduce((prev, child, i) => {
+    const lastPrev = prev[prev.length - 1];
+    if (isStart(children[i])) {
+      if (lastPrev && lastPrev.end === undefined) {
+        console.error('Attempted to start a replacement before the first one was cleaned up')
+        throw new Error('Attempted to start a replacement before the first one was cleaned up');
+      }
+      prev.push({
+        start: i
+      });
+      return prev;
+    }
+    if (isEnd(children[i])) {
+      if (!lastPrev || (lastPrev && lastPrev.start === undefined)) {
+        console.error('Attempted to end a replacement before finding the start', i, lastPrev)
+        throw new Error('Attempted to end a replacement before finding the start');
+      }
+      lastPrev.end = i;
+    }
+    return prev;
+  }, []);
 
-  const changedArray = func(replaced);
+  if (!ranges[ranges.length - 1].end) {
+    console.error('No ending value was found')
+    throw new Error('No ending value was found');
+  }
 
-  children.splice(index, 0, ...changedArray);
+  // The function will return a different length than the original
+  // This offset is used to correct
+  let offset = 0;
 
-  return changedArray;
+  ranges.map(({start, end}) => {
+    const offsetStart = start + offset;
+    const offsetEnd = end + offset;
 
-  function check(index) {
-    if (index && index.type) {
-      const node = find(parent, index);
-      index = children.indexOf(node);
-    }
+    const replaced = children.slice(offsetStart, offsetEnd + 1);
 
-    if (isNaN(index) || index < 0 || index === Infinity) {
-      throw new Error('Expected positive finite index or child node');
-    }
+    const changedArray = func(replaced);
 
-    if (index >= children.length) {
-      index = children.length - 1;
-    }
+    const diff = children.splice(offsetStart, offsetEnd - offsetStart + 1, ...changedArray);
 
-    return index;
-  }
+    offset -= diff.length - changedArray.length;
+
+    return changedArray;
+  });
+
+  return children;
 }
 
-module.exports = replaceBetween;
+module.exports = replaceAllBetween;

index.js

@@ -0,0 +1,66 @@
+{
+  "name": "unist-util-find-all-between",
+  "version": "2.1.0",
+  "description": "Utility to find nodes between two nodes",
+  "main": "index.js",
+  "engines": {
+    "node": ">=10"
+  },
+  "files": [
+    "index.js",
+    "types/index.d.ts"
+  ],
+  "types": "types/index.d.ts",
+  "scripts": {
+    "lint": "xo",
+    "test:types": "tsd",
+    "test:logic": "tap -Rspec --100 test.js",
+    "test": "npm run lint && npm run test:types && npm run test:logic",
+    "report": "nyc report --reporter=text-lcov | coveralls"
+  },
+  "repository": {
+    "type": "git",
+    "url": "git+https://github.com/mrzmmr/unist-util-find-all-between.git"
+  },
+  "keywords": [
+    "unist",
+    "node",
+    "find",
+    "between",
+    "util",
+    "utility"
+  ],
+  "author": "mrzmmr",
+  "license": "MIT",
+  "bugs": {
+    "url": "https://github.com/mrzmmr/unist-util-find-all-between/issues"
+  },
+  "homepage": "https://github.com/mrzmmr/unist-util-find-all-between#readme",
+  "dependencies": {
+    "unist-util-find": "^1.0.1",
+    "unist-util-is": "^4.0.2"
+  },
+  "devDependencies": {
+    "@types/unist": "^2.0.3",
+    "coveralls": "^3.1.0",
+    "prettier": "^2.0.5",
+    "tap": "^14.10.8",
+    "tsd": "^0.13.1",
+    "typescript": "^3.9.7",
+    "xo": "^0.32.1"
+  },
+  "xo": {
+    "prettier": true,
+    "rules": {
+      "unicorn/prefer-number-properties": "off"
+    }
+  },
+  "prettier": {
+    "tabWidth": 2,
+    "useTabs": false,
+    "singleQuote": true,
+    "bracketSpacing": false,
+    "semi": true,
+    "trailingComma": "none"
+  }
+}

package.json

@@ -4,6 +4,43 @@ const {test} = require('tap');
 const findAllBetween = require('.');
 
 test('unist-util-find-all-between', (test) => {
+  test.throws(() => {
+    findAllBetween();
+  }, 'Should fail without parent node');
+
+  test.throws(() => {
+    findAllBetween({type: 'foo'});
+  }, 'Should fail without parent node');
+
+  test.throws(() => {
+    findAllBetween({type: 'foo', children: [
+        {type: 'start'},
+        {type: 'start'}
+      ]}, {type: 'start'}, {type: 'end'});
+  }, 'Tried getting a new start instead of ending');
+
+  test.throws(() => {
+    findAllBetween({type: 'foo', children: [
+        {type: 'start'},
+        {type: 'end'},
+        {type: 'start'}
+      ]}, {type: 'start'}, {type: 'end'});
+  }, 'No ending value was found');
+
+  test.throws(() => {
+    findAllBetween({type: 'foo', children: [
+        {type: 'end'},
+        {type: 'start'}
+      ]}, {type: 'start'}, {type: 'end'});
+  }, 'Attempted to end a replacement before finding the start');
+
+  test.throws(() => {
+    findAllBetween({type: 'foo', children: [
+        {type: 'start'},
+        {type: 'end'},
+        {type: 'end'},
+      ]}, {type: 'start'}, {type: 'end'});
+  }, 'Attempted to end a replacement before finding the start');
 
   test.doesNotThrow(() => {
     test.deepEqual(
@@ -22,8 +59,8 @@ test('unist-util-find-all-between', (test) => {
             }
           ]
         },
-        0,
-        2,
+        {type: 'foo'},
+        {type: 'baz'},
         (v) => [{type: 'test', data: v.length}]
       ),
       [
@@ -33,7 +70,61 @@ test('unist-util-find-all-between', (test) => {
         }
       ]
     );
-  }, 'Replaces range with new output');
+  }, 'Replaces single match with new output');
+
+  test.doesNotThrow(() => {
+    test.deepEqual(
+      findAllBetween(
+        {
+          type: 'foo',
+          children: [
+            {
+              type: 'foo'
+            },
+            {
+              type: 'bar'
+            },
+            {
+              type: 'baz'
+            },
+            {
+              type: 'hello'
+            },
+
+            {
+              type: 'foo'
+            },
+            {
+              type: 'what'
+            },
+            {
+              type: 'another'
+            },
+            {
+              type: 'baz'
+            },
+          ]
+        },
+        {type: 'foo'},
+        {type: 'baz'},
+        (v) => [{type: 'test', data: v.length}]
+      ),
+      [
+        {
+          type: 'test',
+          data: 3
+        },
+        {
+          type: 'hello'
+        },
+        {
+          type: 'test',
+          data: 4
+        }
+      ]
+    );
+  }, 'Replaces two matches with new output');
+
 
   test.end();
 });

test.js

@@ -5,16 +5,16 @@ import {Node, Parent} from 'unist';
 export = findAllBetween;
 
 /**
- * A unist utility to get all children of a parent between two nodes or indices.
+ * A unist utility to get all children of multiple different parents between two nodes or indices.
  *
  * @param parent Parent node to search in
- * @param start A node or index to start search with
- * @param end A node or index to end search with
+ * @param start A node to start search with
+ * @param end A node to end search with
  * @param func The function ran to change the nodes
  */
 declare function findAllBetween<T extends Node>(
   parent: Parent,
-  start: Node | number,
-  end: Node | number,
+  start: Node,
+  end: Node,
   func: (val: Node[]) => Node[]
-): Node[];
+): Node[][];

types/index.d.ts

@@ -17,9 +17,9 @@ expectError(between(parent, n1, n2));
 /**
  * Incorrect types of arguments
  */
-expectError(between((v: Node[]) => v));
-expectError(between(parent, (v: Node[]) => v));
-expectError(between(parent, n1, (v: Node[]) => v));
+expectError(between((v: Node[][]) => v));
+expectError(between(parent, (v: Node[][]) => v));
+expectError(between(parent, n1, (v: Node[][]) => v));
 
 /**
  * Incorrect test type