Changes "void_value" into "no_body" for req/responses with no body (#444)

Author: swallez

compiler/model/build-model.ts

@@ -179,6 +179,8 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int
     )
   }
 
+  let bodyWasSet = false
+
   // Request and Response definitions needs to be handled
   // differently from normal classes
   if (name === 'Request' || name === 'Response') {
@@ -188,7 +190,8 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int
         kind: 'request',
         name: { name, namespace: getNameSpace(declaration) },
         path: new Array<model.Property>(),
-        query: new Array<model.Property>()
+        query: new Array<model.Property>(),
+        body: { kind: 'no_body' }
       }
 
       const response: model.TypeName = {
@@ -210,20 +213,27 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int
           type.path = property.properties
         } else if (property.name === 'query_parameters') {
           type.query = property.properties
-        } else {
-          // the body can either by a value (eg Array<string> or an object with properties)
+        } else if (property.name === 'body') {
+          bodyWasSet = true
+          // the body can either be a value (eg Array<string> or an object with properties)
           if (property.valueOf != null) {
-            type.body = { kind: 'value', value: property.valueOf }
+            if (property.valueOf.kind === 'instance_of' && property.valueOf.type.name === 'Void') {
+              type.body = { kind: 'no_body' }
+            } else {
+              type.body = { kind: 'value', value: property.valueOf }
+            }
           } else if (property.properties.length > 0) {
             type.body = { kind: 'properties', properties: property.properties }
           }
+        } else {
+          assert(member, false, 'Unknown request property ' + property.name)
         }
       }
     } else {
       type = {
         kind: 'response',
         name: { name, namespace: getNameSpace(declaration) },
-        body: undefined
+        body: { kind: 'no_body' }
       }
 
       for (const member of declaration.getMembers()) {
@@ -235,9 +245,14 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int
         )
         const property = visitRequestOrResponseProperty(member)
         if (property.name === 'body') {
+          bodyWasSet = true
           // the body can either by a value (eg Array<string> or an object with properties)
           if (property.valueOf != null) {
-            type.body = { kind: 'value', value: property.valueOf }
+            if (property.valueOf.kind === 'instance_of' && property.valueOf.type.name === 'Void') {
+              type.body = { kind: 'no_body' }
+            } else {
+              type.body = { kind: 'value', value: property.valueOf }
+            }
           } else if (property.properties.length > 0) {
             type.body = { kind: 'properties', properties: property.properties }
           }
@@ -279,6 +294,15 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int
       })
     }
 
+    // If the body wasn't set and we have a parent class, then it's a property body with no additional properties
+    if (!bodyWasSet && type.inherits != null) {
+      const parent = type.inherits.type
+      // RequestBase is special as it's a "marker" base class that doesn't imply a property body type. We should get rid of it.
+      if (!(parent.name === 'RequestBase' && parent.namespace === '_types')) {
+        type.body = { kind: 'properties', properties: new Array<model.Property>() }
+      }
+    }
+
     return type
 
   // Every other class or interface will be handled here

compiler/model/metamodel.ts

@@ -49,7 +49,7 @@ export class TypeName {
 /**
  * Type of a value. Used both for property types and nested type definitions.
  */
-export type ValueOf = InstanceOf | ArrayOf | UnionOf | DictionaryOf | UserDefinedValue | LiteralValue | VoidValue
+export type ValueOf = InstanceOf | ArrayOf | UnionOf | DictionaryOf | UserDefinedValue | LiteralValue
 
 /**
  * A single value
@@ -116,13 +116,6 @@ export class LiteralValue {
   value: string | number | boolean
 }
 
-/**
- * The absence of any type. This is commonly used in APIs that returns an empty body.
- */
-export class VoidValue {
-  kind: 'void_value'
-}
-
 /**
  * An interface or request interface property.
  */
@@ -225,6 +218,7 @@ export class Request extends BaseType {
   // Note: does not extend Interface as properties are split across path, query and body
   kind: 'request'
   generics?: TypeName[]
+  /** The parent defines additional body properties that are added to the body, that has to be a PropertyBody */
   inherits?: Inherits
   implements?: Inherits[]
   /** URL path properties */
@@ -239,10 +233,11 @@ export class Request extends BaseType {
   // We can also pull path parameter descriptions on body properties they replace
 
   /**
-   * Body type. In most cases this is just a list of properties, except for a few specific cases like bulk requests
-   * (an array of bulk operations) or create requests (a user provided document type).
+   * Body type. Most often a list of properties (that can extend those of the inherited class, see above), except for a
+   * few specific cases that use other types such as bulk (array) or create (generic parameter). Or NoBody for requests
+   * that don't have a body.
    */
-  body?: ValueBody | PropertiesBody
+  body: Body
   behaviors?: Inherits[]
   attachedBehaviors?: string[]
 }
@@ -255,11 +250,13 @@ export class Response extends BaseType {
   generics?: TypeName[]
   inherits?: Inherits
   implements?: Inherits[]
-  body?: ValueBody | PropertiesBody
+  body: Body
   behaviors?: Inherits[]
   attachedBehaviors?: string[]
 }
 
+export type Body = ValueBody | PropertiesBody | NoBody
+
 export class ValueBody {
   kind: 'value'
   value: ValueOf
@@ -270,6 +267,10 @@ export class PropertiesBody {
   properties: Property[]
 }
 
+export class NoBody {
+  kind: 'no_body'
+}
+
 /**
  * An enumeration member.
  *

compiler/model/utils.ts

@@ -247,13 +247,6 @@ export function modelType (node: Node): model.ValueOf {
           return type
         }
 
-        case 'Void': {
-          const type: model.VoidValue = {
-            kind: 'void_value'
-          }
-          return type
-        }
-
         default: {
           const generics = node.getTypeArguments().map(node => modelType(node))
           const identifier = node.getTypeName()

compiler/steps/validate-model.ts

@@ -392,22 +392,28 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
     validateProperties(typeDef.query, openGenerics)
     context.pop()
 
-    if (typeDef.body != null) {
-      context.push('body')
-      switch (typeDef.body.kind) {
-        case 'properties':
-          validateProperties(typeDef.body.properties, openGenerics)
-          break
-        case 'value':
-          validateValueOf(typeDef.body.value, openGenerics)
-          break
-        default:
-          // @ts-expect-error
-          // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
-          throw new Error(`Unknown kind: ${typeDef.body.kind}`)
+    context.push('body')
+    if (typeDef.inherits != null && typeDef.body.kind !== 'properties') {
+      if (fqn(typeDef.inherits.type) !== '_types:RequestBase') {
+        modelError('A request with inherited properties must have a PropertyBody')
       }
-      context.pop()
     }
+    switch (typeDef.body.kind) {
+      case 'properties':
+        validateProperties(typeDef.body.properties, openGenerics)
+        break
+      case 'value':
+        validateValueOf(typeDef.body.value, openGenerics)
+        break
+      case 'no_body':
+        // Nothing to validate
+        break
+      default:
+        // @ts-expect-error
+        // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
+        throw new Error(`Unknown kind: ${typeDef.body.kind}`)
+    }
+    context.pop()
   }
 
   function validateResponse (typeDef: model.Response): void {
@@ -421,22 +427,28 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
     // valid overlaps, with some parameters that can also be represented as body properties.
     // Client generators will have to take care of this.
 
-    if (typeDef.body != null) {
-      context.push('body')
-      switch (typeDef.body.kind) {
-        case 'properties':
-          validateProperties(typeDef.body.properties, openGenerics)
-          break
-        case 'value':
-          validateValueOf(typeDef.body.value, openGenerics)
-          break
-        default:
-          // @ts-expect-error
-          // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
-          throw new Error(`Unknown kind: ${typeDef.body.kind}`)
+    context.push('body')
+    if (typeDef.inherits != null && typeDef.body.kind !== 'properties') {
+      if (fqn(typeDef.inherits.type) !== '_types:RequestBase') {
+        modelError('A response with inherited properties must have a PropertyBody')
       }
-      context.pop()
     }
+    switch (typeDef.body.kind) {
+      case 'properties':
+        validateProperties(typeDef.body.properties, openGenerics)
+        break
+      case 'value':
+        validateValueOf(typeDef.body.value, openGenerics)
+        break
+      case 'no_body':
+        // Nothing to validate
+        break
+      default:
+        // @ts-expect-error
+        // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
+        throw new Error(`Unknown kind: ${typeDef.body.kind}`)
+    }
+    context.pop()
   }
 
   function validateInterface (typeDef: model.Interface): void {
@@ -727,10 +739,6 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
         // Nothing to validate
         break
 
-      case 'void_value':
-        // Nothing to validate
-        break
-
       default:
         // @ts-expect-error
         // eslint-disable-next-line @typescript-eslint/restrict-template-expressions

compiler/validation-errors.ts

@@ -68,8 +68,10 @@ export class ValidationErrors {
     let count = 0
     const logArray = function (errs: string[], prefix = ''): void {
       for (const err of errs) {
-        console.error(`${prefix}${err}`)
-        count++
+        if (err !== 'Endpoint has "@stability: TODO' && err !== 'Missing request & response') {
+          console.error(`${prefix}${err}`)
+          count++
+        }
       }
     }