fix(ibm-operationid-naming-convention): make full naming check optional

docs/ibm-cloud-rules.md

@@ -4389,7 +4389,26 @@ For example, <code>create_thing</code> would be preferred over <code>manufacture
 when deciding on an operationId for the <code>POST /v1/things</code> operation.
 Likewise, for the <code>GET /v1/things/{thing_id}</code> operation, we might prefer
 <code>get_thing</code> over <code>retrieve_thing</code> for the operationId.
-<p>This rule will analyze the operations, looking for operationId values that are not using the recommended verbs.
+<p>This rule will analyze the operations, looking for operationId values that are not using the recommended verbs. Furthermore it can also validate the complete name of the operation id by comparing it to the path segments.
+</td>
+</tr>
+<tr>
+<td valign=top><b>Configuration:</b></td>
+<td>This rule can be configured to validate the complete name of the operation id, or only the verb it begins with.
+<p>The default configuration object provided in the rule definition is:
+<pre>
+{
+  strict: true
+}
+</pre>
+<p>To switch off the complete name validation and only validate the verbs the operation ids begin with, you'll need to
+<a href="#replace-a-rule-from-ibm-cloudopenapi-ruleset">replace this rule with a new rule within your
+custom ruleset</a> and modify the configuration such that the value of the <code>strict</code> field to <code>false</code>
+<pre>
+{
+  strict: false
+}
+</pre>
 </td>
 </tr>
 <tr>
@@ -4410,6 +4429,10 @@ paths:
       operationId: manufacture_thing
       description: Create a new Thing instance.
       summary: Create a Thing
+    get:
+      operationId: list_thing
+      description: List all Thing instances.
+      summary: List Thing
   '/v1/things/{thing_id}':
     get:
       operationId: retrieve_thing
@@ -4428,6 +4451,10 @@ paths:
       operationId: create_thing
       description: Create a new Thing instance.
       summary: Create a Thing
+    get:
+      operationId: list_things
+      description: List all Thing instances.
+      summery: List Things
   '/v1/things/{thing_id}':
     get:
       operationId: get_thing

packages/ruleset/src/functions/operationid-naming-convention.js

@@ -7,11 +7,11 @@ const { each, merge, pickBy, reduce } = require('lodash');
 const { operationMethods } = require('../utils');
 const inflected = require('inflected');
 
-module.exports = function (rootDocument) {
-  return operationIdNamingConvention(rootDocument);
+module.exports = function (rootDocument, options) {
+  return operationIdNamingConvention(rootDocument, options.strict);
 };
 
-function operationIdNamingConvention(resolvedSpec) {
+function operationIdNamingConvention(resolvedSpec, fullNamingCheck) {
   const operations = reduce(
     resolvedSpec.paths,
     (arr, path, pathKey) => {
@@ -59,23 +59,31 @@ function operationIdNamingConvention(resolvedSpec) {
             p.startsWith(op.pathKey + '/{')
           );
 
-      const { checkPassed, correctIds, operationId } =
+      const { checkPassed, correctIds, operationId, verbs } =
         operationIdPassedConventionCheck(
           isResourceOriented,
           op['opKey'],
           op.operationId,
           pathEndsWithParam,
           numParamRefs,
-          op.pathKey
+          op.pathKey,
+          fullNamingCheck
         );
 
       if (checkPassed === false) {
-        errors.push({
-          message: `operationIds should follow naming convention: operationId should be ${correctIds.join(
-            ' or '
-          )} but it's ${operationId} instead`,
-          path: [...op.path, 'operationId'],
-        });
+        if (fullNamingCheck) {
+          errors.push({
+            message: `operationIds should follow naming convention: operationId should be ${correctIds.join(
+              ' or '
+            )} but it's ${operationId} instead`,
+            path: [...op.path, 'operationId'],
+          });
+        } else {
+          errors.push({
+            message: `operationIds should follow naming convention: operationId verb should be ${verbs.join(' or ')}`,
+            path: [...op.path, 'operationId'],
+          });
+        }
       }
     }
   });
@@ -139,7 +147,8 @@ function operationIdPassedConventionCheck(
   operationId,
   pathEndsWithParam,
   numParamRefs,
-  fullPath
+  fullPath,
+  fullNamingCheck
 ) {
   // Useful for debugging.
   // console.log(`Debug: ${httpMethod} ${isResourceOriented} ${pathEndsWithParam} ${numParamRefs}  ${operationId}`);
@@ -202,27 +211,36 @@ function operationIdPassedConventionCheck(
   // that the operationId starts with that verb
   // and that the rest of the operation id matches
   // the path according to the naming conventions
-  const convertedPath = fullPath
-    .replace(/^\/+/, '')
-    .split('/')
-    .filter(part => !part.startsWith('{') && !part.endsWith('}'))
-    .filter(part => !/^v\d+$/.test(part));
-
-  const isPlural = pluralVerbs.some(verb => verbs.includes(verb));
-
-  // Singularize the words in the path according to the naming conventions.
-  for (let i = 0; i < convertedPath.length; i++) {
-    if (i !== convertedPath.length - 1 || !isPlural || pathEndsWithParam)
-      convertedPath[i] = inflected.singularize(convertedPath[i]);
-  }
+  if (fullNamingCheck) {
+    const convertedPath = fullPath
+      .replace(/^\/+/, '')
+      .split('/')
+      .filter(part => !part.startsWith('{') && !part.endsWith('}'))
+      .filter(part => !/^v\d+$/.test(part));
+
+    const isPlural = pluralVerbs.some(verb => verbs.includes(verb));
+
+    // Singularize the words in the path according to the naming conventions.
+    for (let i = 0; i < convertedPath.length; i++) {
+      if (i !== convertedPath.length - 1 || !isPlural || pathEndsWithParam)
+        convertedPath[i] = inflected.singularize(convertedPath[i]);
+    }
 
-  const correctIds = [];
+    const correctIds = [];
 
-  for (let i = 0; i < verbs.length; i++) {
-    const correctId = verbs[i] + '_' + convertedPath.join('_');
-    if (correctId === operationId) return { checkPassed: true };
-    else correctIds.push(correctId);
-  }
+    for (let i = 0; i < verbs.length; i++) {
+      const correctId = verbs[i] + '_' + convertedPath.join('_');
+      if (correctId === operationId) return { checkPassed: true };
+      else correctIds.push(correctId);
+    }
 
-  return { checkPassed: false, correctIds, operationId };
+    return { checkPassed: false, correctIds, operationId };
+  } else {
+    if (verbs.length > 0) {
+      const checkPassed = verbs
+        .map(verb => operationId.startsWith(verb))
+        .some(v => v);
+      return { checkPassed, verbs: verbs };
+    }
+  }
 }

packages/ruleset/src/rules/operationid-naming-convention.js

@@ -15,5 +15,8 @@ module.exports = {
   resolved: true,
   then: {
     function: operationIdNamingConvention,
+    functionOptions: {
+      strict: true,
+    },
   },
 };

packages/ruleset/test/rules/operationid-naming-convention.test.js

@@ -14,8 +14,10 @@ const {
 const rule = operationIdNamingConvention;
 const ruleId = 'ibm-operation-id-naming-convention';
 const expectedSeverity = severityCodes.warning;
-const expectedMsgPrefix =
+const expectedStrictMsgPrefix =
   /^operationIds should follow naming convention: operationId should be.*$/;
+const expectedNotStrictMsgPrefix =
+  /^operationIds should follow naming convention: operationId verb should be.*$/;
 
 describe(`Spectral rule: ${ruleId}`, () => {
   describe('Should not yield errors', () => {
@@ -204,7 +206,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*list_drinks*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe('paths./v1/drinks.get.operationId');
@@ -220,7 +222,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*get_drink*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe(
@@ -241,7 +243,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*get_drink_glass or check_drink_glass*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe(
@@ -257,7 +259,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*create_drink*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe('paths./v1/drinks.post.operationId');
@@ -274,7 +276,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*create_drink*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe(
@@ -293,7 +295,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*update_drink*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe(
@@ -312,7 +314,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*replace_drink*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe(
@@ -331,7 +333,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*replace_drinks*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe('paths./v1/drinks.put.operationId');
@@ -348,7 +350,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*replace_drink*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe(
@@ -369,7 +371,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(
         /^.*replace_drink_glasses or set_drink_glasses*/
       );
@@ -392,7 +394,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*replace_drink_glass or add_drink_glass*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe(
@@ -411,7 +413,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*delete_drink*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe(
@@ -432,7 +434,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(
         /^.*delete_drink_glasses or unset_drink_glasses*/
       );
@@ -455,12 +457,35 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(1);
       const r = results[0];
       expect(r.code).toBe(ruleId);
-      expect(r.message).toMatch(expectedMsgPrefix);
+      expect(r.message).toMatch(expectedStrictMsgPrefix);
       expect(r.message).toMatch(/^.*delete_drink_glass or remove_drink_glass*/);
       expect(r.severity).toBe(expectedSeverity);
       expect(r.path.join('.')).toBe(
         'paths./v1/drinks/{drink_id}/glasses/{glass_id}.delete.operationId'
       );
     });
+
+    it('path has multiple path params, delete, not strict', async () => {
+      const testDocument = makeCopy(rootDocument);
+
+      testDocument.paths['/v1/drinks/{drink_id}/glasses/{glass_id}'] = {
+        delete: {
+          operationId: 'smash_drink_glass',
+        },
+      };
+
+      rule.then.functionOptions.strict = false;
+
+      const results = await testRule(ruleId, rule, testDocument);
+      expect(results).toHaveLength(1);
+      const r = results[0];
+      expect(r.code).toBe(ruleId);
+      expect(r.message).toMatch(expectedNotStrictMsgPrefix);
+      expect(r.message).toMatch(/^.*delete or remove*/);
+      expect(r.severity).toBe(expectedSeverity);
+      expect(r.path.join('.')).toBe(
+        'paths./v1/drinks/{drink_id}/glasses/{glass_id}.delete.operationId'
+      );
+    });
   });
 });