Merge pull request #51 from join-com/fix-class-field-deprecation

fix: extend previous fix for deprecated fields

internal/generator/runner_generate_class.go

@@ -138,7 +138,7 @@ func (r *Runner) generateTypescriptMessageClass(generatedFileStream *protogen.Ge
 	r.indentLevel += 2
 
 	for _, fieldSpec := range messageSpec.GetField() {
-		r.generateTypescriptClassField(generatedFileStream, fieldSpec, messageSpec, messageOptions, requiredFields)
+		r.generateTypescriptClassField(generatedFileStream, fieldSpec, messageSpec, requiredFields)
 	}
 
 	r.generateTypescriptClassPatchedMethods(generatedFileStream, messageSpec, requiredFields, hasEnums)
@@ -151,12 +151,11 @@ func (r *Runner) generateTypescriptClassField(
 	generatedFileStream *protogen.GeneratedFile,
 	fieldSpec *descriptorpb.FieldDescriptorProto,
 	messageSpec *descriptorpb.DescriptorProto,
-	messageOptions *descriptorpb.MessageOptions,
 	requiredFields bool,
 ) {
 	fieldOptions := fieldSpec.GetOptions()
 	if fieldOptions != nil {
-		if messageOptions.GetDeprecated() {
+		if fieldOptions.GetDeprecated() {
 			r.P(generatedFileStream, "/**\n  * @deprecated\n */")
 		}
 	}

tests/__tests__/generated/Regressions.ts

@@ -17,7 +17,7 @@ export namespace Regressions {
     inner?: IReg01Inner
   }
 
-  export interface IWithDeprecatedField {
+  export interface IMessageWithDeprecatedField {
     notDeprecated?: string
     /**
      * @deprecated
@@ -28,7 +28,7 @@ export namespace Regressions {
   /**
    * @deprecated
    */
-  export interface IDeprecatedWithDeprecatedField {
+  export interface IDeprecatedMessageWithDeprecatedField {
     notDeprecated?: string
     /**
      * @deprecated
@@ -39,12 +39,12 @@ export namespace Regressions {
   /**
    * @deprecated
    */
-  @protobufjs.Type.d('regressions_DeprecatedWithDeprecatedField')
-  export class DeprecatedWithDeprecatedField
-    extends protobufjs.Message<DeprecatedWithDeprecatedField>
+  @protobufjs.Type.d('regressions_DeprecatedMessageWithDeprecatedField')
+  export class DeprecatedMessageWithDeprecatedField
+    extends protobufjs.Message<DeprecatedMessageWithDeprecatedField>
     implements
-      ConvertibleTo<IDeprecatedWithDeprecatedField>,
-      IDeprecatedWithDeprecatedField
+      ConvertibleTo<IDeprecatedMessageWithDeprecatedField>,
+      IDeprecatedMessageWithDeprecatedField
   {
     @protobufjs.Field.d(1, 'string', 'optional')
     public notDeprecated?: string
@@ -55,41 +55,96 @@ export namespace Regressions {
     @protobufjs.Field.d(2, 'string', 'optional')
     public deprecated?: string
 
-    public asInterface(): IDeprecatedWithDeprecatedField {
+    public asInterface(): IDeprecatedMessageWithDeprecatedField {
       const message = {
         ...this,
       }
       for (const fieldName of Object.keys(message)) {
         if (
-          message[fieldName as keyof IDeprecatedWithDeprecatedField] == null
+          message[fieldName as keyof IDeprecatedMessageWithDeprecatedField] ==
+          null
         ) {
           // We remove the key to avoid problems with code making too many assumptions
-          delete message[fieldName as keyof IDeprecatedWithDeprecatedField]
+          delete message[
+            fieldName as keyof IDeprecatedMessageWithDeprecatedField
+          ]
         }
       }
       return message
     }
 
     public static fromInterface(
       this: void,
-      value: IDeprecatedWithDeprecatedField
-    ): DeprecatedWithDeprecatedField {
-      return DeprecatedWithDeprecatedField.fromObject(value)
+      value: IDeprecatedMessageWithDeprecatedField
+    ): DeprecatedMessageWithDeprecatedField {
+      return DeprecatedMessageWithDeprecatedField.fromObject(value)
     }
 
     public static decodePatched(
       this: void,
       reader: protobufjs.Reader | Uint8Array
-    ): IDeprecatedWithDeprecatedField {
-      return DeprecatedWithDeprecatedField.decode(reader).asInterface()
+    ): IDeprecatedMessageWithDeprecatedField {
+      return DeprecatedMessageWithDeprecatedField.decode(reader).asInterface()
     }
 
     public static encodePatched(
       this: void,
-      message: IDeprecatedWithDeprecatedField,
+      message: IDeprecatedMessageWithDeprecatedField,
       writer?: protobufjs.Writer
     ): protobufjs.Writer {
-      return DeprecatedWithDeprecatedField.encode(message, writer)
+      return DeprecatedMessageWithDeprecatedField.encode(message, writer)
+    }
+  }
+
+  @protobufjs.Type.d('regressions_MessageWithDeprecatedField')
+  export class MessageWithDeprecatedField
+    extends protobufjs.Message<MessageWithDeprecatedField>
+    implements
+      ConvertibleTo<IMessageWithDeprecatedField>,
+      IMessageWithDeprecatedField
+  {
+    @protobufjs.Field.d(1, 'string', 'optional')
+    public notDeprecated?: string
+
+    /**
+     * @deprecated
+     */
+    @protobufjs.Field.d(2, 'string', 'optional')
+    public deprecated?: string
+
+    public asInterface(): IMessageWithDeprecatedField {
+      const message = {
+        ...this,
+      }
+      for (const fieldName of Object.keys(message)) {
+        if (message[fieldName as keyof IMessageWithDeprecatedField] == null) {
+          // We remove the key to avoid problems with code making too many assumptions
+          delete message[fieldName as keyof IMessageWithDeprecatedField]
+        }
+      }
+      return message
+    }
+
+    public static fromInterface(
+      this: void,
+      value: IMessageWithDeprecatedField
+    ): MessageWithDeprecatedField {
+      return MessageWithDeprecatedField.fromObject(value)
+    }
+
+    public static decodePatched(
+      this: void,
+      reader: protobufjs.Reader | Uint8Array
+    ): IMessageWithDeprecatedField {
+      return MessageWithDeprecatedField.decode(reader).asInterface()
+    }
+
+    public static encodePatched(
+      this: void,
+      message: IMessageWithDeprecatedField,
+      writer?: protobufjs.Writer
+    ): protobufjs.Writer {
+      return MessageWithDeprecatedField.encode(message, writer)
     }
   }
 
@@ -175,51 +230,4 @@ export namespace Regressions {
       return Reg01Outer.encode(message, writer)
     }
   }
-
-  @protobufjs.Type.d('regressions_WithDeprecatedField')
-  export class WithDeprecatedField
-    extends protobufjs.Message<WithDeprecatedField>
-    implements ConvertibleTo<IWithDeprecatedField>, IWithDeprecatedField
-  {
-    @protobufjs.Field.d(1, 'string', 'optional')
-    public notDeprecated?: string
-
-    @protobufjs.Field.d(2, 'string', 'optional')
-    public deprecated?: string
-
-    public asInterface(): IWithDeprecatedField {
-      const message = {
-        ...this,
-      }
-      for (const fieldName of Object.keys(message)) {
-        if (message[fieldName as keyof IWithDeprecatedField] == null) {
-          // We remove the key to avoid problems with code making too many assumptions
-          delete message[fieldName as keyof IWithDeprecatedField]
-        }
-      }
-      return message
-    }
-
-    public static fromInterface(
-      this: void,
-      value: IWithDeprecatedField
-    ): WithDeprecatedField {
-      return WithDeprecatedField.fromObject(value)
-    }
-
-    public static decodePatched(
-      this: void,
-      reader: protobufjs.Reader | Uint8Array
-    ): IWithDeprecatedField {
-      return WithDeprecatedField.decode(reader).asInterface()
-    }
-
-    public static encodePatched(
-      this: void,
-      message: IWithDeprecatedField,
-      writer?: protobufjs.Writer
-    ): protobufjs.Writer {
-      return WithDeprecatedField.encode(message, writer)
-    }
-  }
 }

tests/__tests__/generated/Test.ts

@@ -376,6 +376,9 @@ export namespace Foo {
     @protobufjs.Field.d(12, 'sint32', 'repeated')
     public fieldSint32Repeated?: number[]
 
+    /**
+     * @deprecated
+     */
     @protobufjs.Field.d(13, 'sint64', 'optional')
     public fieldSint64?: number
 

tests/__tests__/generated/common/Common.ts

@@ -22,6 +22,9 @@ export namespace Common {
     extends protobufjs.Message<OtherPkgMessage>
     implements ConvertibleTo<IOtherPkgMessage>, IOtherPkgMessage
   {
+    /**
+     * @deprecated
+     */
     @protobufjs.Field.d(1, 'string', 'optional')
     public firstName?: string
 

tests/__tests__/generated/common/Extra.ts

@@ -25,6 +25,9 @@ export namespace Common {
     extends protobufjs.Message<ExtraPkgMessage>
     implements ConvertibleTo<IExtraPkgMessage>
   {
+    /**
+     * @deprecated
+     */
     @protobufjs.Field.d(1, 'string', 'optional')
     public firstName?: string
 

tests/__tests__/proto/regressions.proto

@@ -20,12 +20,12 @@ message Reg01Outer {
 // Regression 02
 // ----------------------------------------------------------------------------
 
-message WithDeprecatedField {
+message MessageWithDeprecatedField {
   string not_deprecated = 1;
   string deprecated = 2 [deprecated=true];
 }
 
-message DeprecatedWithDeprecatedField {
+message DeprecatedMessageWithDeprecatedField {
   option deprecated = true;
   string not_deprecated = 1;
   string deprecated = 2 [deprecated=true];

tests/__tests__/regressions.test.ts

@@ -1,7 +1,7 @@
 // eslint-disable-next-line node/no-unpublished-import
 import * as ts from 'typescript'
 // eslint-disable-next-line node/no-unpublished-import
-import { InterfaceDeclaration, Project } from 'ts-morph'
+import { ClassDeclaration, InterfaceDeclaration, Project } from 'ts-morph'
 import { Regressions } from './generated/Regressions'
 // eslint-disable-next-line node/no-unpublished-import
 import { parse as parseComment } from 'comment-parser'
@@ -44,33 +44,60 @@ describe('regressions', () => {
     expect(moduleDeclaration).toBeDefined()
     expect(moduleDeclaration.getName()).toBe('Regressions')
 
+    // Here is where we verify the fix works as intended.
+    // -------------------------------------------------------------------------
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    const withDeprecatedFieldInterface = moduleDeclaration.getInterface(
-      'IWithDeprecatedField'
+    const messageInterfaceWithDeprecatedField = moduleDeclaration.getInterface(
+      'IMessageWithDeprecatedField'
     )!
-    expect(withDeprecatedFieldInterface).toBeDefined()
+    expect(messageInterfaceWithDeprecatedField).toBeDefined()
     verifyThatDeprecatedFieldHasJSDocDeprecationAnnotation(
-      withDeprecatedFieldInterface
+      messageInterfaceWithDeprecatedField
     )
     verifyThatNotDeprecatedFieldDoesNotHaveJSDocAnnotation(
-      withDeprecatedFieldInterface
+      messageInterfaceWithDeprecatedField
     )
 
-    const deprecatedWithDeprecatedFieldInterface =
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    const messageClassWithDeprecatedField = moduleDeclaration.getClass(
+      'MessageWithDeprecatedField'
+    )!
+    expect(messageClassWithDeprecatedField).toBeDefined()
+    verifyThatDeprecatedFieldHasJSDocDeprecationAnnotation(
+      messageClassWithDeprecatedField
+    )
+    verifyThatNotDeprecatedFieldDoesNotHaveJSDocAnnotation(
+      messageClassWithDeprecatedField
+    )
+
+    // Here we verify that the fix did not introduce any unintended regression.
+    // -------------------------------------------------------------------------
+    const deprecatedMessageInterfaceWithDeprecatedField =
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      moduleDeclaration.getInterface('IDeprecatedMessageWithDeprecatedField')!
+    expect(deprecatedMessageInterfaceWithDeprecatedField).toBeDefined()
+    verifyThatDeprecatedFieldHasJSDocDeprecationAnnotation(
+      deprecatedMessageInterfaceWithDeprecatedField
+    )
+    verifyThatNotDeprecatedFieldDoesNotHaveJSDocAnnotation(
+      deprecatedMessageInterfaceWithDeprecatedField
+    )
+
+    const deprecatedMessageClassWithDeprecatedField =
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      moduleDeclaration.getInterface('IDeprecatedWithDeprecatedField')!
-    expect(deprecatedWithDeprecatedFieldInterface).toBeDefined()
+      moduleDeclaration.getClass('DeprecatedMessageWithDeprecatedField')!
+    expect(deprecatedMessageClassWithDeprecatedField).toBeDefined()
     verifyThatDeprecatedFieldHasJSDocDeprecationAnnotation(
-      deprecatedWithDeprecatedFieldInterface
+      deprecatedMessageClassWithDeprecatedField
     )
     verifyThatNotDeprecatedFieldDoesNotHaveJSDocAnnotation(
-      deprecatedWithDeprecatedFieldInterface
+      deprecatedMessageClassWithDeprecatedField
     )
   })
 })
 
 function verifyThatDeprecatedFieldHasJSDocDeprecationAnnotation(
-  generatedInterface: InterfaceDeclaration
+  generatedInterface: InterfaceDeclaration | ClassDeclaration
 ): void {
   const deprecatedField =
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
@@ -94,7 +121,7 @@ function verifyThatDeprecatedFieldHasJSDocDeprecationAnnotation(
 }
 
 function verifyThatNotDeprecatedFieldDoesNotHaveJSDocAnnotation(
-  generatedInterface: InterfaceDeclaration
+  generatedInterface: InterfaceDeclaration | ClassDeclaration
 ): void {
   const notDeprecatedField =
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion