feat: add warning, non-204 success response should have response body (…

…#127)

- added logic to check for response bodies in non-204 success responses in the oas3 plugin
- updated the README.md to help users resolve potential issue with `npm run link`
- added no_response_body rule and set the default value to 'warning'

README.md

@@ -57,6 +57,8 @@ The `-g` flag installs the tool globally so that the validator can be run from a
 3. Install the dependencies using `npm install`
 4. Build the command line tool by running `npm run link`.
 
+_If you installed the validator using `npm install -g ibm-openapi-validator`, you will need to run `npm uninstall -g ibm-openapi-validator` before running `npm run link`._
+
 ### Platform specific binaries
 It is possible to build platform specific binaries for Linux, MacOS, and Windows that do not depend on having node.js installed.
 
@@ -202,6 +204,7 @@ The supported rules are described below:
 | no_request_body_content      | [Flag any operations with a `requestBody` that does not have a `content` field.][3] | oas3     |
 | no_request_body_name         | Flag any operations with a non-form `requestBody` that does not have a name set with `x-codegen-request-body-name`. | oas3|
 
+
 ##### pagination
 | Rule                        | Description                                                              | Spec   |
 | --------------------------- | ------------------------------------------------------------------------ | ------ |
@@ -237,6 +240,7 @@ The supported rules are described below:
 | inline_response_schema    | Flag any response object with a schema that doesn't reference a named model. | shared |
 | no_response_codes         | Flag any response object that has no valid response codes.   | oas3 |
 | no_success_response_codes | Flag any response object that has no success response codes. | oas3 |
+| no_response_body          | Flag any non-204 success responses without a response body.  | oas3 |
 
 ##### schemas
 | Rule                        | Description                                                                   | Spec     |
@@ -355,6 +359,7 @@ The default values for each rule are described below.
 | ------------------------- | ------- |
 | no_response_codes         | error   |
 | no_success_response_codes | warning |
+| no_response_body          | warning |
 
 
 ##### shared

src/.defaultsForValidator.js

@@ -94,7 +94,8 @@ const defaults = {
     },
     'responses': {
       'no_response_codes': 'error',
-      'no_success_response_codes': 'warning'
+      'no_success_response_codes': 'warning',
+      'no_response_body': 'warning'
     }
   }
 };

src/plugins/validation/oas3/semantic-validators/responses.js

@@ -9,6 +9,9 @@
 // A 204 response MUST not define a response body
 // https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.2.5
 
+// Assertation 4:
+// A non-204 success response MUST define a response body
+
 const { walk } = require('../../../utils');
 
 module.exports.validate = function({ jsSpec }, config) {
@@ -57,6 +60,23 @@ module.exports.validate = function({ jsSpec }, config) {
               message
             });
           }
+        } else {
+          const checkStatus = config.no_response_body;
+          // if response body rule is on, loops through success codes and issues warning (by default)
+          // for non-204 success responses without a response body
+          if (checkStatus !== 'off') {
+            for (const successCode of successCodes) {
+              if (successCode != '204' && !obj[successCode].content) {
+                result[checkStatus].push({
+                  path: path.concat([successCode]),
+                  message:
+                    `A ` +
+                    successCode +
+                    ` response should include a response body. Use 204 for responses without content.`
+                });
+              }
+            }
+          }
         }
       }
     }

test/cli-validator/mockFiles/oas3/clean.yml

@@ -54,7 +54,7 @@ paths:
       tags:
         - pets
       responses:
-        '201':
+        '204':
           description: Null response
         default:
           description: unexpected error

test/plugins/validation/oas3/responses.js

@@ -8,7 +8,8 @@ describe('validation plugin - semantic - responses - oas3', function() {
     const config = {
       responses: {
         no_response_codes: 'error',
-        no_success_response_codes: 'warning'
+        no_success_response_codes: 'warning',
+        no_response_body: 'warning'
       }
     };
 
@@ -41,7 +42,8 @@ describe('validation plugin - semantic - responses - oas3', function() {
     const config = {
       responses: {
         no_response_codes: 'error',
-        no_success_response_codes: 'warning'
+        no_success_response_codes: 'warning',
+        no_response_body: 'warning'
       }
     };
 
@@ -74,7 +76,8 @@ describe('validation plugin - semantic - responses - oas3', function() {
     const config = {
       responses: {
         no_response_codes: 'error',
-        no_success_response_codes: 'warning'
+        no_success_response_codes: 'warning',
+        no_response_body: 'warning'
       }
     };
 
@@ -85,8 +88,8 @@ describe('validation plugin - semantic - responses - oas3', function() {
             summary: 'this is a summary',
             operationId: 'operationId',
             responses: {
-              '200': {
-                description: 'successful operation call'
+              '204': {
+                description: 'successful operation call with no response body'
               }
             }
           }
@@ -99,11 +102,124 @@ describe('validation plugin - semantic - responses - oas3', function() {
     expect(res.warnings.length).toEqual(0);
   });
 
+  it('should complain when a non-204 success does not have response body', function() {
+    const config = {
+      responses: {
+        no_response_codes: 'error',
+        no_success_response_codes: 'warning',
+        no_response_body: 'warning'
+      }
+    };
+
+    const spec = {
+      paths: {
+        '/example': {
+          get: {
+            summary: 'this is a summary',
+            operationId: 'operationId',
+            responses: {
+              '200': {
+                description: 'successful operation call with no response body'
+              }
+            }
+          }
+        }
+      }
+    };
+
+    const res = validate({ jsSpec: spec }, config);
+    expect(res.warnings.length).toEqual(1);
+    expect(res.warnings[0].path).toEqual([
+      'paths',
+      '/example',
+      'get',
+      'responses',
+      '200'
+    ]);
+    expect(res.warnings[0].message).toEqual(
+      `A 200 response should include a response body. Use 204 for responses without content.`
+    );
+  });
+
+  it('should issue multiple warnings when multiple non-204 successes do not have response bodies', function() {
+    const config = {
+      responses: {
+        no_response_codes: 'error',
+        no_success_response_codes: 'warning',
+        no_response_body: 'warning'
+      }
+    };
+
+    const spec = {
+      paths: {
+        '/example1': {
+          get: {
+            summary: 'this is a summary',
+            operationId: 'operationId_1',
+            responses: {
+              '200': {
+                description: 'first successful response with no response body'
+              },
+              '202': {
+                description: 'second successful response with no response body'
+              }
+            }
+          }
+        },
+        '/example2': {
+          get: {
+            summary: 'this is a summary',
+            operationId: 'operationId_2',
+            responses: {
+              '203': {
+                description: 'third successful response with no response body'
+              }
+            }
+          }
+        }
+      }
+    };
+
+    const res = validate({ jsSpec: spec }, config);
+    expect(res.warnings.length).toEqual(3);
+    expect(res.warnings[0].path).toEqual([
+      'paths',
+      '/example1',
+      'get',
+      'responses',
+      '200'
+    ]);
+    expect(res.warnings[0].message).toEqual(
+      `A 200 response should include a response body. Use 204 for responses without content.`
+    );
+    expect(res.warnings[1].path).toEqual([
+      'paths',
+      '/example1',
+      'get',
+      'responses',
+      '202'
+    ]);
+    expect(res.warnings[1].message).toEqual(
+      `A 202 response should include a response body. Use 204 for responses without content.`
+    );
+    expect(res.warnings[2].path).toEqual([
+      'paths',
+      '/example2',
+      'get',
+      'responses',
+      '203'
+    ]);
+    expect(res.warnings[2].message).toEqual(
+      `A 203 response should include a response body. Use 204 for responses without content.`
+    );
+  });
+
   it('should complain about having only error responses', function() {
     const config = {
       responses: {
         no_response_codes: 'error',
-        no_success_response_codes: 'warning'
+        no_success_response_codes: 'warning',
+        no_response_body: 'warning'
       }
     };
 
@@ -150,7 +266,8 @@ describe('validation plugin - semantic - responses - oas3', function() {
     const config = {
       responses: {
         no_response_codes: 'error',
-        no_success_response_codes: 'warning'
+        no_success_response_codes: 'warning',
+        no_response_body: 'warning'
       }
     };