fix: schema-path handles falsy values and JSON path expression as fie…

…ld (#917)

* fix: schema-path validates falsy value and accepts JSON path expressions as field

* chore: useless comment

* test: rename test case

* test: rename once again

src/functions/__tests__/schema-path.test.ts

@@ -7,17 +7,21 @@ function runSchemaPath(target: any, field: string, schemaPathStr: string) {
   } as any);
 }
 
-describe('schema', () => {
+describe('schema-path', () => {
   // Check the example field matches the contents of schema
   const fieldToCheck = 'example';
   const path = '$.schema';
 
-  test('will pass when example is valid', () => {
+  test.each([
+    ['turtle', 'string'],
+    [0, 'number'],
+    [null, 'null'],
+  ])('will pass when %s example is valid', (example, type) => {
     const target = {
       schema: {
-        type: 'string',
+        type,
       },
-      example: 'turtle',
+      example,
     };
 
     expect(runSchemaPath(target, fieldToCheck, path)).toHaveLength(0);
@@ -65,6 +69,56 @@ describe('schema', () => {
     ]);
   });
 
+  test('will error with invalid falsy input', () => {
+    const target = {
+      schema: {
+        type: 'string',
+      },
+      example: null,
+    };
+    expect(runSchemaPath(target, fieldToCheck, path)).toEqual([
+      {
+        path: ['example'],
+        message: '{{property|gravis|append-property|optional-typeof}}type should be string',
+      },
+    ]);
+  });
+
+  test('will error with invalid array-ish input', () => {
+    const target = {
+      schema: {
+        type: 'object',
+        properties: {
+          id: {
+            type: 'string',
+          },
+        },
+      },
+      examples: {
+        'application/json': {
+          id: 1,
+          name: 'get food',
+          completed: false,
+        },
+        'application/yaml': {
+          id: 1,
+          name: 'get food',
+          completed: false,
+        },
+      },
+    };
+    expect(runSchemaPath(target, '$.examples.*', path)).toEqual([
+      {
+        message: '{{property|gravis|append-property|optional-typeof}}type should be string',
+        path: ['examples', 'application/json', 'id'],
+      },
+      {
+        message: '{{property|gravis|append-property|optional-typeof}}type should be string',
+        path: ['examples', 'application/yaml', 'id'],
+      },
+    ]);
+  });
+
   test('will error formats', () => {
     const target = {
       schema: {
@@ -92,7 +146,12 @@ describe('schema', () => {
         },
         notNonsense: 'turtle',
       };
-      expect(runSchemaPath(target, invalidFieldToCheck, path)).toHaveLength(0);
+      expect(runSchemaPath(target, invalidFieldToCheck, path)).toEqual([
+        {
+          message: '{{property|double-quotes|append-property}}does not exist',
+          path: ['nonsense'],
+        },
+      ]);
     });
   });
 });

src/functions/schema-path.ts

@@ -7,10 +7,11 @@
  * The primary use case for this was validating OpenAPI examples
  * against their schema, but this could be used for other things.
  */
-import { IFunction, IRule, RuleFunction } from '../types';
-import { schema } from './schema';
+import { JSONPath } from 'jsonpath-plus';
 
-const { JSONPath } = require('jsonpath-plus');
+import { IFunction, IFunctionResult, IRule, RuleFunction } from '../types';
+import { getLintTargets } from '../utils';
+import { schema } from './schema';
 
 export interface ISchemaPathOptions {
   schemaPath: string;
@@ -21,29 +22,29 @@ export interface ISchemaPathOptions {
 export type SchemaPathRule = IRule<RuleFunction.SCHEMAPATH, ISchemaPathOptions>;
 
 export const schemaPath: IFunction<ISchemaPathOptions> = (targetVal, opts, paths, otherValues) => {
-  if (!targetVal || typeof targetVal !== 'object') return [];
-
-  const { original: object } = otherValues;
-
   // The subsection of the targetVal which contains the good bit
-  const relevantObject = opts.field ? object[opts.field] : object;
-  if (!relevantObject) return [];
-  const { target, given } = paths;
+  const relevantItems = getLintTargets(targetVal, opts.field);
 
   // The subsection of the targetValue which contains the schema for us to validate the good bit against
-  let schemaObject;
-  try {
-    schemaObject = JSONPath({ path: opts.schemaPath, json: object })[0];
-  } catch (error) {
-    console.error(error);
-  }
-
-  if (opts.field) {
-    given.push(opts.field);
-    if (target) {
-      target.push(opts.field);
+  const schemaObject = JSONPath({ path: opts.schemaPath, json: targetVal })[0];
+
+  const results: IFunctionResult[] = [];
+
+  for (const relevantItem of relevantItems) {
+    const result = schema(
+      relevantItem.value,
+      { schema: schemaObject },
+      {
+        given: paths.given,
+        target: [...(paths.target || paths.given), ...relevantItem.path],
+      },
+      otherValues,
+    );
+
+    if (result !== void 0) {
+      results.push(...result);
     }
   }
 
-  return schema(relevantObject, { schema: schemaObject }, paths, otherValues);
+  return results;
 };

src/linter.ts

@@ -6,9 +6,7 @@ import { DocumentInventory } from './documentInventory';
 import { IMessageVars, message } from './rulesets/message';
 import { getDiagnosticSeverity } from './rulesets/severity';
 import { IFunction, IGivenNode, IRuleResult, IRunRule, IThen } from './types';
-import { getClosestJsonPath, printPath, PrintStyle } from './utils';
-
-const { JSONPath } = require('jsonpath-plus');
+import { getClosestJsonPath, getLintTargets, printPath, PrintStyle } from './utils';
 
 // TODO(SO-23): unit test but mock whatShouldBeLinted
 export const lintNode = (
@@ -19,55 +17,7 @@ export const lintNode = (
   inventory: DocumentInventory,
 ): IRuleResult[] => {
   const givenPath = node.path[0] === '$' ? node.path.slice(1) : node.path;
-  const targetValue = node.value;
-
-  const targets: any[] = [];
-  if (then && then.field) {
-    if (then.field === '@key') {
-      for (const key of Object.keys(targetValue)) {
-        targets.push({
-          path: key,
-          value: key,
-        });
-      }
-    } else if (then.field[0] === '$') {
-      try {
-        JSONPath({
-          path: then.field,
-          json: targetValue,
-          resultType: 'all',
-          callback: (result: any) => {
-            targets.push({
-              path: JSONPath.toPathArray(result.path),
-              value: result.value,
-            });
-          },
-        });
-      } catch (e) {
-        console.error(e);
-      }
-    } else {
-      // lodash lookup
-      targets.push({
-        path: typeof then.field === 'string' ? then.field.split('.') : then.field,
-        value: get(targetValue, then.field),
-      });
-    }
-  } else {
-    targets.push({
-      path: [],
-      value: targetValue,
-    });
-  }
-
-  if (!targets.length) {
-    // must call then at least once, with no document
-    targets.push({
-      path: [],
-      value: undefined,
-    });
-  }
-
+  const targets = getLintTargets(node.value, then.field);
   const results: IRuleResult[] = [];
 
   for (const target of targets) {

src/utils/__tests__/getLintTargets.spec.ts

@@ -0,0 +1,151 @@
+import { getLintTargets } from '../getLintTargets';
+
+describe('getLintTargets', () => {
+  describe('when @key is given as field', () => {
+    it('given object, returns its keys', () => {
+      expect(getLintTargets({ a: null, b: true }, '@key')).toStrictEqual([
+        {
+          path: ['a'],
+          value: 'a',
+        },
+        {
+          path: ['b'],
+          value: 'b',
+        },
+      ]);
+    });
+
+    it('given array, returns its indicies', () => {
+      expect(getLintTargets(['foo', 'bar'], '@key')).toStrictEqual([
+        {
+          path: ['0'],
+          value: '0',
+        },
+        {
+          path: ['1'],
+          value: '1',
+        },
+      ]);
+    });
+
+    it('given primitive property, returns the whole input', () => {
+      expect(getLintTargets('abc', '0')).toStrictEqual([
+        {
+          path: [],
+          value: 'abc',
+        },
+      ]);
+    });
+  });
+
+  describe('when property path is given as field', () => {
+    it('given existing property, returns lint targets', () => {
+      expect(getLintTargets({ a: null }, 'a')).toStrictEqual([
+        {
+          path: ['a'],
+          value: null,
+        },
+      ]);
+
+      expect(getLintTargets({ foo: ['a'] }, 'foo[0]')).toStrictEqual([
+        {
+          path: ['foo', '0'],
+          value: 'a',
+        },
+      ]);
+
+      expect(getLintTargets(['foo'], '0')).toStrictEqual([
+        {
+          path: ['0'],
+          value: 'foo',
+        },
+      ]);
+
+      expect(getLintTargets({ a: void 0 }, 'a')).toStrictEqual([
+        {
+          path: ['a'],
+          value: void 0,
+        },
+      ]);
+    });
+
+    it('given non-existing property, returns lint target with undefined value', () => {
+      expect(getLintTargets({ a: null }, 'b')).toStrictEqual([
+        {
+          path: ['b'],
+          value: void 0,
+        },
+      ]);
+
+      expect(getLintTargets(['foo'], '1')).toStrictEqual([
+        {
+          path: ['1'],
+          value: void 0,
+        },
+      ]);
+    });
+
+    it('given primitive property, returns the whole input', () => {
+      expect(getLintTargets('abc', '0')).toStrictEqual([
+        {
+          path: [],
+          value: 'abc',
+        },
+      ]);
+    });
+  });
+
+  describe('when JSON Path expression is given as field', () => {
+    it('given existing property, returns lint targets', () => {
+      expect(getLintTargets({ a: null }, '$')).toStrictEqual([
+        {
+          path: [],
+          value: {
+            a: null,
+          },
+        },
+      ]);
+
+      expect(getLintTargets({ foo: ['a'] }, '$.foo.*')).toStrictEqual([
+        {
+          path: ['foo', '0'],
+          value: 'a',
+        },
+      ]);
+    });
+
+    it('given non-existing property, returns lint target with undefined value', () => {
+      expect(getLintTargets({ a: null }, '$.b')).toStrictEqual([
+        {
+          path: [],
+          value: void 0,
+        },
+      ]);
+
+      expect(getLintTargets(['foo'], '$..bar')).toStrictEqual([
+        {
+          path: [],
+          value: void 0,
+        },
+      ]);
+    });
+
+    it('given primitive property, returns the whole input', () => {
+      expect(getLintTargets('abc', '0')).toStrictEqual([
+        {
+          path: [],
+          value: 'abc',
+        },
+      ]);
+    });
+  });
+
+  it('given no field, returns the whole input', () => {
+    expect(getLintTargets({ a: true }, void 0)).toStrictEqual([
+      {
+        path: [],
+        value: { a: true },
+      },
+    ]);
+  });
+});