Change to return undefined for invalid points, positions

Closes GH-12.

Reviewed-by: Christian Murphy <christian.murphy.42@gmail.com>
Reviewed-by: Remco Haszing <remcohaszing@gmail.com>

Author: wooorm

lib/index.js

@@ -44,11 +44,16 @@ export const pointEnd = point('end')
  *
  * @param {NodeLike | Node | null | undefined} [node]
  *   Node.
- * @returns {Position}
+ * @returns {Position | undefined}
  *   Position.
  */
 export function position(node) {
-  return {start: pointStart(node), end: pointEnd(node)}
+  const start = pointStart(node)
+  const end = pointEnd(node)
+
+  if (start && end) {
+    return {start, end}
+  }
 }
 
 /**
@@ -66,19 +71,25 @@ function point(type) {
    * Get the point info of `node` at a bound side.
    *
    * @param {NodeLike | Node | null | undefined} [node]
-   * @returns {Point}
+   * @returns {Point | undefined}
    */
   function point(node) {
     const point = (node && node.position && node.position[type]) || {}
 
-    // To do: next major: don’t return points when invalid.
-    return {
-      // @ts-expect-error: in practice, null is allowed.
-      line: point.line || null,
-      // @ts-expect-error: in practice, null is allowed.
-      column: point.column || null,
-      // @ts-expect-error: in practice, null is allowed.
-      offset: point.offset > -1 ? point.offset : null
+    if (
+      typeof point.line === 'number' &&
+      point.line > 0 &&
+      typeof point.column === 'number' &&
+      point.column > 0
+    ) {
+      return {
+        line: point.line,
+        column: point.column,
+        offset:
+          typeof point.offset === 'number' && point.offset > -1
+            ? point.offset
+            : undefined
+      }
     }
   }
 }

readme.md

@@ -106,7 +106,7 @@ Get the positional info of `node`.
 
 ###### Returns
 
-Position ([`Position`][unist-position]).
+Position, if valid ([`Position`][unist-position] or `undefined`).
 
 ### `pointEnd(node)`
 
@@ -119,7 +119,7 @@ Get the ending point of `node`.
 
 ###### Returns
 
-Point ([`Point`][unist-point]).
+Point, if valid ([`Point`][unist-point] or `undefined`).
 
 ### `pointStart(node)`
 
@@ -132,7 +132,7 @@ Get the starting point of `node`.
 
 ###### Returns
 
-Point ([`Point`][unist-point]).
+Point, if valid ([`Point`][unist-point] or `undefined`).
 
 ## Types
 

test.js

@@ -17,8 +17,6 @@ const noPoints = {type: 'c', position: {}}
 
 const noPosition = {type: 'd'}
 
-const generated = {line: null, column: null, offset: null}
-
 test('core', () => {
   assert.deepEqual(
     Object.keys(mod).sort(),
@@ -34,28 +32,52 @@ test('position', () => {
     'should get the whole position'
   )
 
+  assert.deepEqual(
+    position({
+      type: 'x',
+      position: {
+        start: {line: 0, column: 0, offset: -1},
+        end: {line: 0, column: 0, offset: -1}
+      }
+    }),
+    undefined,
+    'should not get too low values'
+  )
+
+  assert.deepEqual(
+    position({
+      type: 'x',
+      position: {start: {line: 1, column: 1}, end: {line: 1, column: 2}}
+    }),
+    {
+      start: {line: 1, column: 1, offset: undefined},
+      end: {line: 1, column: 2, offset: undefined}
+    },
+    'should support points w/o `offset`'
+  )
+
   assert.deepEqual(
     position(noFields),
-    {start: generated, end: generated},
-    'should return an empty position without fields'
+    undefined,
+    'should return nothing when without fields'
   )
 
   assert.deepEqual(
     position(noPoints),
-    {start: generated, end: generated},
-    'should return an empty position without points'
+    undefined,
+    'should return nothing when without points'
   )
 
   assert.deepEqual(
     position(noPosition),
-    {start: generated, end: generated},
-    'should return an empty position without position'
+    undefined,
+    'should return nothing when without position'
   )
 
   assert.deepEqual(
     position(),
-    {start: generated, end: generated},
-    'should return an empty position without node'
+    undefined,
+    'should return nothing when without node'
   )
 })
 
@@ -66,28 +88,49 @@ test('pointStart', () => {
     'should get a side'
   )
 
+  assert.deepEqual(
+    pointStart({
+      type: 'x',
+      position: {
+        start: {line: 0, column: 0, offset: -1},
+        end: {line: 0, column: 0, offset: -1}
+      }
+    }),
+    undefined,
+    'should not get too low values'
+  )
+
+  assert.deepEqual(
+    pointStart({
+      type: 'x',
+      position: {start: {line: 1, column: 1}, end: {line: 1, column: 2}}
+    }),
+    {line: 1, column: 1, offset: undefined},
+    'should support points w/o `offset`'
+  )
+
   assert.deepEqual(
     pointStart(noFields),
-    generated,
-    'should return an empty point without fields'
+    undefined,
+    'should return nothing when without fields'
   )
 
   assert.deepEqual(
     pointStart(noPoints),
-    generated,
-    'should return an empty point without points'
+    undefined,
+    'should return nothing when without points'
   )
 
   assert.deepEqual(
     pointStart(noPosition),
-    generated,
-    'should return an empty point without position'
+    undefined,
+    'should return nothing when without position'
   )
 
   assert.deepEqual(
     pointStart(),
-    generated,
-    'should return an empty point without node'
+    undefined,
+    'should return nothing when without node'
   )
 })
 
@@ -98,27 +141,48 @@ test('pointEnd', () => {
     'should get a side'
   )
 
+  assert.deepEqual(
+    pointEnd({
+      type: 'x',
+      position: {
+        start: {line: 0, column: 0, offset: -1},
+        end: {line: 0, column: 0, offset: -1}
+      }
+    }),
+    undefined,
+    'should not get too low values'
+  )
+
+  assert.deepEqual(
+    pointEnd({
+      type: 'x',
+      position: {start: {line: 1, column: 1}, end: {line: 1, column: 2}}
+    }),
+    {line: 1, column: 2, offset: undefined},
+    'should support points w/o `offset`'
+  )
+
   assert.deepEqual(
     pointEnd(noFields),
-    generated,
-    'should return an empty point without fields'
+    undefined,
+    'should return nothing when without fields'
   )
 
   assert.deepEqual(
     pointEnd(noPoints),
-    generated,
-    'should return an empty point without points'
+    undefined,
+    'should return nothing when without points'
   )
 
   assert.deepEqual(
     pointEnd(noPosition),
-    generated,
-    'should return an empty point without position'
+    undefined,
+    'should return nothing when without position'
   )
 
   assert.deepEqual(
     pointEnd(),
-    generated,
-    'should return an empty point without node'
+    undefined,
+    'should return nothing when without node'
   )
 })