findClassDeclarationBreakingChanges

tools/js-sdk-release-tools/packages/typescript-codegen-breaking-change-detector/misc/test-cases/patch-detection/baseline-package/patch.api.md

@@ -31,4 +31,15 @@ export function funcReturnType(a: string): string
 export function funcParameterCount(a: string, b: string): string
 export function funcParameterType(a: string): string
 export function funcRemove(a: string): string
+
+export class classPropertyChange {a: string;}
+export class classPropertyType {a: string;}
+export class classRemove {a: string;}
+export class classExpand {a: string;}
+export class classNarrow {a: string;b: string;}
+export class classConstructorParameterCount { constructor(a: string, b: string){} }
+export class classConstructorParameterType { constructor(a: string, b: string){} }
+export class classConstructorParameterOptional { constructor(a: string, b: string){} }
+export class classConstructorRemove { constructor(a: number, b: number); constructor(a?: number, b?: number, c?: number){} }
+export class classConstructorAdd { }
 ```

tools/js-sdk-release-tools/packages/typescript-codegen-breaking-change-detector/misc/test-cases/patch-detection/current-package/patch.api.md

@@ -35,4 +35,15 @@ export function funcReturnType(a: string): number
 export function funcParameterCount(a: string, b: string, c: string): string
 export function funcParameterType(a: number): string
 export function funcAdd(a: string): string
+
+export class classPropertyChange {private a: string;}
+export class classPropertyType {a: number;}
+export class classAdd {a: string;}
+export class classExpand {a: string;b: string;}
+export class classNarrow {a: string;}
+export class classConstructorParameterCount { constructor(a: string){} }
+export class classConstructorParameterType { constructor(a: number, b: number){} }
+export class classConstructorParameterOptional { constructor(a: string, b?: string){} }
+export class classConstructorRemove { constructor(a?: number, b?: number, c?: number){} }
+export class classConstructorAdd { constructor(a: number, b: number){} }
 ```

tools/js-sdk-release-tools/packages/typescript-codegen-breaking-change-detector/src/azure/common/types.ts

@@ -13,6 +13,7 @@ import {
   SourceFile,
   TypeAliasDeclaration,
   Node,
+  ConstructorDeclaration,
 } from 'ts-morph';
 
 export interface ParseForESLintResult {
@@ -81,6 +82,8 @@ export enum DiffReasons {
   RequiredToOptional = 8,
   ReadonlyToMutable = 16,
 
+  ModifierFlag = 17,
+
   // new features
   Added = 1024,
 }
@@ -108,6 +111,8 @@ export enum DiffLocation {
   Property,
   TypeAlias,
   Interface,
+  Class,
+  Constructor
 }
 
 export enum AssignDirection {
@@ -120,3 +125,8 @@ export type FindMappingCallSignature = (
   target: Signature,
   signatures: Signature[]
 ) => { signature: Signature; id: string } | undefined;
+
+export type FindMappingConstructor = (
+  currentIndex: ConstructorDeclaration,
+  constraints: ConstructorDeclaration[],
+) => ConstructorDeclaration | undefined;

tools/js-sdk-release-tools/packages/typescript-codegen-breaking-change-detector/src/azure/diff/declaration-diff.ts

@@ -11,6 +11,9 @@ import {
   TypeNode,
   TypeAliasDeclaration,
   CallSignatureDeclaration,
+  ClassDeclaration,
+  ConstructorDeclaration,
+  ClassMemberTypes,
 } from 'ts-morph';
 import {
   DiffLocation,
@@ -19,6 +22,7 @@ import {
   FindMappingCallSignature,
   AssignDirection,
   NameNode,
+  FindMappingConstructor,
 } from '../common/types';
 import {
   getCallableEntityParametersFromSymbol,
@@ -63,7 +67,7 @@ function findBreakingReasons(source: Node, target: Node): DiffReasons {
   if (!assignable) breakingReasons |= DiffReasons.TypeChanged;
 
   // check required -> optional
-  const isOptional = (node: Node) => node.getSymbolOrThrow().isOptional();
+  const isOptional = (node: Node) => node.asKind(SyntaxKind.Parameter)?.isOptional();
   const incompatibleOptional = isOptional(target) && !isOptional(source);
   if (incompatibleOptional) breakingReasons |= DiffReasons.RequiredToOptional;
 
@@ -383,6 +387,129 @@ export function findTypeAliasBreakingChanges(source: TypeAliasDeclaration, targe
   return [createDiffPair(DiffLocation.TypeAlias, DiffReasons.TypeChanged, sourceNameNode, targetNameNode)];
 }
 
+export function findClassDeclarationBreakingChanges(source: ClassDeclaration, target: ClassDeclaration): DiffPair[] {
+  const targetConstructors = target.getConstructors();
+  const sourceConstructors = source.getConstructors();
+  const constructorBreakingChanges = findConstructorBreakingChanges(sourceConstructors, targetConstructors);
+
+  const targetProperties = target.getType().getProperties();
+  const sourceProperties = source.getType().getProperties();
+  const propertyBreakingChanges = findPropertyBreakingChanges(sourceProperties, targetProperties);
+
+  //source.getMembers().forEach((p) => console.log(p.getText() + ' ' + p.getCombinedModifierFlags()));
+  const targetMembers = target.getMembers();
+  const sourceMembers = source.getMembers();
+  const memberBreakingChanges = findMemberBreakingChanges(sourceMembers, targetMembers);
+
+  return [...constructorBreakingChanges, ...propertyBreakingChanges, ...memberBreakingChanges];
+}
+
+export function isSameConstructor(left: ConstructorDeclaration, right: ConstructorDeclaration): boolean {
+  const leftOverloads = left.getOverloads()
+  const rightOverloads = right.getOverloads()
+  const overloads = leftOverloads.filter((t) => {
+    const compatibleSourceFunction = rightOverloads.find((s) => {
+      // NOTE: isTypeAssignableTo does not work for overloads
+      const parameterPairs = [
+        ...findParameterBreakingChangesCore(s.getParameters(), t.getParameters(), '', '', s, t),
+        ...findParameterBreakingChangesCore(t.getParameters(), s.getParameters(), '', '', t, s),
+      ];
+      return parameterPairs.length === 0;
+    });
+    return compatibleSourceFunction === undefined;
+  });
+  return overloads.length === 0;
+}
+
+
+function findConstructorBreakingChanges(
+  sourceConstraints: ConstructorDeclaration[],
+  targetConstraints: ConstructorDeclaration[]
+): DiffPair[] {
+  const pairs = targetConstraints.reduce((result, targetConstraint, currentIndex) => {
+    const defaultFindMappingConstructor: FindMappingConstructor = (
+      target: ConstructorDeclaration,
+      constraints: ConstructorDeclaration[]
+    ) => {
+      const constraint = constraints.find((s) => isSameConstructor(target, s));
+      if (!constraint) return undefined;
+      return constraint;
+    };
+    const resolvedFindMappingConstructor = defaultFindMappingConstructor;
+    const sourceContext = resolvedFindMappingConstructor(targetConstraint, sourceConstraints);
+    if (sourceContext) {
+      const sourceConstraint = sourceContext;
+
+      // handle parameters
+      const getParameters = (s: ConstructorDeclaration): ParameterDeclaration[] => s.getParameters();
+
+      const parameterPairs = findParameterBreakingChangesCore(
+        getParameters(sourceConstraint),
+        getParameters(targetConstraint),
+        currentIndex.toString(),
+        currentIndex.toString(),
+        sourceConstraint,
+        targetConstraint
+      );
+      if (parameterPairs.length > 0) result.push(...parameterPairs);
+
+      return result;
+    }
+
+    // not found
+    const getNameNode = (c: ConstructorDeclaration): NameNode => ({ name: c.getText(), node: c });
+    const targetNameNode = getNameNode(targetConstraint);
+    const pair = createDiffPair(DiffLocation.Constructor, DiffReasons.Removed, undefined, targetNameNode);
+    result.push(pair);
+    return result;
+  }, new Array<DiffPair>());
+  return pairs;
+}
+
+function findMemberBreakingChanges(sourceMembers: ClassMemberTypes[], targetMembers: ClassMemberTypes[]): DiffPair[] {
+  const sourcePropMap = sourceMembers.reduce((map, p) => {
+    map.set(p.getSymbol()!.getName(), p);
+    return map;
+  }, new Map<string, ClassMemberTypes>());
+
+  const getLocation = (symbol: Symbol): DiffLocation => {
+    let location;
+    switch(symbol.getFlags()){
+      case SymbolFlags.Method:
+        location = DiffLocation.Signature;
+        break;
+      case SymbolFlags.Property:  
+        location = DiffLocation.Property;
+        break;  
+      default:
+        location = DiffLocation.Constructor;
+    }
+    return location
+  };  
+
+  const changed = targetMembers.reduce((result, targetMember) => {
+    const name = targetMember.getSymbol()!.getName();
+    const sourceMember = sourcePropMap.get(name);
+    if (!sourceMember) return result;
+
+    const targetMemberModifierFlag = targetMember.getCombinedModifierFlags();
+    const sourceMemberModifierFlag = sourceMember.getCombinedModifierFlags();
+    const location = getLocation(targetMember.getSymbol()!);
+    if (targetMemberModifierFlag !== sourceMemberModifierFlag) {
+      return [
+        ...result,
+        createDiffPair(
+          location,
+          DiffReasons.ModifierFlag,
+          getNameNodeFromSymbol(sourceMember.getSymbol()!),
+          getNameNodeFromSymbol(targetMember.getSymbol()!)
+        ),
+      ];
+    }
+    return result;
+  }, new Array<DiffPair>());
+  return [ ...changed];
+}
 export function createDiffPair(
   location: DiffLocation,
   reasons: DiffReasons,

tools/js-sdk-release-tools/packages/typescript-codegen-breaking-change-detector/src/azure/patch/patch-detection.ts

@@ -7,6 +7,7 @@ import {
   checkRemovedDeclaration,
   createDiffPair,
   checkAddedDeclaration,
+  findClassDeclarationBreakingChanges,
 } from '../diff/declaration-diff';
 import { logger } from '../../logging/logger';
 
@@ -134,6 +135,43 @@ export function patchFunction(name: string, astContext: AstContext): DiffPair[]
   return pairs;
 }
 
+export function patchClass(name: string, astContext: AstContext): DiffPair[] {
+  const baseline = astContext.baseline.getClass(name);
+  const current = astContext.current.getClass(name);
+  const addPair = checkAddedDeclaration(DiffLocation.Class, baseline, current);
+  if (addPair) return [addPair];
+
+  const removePair = checkRemovedDeclaration(DiffLocation.Class, baseline, current);
+  if (removePair) return [removePair];
+  const breakingChangePairs = patchDeclaration(
+    AssignDirection.CurrentToBaseline,
+    findClassDeclarationBreakingChanges,
+    baseline!,
+    current!,
+  );
+
+  const newFeaturePairs = patchDeclaration(
+    AssignDirection.BaselineToCurrent,
+    findClassDeclarationBreakingChanges,
+    baseline!,
+    current!,
+  ).filter((p) => 
+    p.reasons === DiffReasons.Removed ||
+    p.reasons === DiffReasons.RequiredToOptional)
+  .map((p) => {
+    if(p.reasons === DiffReasons.Removed)
+    {
+      p.reasons = DiffReasons.Added;
+      p.assignDirection = AssignDirection.CurrentToBaseline;
+      const temp = p.source;
+      p.source = p.target;
+      p.target = temp;      
+    }
+    return p;
+  });
+  return [...breakingChangePairs, ...newFeaturePairs];
+}
+
 export function patchDeclaration<T extends Node>(
   assignDirection: AssignDirection,
   findBreakingChanges: (source: T, target: T, ...extra: any) => DiffPair[],

tools/js-sdk-release-tools/packages/typescript-codegen-breaking-change-detector/src/tests/patch-detection.test.ts

@@ -3,7 +3,7 @@ import { describe, expect, test } from 'vitest';
 
 import { join } from 'node:path';
 import { createAstContext } from '../azure/detect-breaking-changes';
-import { patchFunction, patchRoutes, patchTypeAlias } from '../azure/patch/patch-detection';
+import { patchFunction, patchRoutes, patchTypeAlias, patchClass} from '../azure/patch/patch-detection';
 import { createTempFolder, getFormattedDate } from './utils';
 import { DiffLocation, DiffReasons, AssignDirection } from '../azure/common/types';
 
@@ -180,4 +180,107 @@ describe("patch current tool's breaking changes", async () => {
       if (tempFolder) remove(tempFolder);
     }
   });
+
+  test('detect class', async () => {
+    const currentApiViewPath = join(__dirname, testCaseDir, 'current-package/patch.api.md');
+    const baselineApiViewPath = join(__dirname, testCaseDir, 'baseline-package/patch.api.md');
+    const date = getFormattedDate();
+
+    let tempFolder: string | undefined = undefined;
+    try {
+      tempFolder = await createTempFolder(`.tmp/temp-${date}`);
+      const astContext = await createAstContext(baselineApiViewPath, currentApiViewPath, tempFolder);
+      let breakingPairs = patchClass('classPropertyChange', astContext);
+      expect(breakingPairs.length).toBe(1);
+      expect(breakingPairs[0].assignDirection).toBe(AssignDirection.CurrentToBaseline);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Property);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.ModifierFlag);
+      expect(breakingPairs[0].target?.name).toBe('a');
+
+      breakingPairs = patchClass('classPropertyType', astContext);
+      expect(breakingPairs.length).toBe(1);
+      expect(breakingPairs[0].assignDirection).toBe(AssignDirection.CurrentToBaseline);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Property);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.TypeChanged);
+      expect(breakingPairs[0].target?.name).toBe('a');
+
+      breakingPairs = patchClass('classRemove', astContext);
+      expect(breakingPairs.length).toBe(1);
+      expect(breakingPairs[0].assignDirection).toBe(AssignDirection.CurrentToBaseline);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Class);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.Removed);
+      expect(breakingPairs[0].target?.name).toBe('classRemove');
+
+      breakingPairs = patchClass('classAdd', astContext);
+      expect(breakingPairs.length).toBe(1);
+      expect(breakingPairs[0].assignDirection).toBe(AssignDirection.CurrentToBaseline);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Class);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.Added);
+      expect(breakingPairs[0].source?.name).toBe('classAdd');
+
+      breakingPairs = patchClass('classExpand', astContext);
+      expect(breakingPairs.length).toBe(1);
+      expect(breakingPairs[0].assignDirection).toBe(AssignDirection.CurrentToBaseline);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Property);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.Added);
+      expect(breakingPairs[0].source?.name).toBe('b');
+
+      breakingPairs = patchClass('classNarrow', astContext);
+      expect(breakingPairs.length).toBe(1);
+      expect(breakingPairs[0].assignDirection).toBe(AssignDirection.CurrentToBaseline);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Property);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.Removed);
+      expect(breakingPairs[0].target?.name).toBe('b');
+
+      breakingPairs = patchClass('classConstructorParameterCount', astContext);
+      expect(breakingPairs.length).toBe(1);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Signature_ParameterList);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.CountChanged);
+      expect(breakingPairs[0].target?.node.getText()).toBe(
+        'constructor(a: string, b: string){}'
+      );
+
+      breakingPairs = patchClass('classConstructorParameterType', astContext);
+      expect(breakingPairs.length).toBe(2);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Parameter);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.TypeChanged);
+      expect(breakingPairs[0].target?.node.getText()).toBe('a: string');
+
+      expect(breakingPairs[1].location).toBe(DiffLocation.Parameter);
+      expect(breakingPairs[1].reasons).toBe(DiffReasons.TypeChanged);
+      expect(breakingPairs[1].target?.node.getText()).toBe('b: string');
+
+      breakingPairs = patchClass('classConstructorRemove', astContext);
+      expect(breakingPairs.length).toBe(1);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Constructor);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.Removed);
+      expect(breakingPairs[0].source).toBeUndefined();
+      expect(breakingPairs[0].target?.node.getText()).toBe(
+        'constructor(a?: number, b?: number, c?: number){}'
+      );
+
+      breakingPairs = patchClass('classConstructorAdd', astContext);
+      expect(breakingPairs.length).toBe(1);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Constructor);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.Added);
+      expect(breakingPairs[0].target).toBeUndefined();
+      expect(breakingPairs[0].source?.node.getText()).toBe(
+        'constructor(a: number, b: number){}'
+      );
+
+      breakingPairs = patchClass('classConstructorParameterOptional', astContext);
+      expect(breakingPairs.length).toBe(1);
+      expect(breakingPairs[0].location).toBe(DiffLocation.Parameter);
+      expect(breakingPairs[0].reasons).toBe(DiffReasons.RequiredToOptional);
+      expect(breakingPairs[0].target?.node.getText()).toBe(
+        'b?: string'
+      );;
+      expect(breakingPairs[0].source?.node.getText()).toBe(
+        'b: string'
+      );
+
+    } finally {
+      if (tempFolder) remove(tempFolder);
+    }
+  });
 });