From 70fc34dabec3a6965062759d4dd0d8a4e97e7103 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= <jakub@stoplight.io>
Date: Sat, 28 Sep 2019 13:49:39 +0200
Subject: [PATCH] feat(rule): remove 'when'

BREAKING CHANGE: 'when' is gone. You can write a jsonpath extension instead
---
 src/__tests__/linter.test.ts | 147 +++++------------------------------
 src/linter.ts                |  85 +-------------------
 src/meta/rule.schema.json    |  14 ----
 src/types/rule.ts            |   9 ---
 4 files changed, 20 insertions(+), 235 deletions(-)

diff --git a/src/__tests__/linter.test.ts b/src/__tests__/linter.test.ts
index 36b6c651e..4d390c36b 100644
--- a/src/__tests__/linter.test.ts
+++ b/src/__tests__/linter.test.ts
@@ -57,7 +57,7 @@ describe('linter', () => {
     expect(result).toHaveLength(0);
   });
 
-  test('should return all properties', async () => {
+  test('should return all properties matching 4xx response code', async () => {
     const message = '4xx responses require a description';
 
     spectral.setFunctions({
@@ -76,11 +76,7 @@ describe('linter', () => {
 
     spectral.setRules({
       rule1: {
-        given: '$.responses[*]',
-        when: {
-          field: '@key',
-          pattern: '^4.*',
-        },
+        given: '$.responses.[?(@property >= 400 && @property < 500)]',
         then: {
           field: 'description',
           function: 'func1',
@@ -99,22 +95,24 @@ describe('linter', () => {
       },
     });
 
-    expect(result[0]).toMatchObject({
-      code: 'rule1',
-      message,
-      severity: DiagnosticSeverity.Warning,
-      path: ['responses', '404'],
-      range: {
-        end: {
-          line: 6,
-          character: 25,
-        },
-        start: {
-          character: 10,
-          line: 5,
+    expect(result).toEqual([
+      {
+        code: 'rule1',
+        message,
+        severity: DiagnosticSeverity.Warning,
+        path: ['responses', '404'],
+        range: {
+          end: {
+            line: 6,
+            character: 25,
+          },
+          start: {
+            character: 10,
+            line: 5,
+          },
         },
       },
-    });
+    ]);
   });
 
   test('should support rule overriding severity', async () => {
@@ -689,115 +687,6 @@ responses:: !!foo
     });
   });
 
-  describe('functional tests for the when statement', () => {
-    let fakeLintingFunction: any;
-
-    beforeEach(() => {
-      fakeLintingFunction = jest.fn();
-      spectral.setFunctions({
-        [fnName]: fakeLintingFunction,
-      });
-      spectral.setRules(rules);
-    });
-
-    describe('given no when', () => {
-      test('should simply lint anything it matches in the given', async () => {
-        await spectral.run(target);
-
-        expect(fakeLintingFunction).toHaveBeenCalledTimes(1);
-        expect(fakeLintingFunction.mock.calls[0][0]).toEqual(target.responses);
-      });
-    });
-
-    describe('given when with no pattern and regular field', () => {
-      test('should call linter if when field exists', async () => {
-        spectral.mergeRules({
-          example: {
-            when: {
-              field: '200.description',
-            },
-          },
-        });
-        await spectral.run(target);
-
-        expect(fakeLintingFunction).toHaveBeenCalledTimes(1);
-        expect(fakeLintingFunction.mock.calls[0][0]).toEqual({
-          '200': { description: 'a' },
-          '201': { description: 'b' },
-          '300': { description: 'c' },
-        });
-      });
-
-      test('should not call linter if when field not exist', async () => {
-        spectral.mergeRules({
-          example: {
-            when: {
-              field: '302.description',
-            },
-          },
-        });
-        await spectral.run(target);
-
-        expect(fakeLintingFunction).toHaveBeenCalledTimes(0);
-      });
-    });
-
-    describe('given "when" with a pattern and regular field', () => {
-      test('should NOT lint if pattern does not match', async () => {
-        spectral.mergeRules({
-          example: {
-            when: {
-              field: '200.description',
-              pattern: 'X',
-            },
-          },
-        });
-        await spectral.run(target);
-
-        expect(fakeLintingFunction).toHaveBeenCalledTimes(0);
-      });
-
-      test('should lint if pattern does match', async () => {
-        spectral.mergeRules({
-          example: {
-            when: {
-              field: '200.description',
-              pattern: 'a',
-            },
-          },
-        });
-        await spectral.run(target);
-
-        expect(fakeLintingFunction).toHaveBeenCalledTimes(1);
-        expect(fakeLintingFunction.mock.calls[0][0]).toEqual({
-          '200': { description: 'a' },
-          '201': { description: 'b' },
-          '300': { description: 'c' },
-        });
-      });
-    });
-
-    describe('given "when" with a pattern and @key field', () => {
-      test('should lint ONLY part of object that matches pattern', async () => {
-        spectral.mergeRules({
-          example: {
-            given: '$.responses[*]',
-            when: {
-              field: '@key',
-              pattern: '2..',
-            },
-          },
-        });
-        await spectral.run(target);
-
-        expect(fakeLintingFunction).toHaveBeenCalledTimes(2);
-        expect(fakeLintingFunction.mock.calls[0][0]).toEqual({
-          description: 'a',
-        });
-      });
-    });
-  });
-
   describe('functional tests for the then statement', () => {
     let fakeLintingFunction: any;
     let fakeLintingFunction2: any;
diff --git a/src/linter.ts b/src/linter.ts
index de25f634b..bf3e13aef 100644
--- a/src/linter.ts
+++ b/src/linter.ts
@@ -19,14 +19,7 @@ export const lintNode = (
   resolved: Resolved,
 ): IRuleResult[] => {
   const givenPath = node.path[0] === '$' ? node.path.slice(1) : node.path;
-  const conditioning = whatShouldBeLinted(givenPath, node.value, rule);
-
-  // If the 'when' condition is not satisfied, simply don't run the linter
-  if (!conditioning.lint) {
-    return [];
-  }
-
-  const targetValue = conditioning.value;
+  const targetValue = node.value;
 
   const targets: any[] = [];
   if (then && then.field) {
@@ -137,83 +130,9 @@ export const lintNode = (
   return results;
 };
 
-// TODO(SO-23): unit test idividually
-export const whatShouldBeLinted = (
-  path: JsonPath,
-  originalValue: any,
-  rule: IRunRule,
-): { lint: boolean; value: any } => {
-  const leaf = path[path.length - 1];
-
-  const when = rule.when;
-  if (!when) {
-    return {
-      lint: true,
-      value: originalValue,
-    };
-  }
-
-  const pattern = when.pattern;
-  const field = when.field;
-
-  // TODO: what if someone's field is called '@key'? should we use @@key?
-  const isKey = field === '@key';
-
-  if (!pattern) {
-    // isKey doesn't make sense without pattern
-    if (isKey) {
-      return {
-        lint: false,
-        value: originalValue,
-      };
-    }
-
-    return {
-      lint: has(originalValue, field),
-      value: originalValue,
-    };
-  }
-
-  if (isKey && pattern) {
-    return keyAndOptionalPattern(leaf, pattern, originalValue);
-  }
-
-  const fieldValue = String(get(originalValue, when.field));
-
-  return {
-    lint: fieldValue.match(pattern) !== null,
-    value: originalValue,
-  };
-};
-
-function keyAndOptionalPattern(key: string | number, pattern: string, value: any) {
-  /** arrays, look at the keys on the array object. note, this number check on id prop is not foolproof... */
-  if (typeof key === 'number' && typeof value === 'object') {
-    for (const k of Object.keys(value)) {
-      if (String(k).match(pattern)) {
-        return {
-          lint: true,
-          value,
-        };
-      }
-    }
-  } else if (String(key).match(pattern)) {
-    // objects
-    return {
-      lint: true,
-      value,
-    };
-  }
-
-  return {
-    lint: false,
-    value,
-  };
-}
-
 // todo: revisit -> https://github.com/stoplightio/spectral/issues/608
 function getClosestJsonPath(data: unknown, path: JsonPath) {
-  if (data === null || typeof data !== 'object') return [];
+  if (!isObject(data)) return [];
 
   while (path.length > 0 && !has(data, path)) {
     path.pop();
diff --git a/src/meta/rule.schema.json b/src/meta/rule.schema.json
index 5963e09d3..d4a3043f3 100644
--- a/src/meta/rule.schema.json
+++ b/src/meta/rule.schema.json
@@ -109,20 +109,6 @@
                         "validation"
                     ],
                     "type": "string"
-                },
-                "when": {
-                    "properties": {
-                        "field": {
-                            "type": "string"
-                        },
-                        "pattern": {
-                            "type": "string"
-                        }
-                    },
-                    "required": [
-                        "field"
-                    ],
-                    "type": "object"
                 }
             },
             "required": [
diff --git a/src/types/rule.ts b/src/types/rule.ts
index 6064de809..9e1bb2b89 100644
--- a/src/types/rule.ts
+++ b/src/types/rule.ts
@@ -30,15 +30,6 @@ export interface IRule<T = string, O = any> {
   // If undefined or true, resolved data will be supplied
   resolved?: boolean;
 
-  when?: {
-    // the `path.to.prop` to field, or special `@key` value to target keys for matched `given` object
-    // EXAMPLE: if the target object is an oas object and given = `$..responses[*]`, then `@key` would be the response code (200, 400, etc)
-    field: string;
-
-    // a regex pattern
-    pattern?: string;
-  };
-
   then: IThen<T, O> | Array<IThen<T, O>>;
 }