Add check for unflattened models (#41404)

* add support for post-processing checks

* add checker for unflattened models

* refactor

* clean up

---------

Co-authored-by: catalinaperalta <caperal@microsoft.com>

Author: catalinaperalta

scripts/breaking_changes_checker/_models.py

@@ -47,3 +47,8 @@ class ChangesChecker(Protocol):
 
     def run_check(self, diff: dict, stable_nodes: dict, current_nodes: dict, **kwargs) -> List[BreakingChange]:
         ...
+
+@runtime_checkable
+class PostProcessingChecker(Protocol):
+    def run_check(self, breaking_changes: List, features_added: List, *, diff: dict, stable_nodes: dict, current_nodes: dict, **kwargs) -> tuple[list, list]:
+        ...

scripts/breaking_changes_checker/breaking_changes_tracker.py

@@ -10,7 +10,7 @@
 from enum import Enum
 from typing import Any, Dict, List, Union
 from copy import deepcopy
-from _models import ChangesChecker, Suppression, RegexSuppression
+from _models import ChangesChecker, Suppression, RegexSuppression, PostProcessingChecker
 
 
 class BreakingChangeType(str, Enum):
@@ -91,8 +91,8 @@ def __init__(self, stable: Dict, current: Dict, package_name: str, **kwargs: Any
         self.stable = stable
         self.current = current
         self.diff = jsondiff.diff(stable, current)
-        self.features_added = []
-        self.breaking_changes = []
+        self.features_added: List = []
+        self.breaking_changes: List = []
         self.package_name = package_name
         self._module_name = None
         self._class_name = None
@@ -103,13 +103,30 @@ def __init__(self, stable: Dict, current: Dict, package_name: str, **kwargs: Any
         for checker in checkers:
             if not isinstance(checker, ChangesChecker):
                 raise TypeError(f"Checker {checker} does not implement ChangesChecker protocol")
+        post_processing_checkers: List[PostProcessingChecker] = kwargs.get("post_processing_checkers", [])
+        for checker in post_processing_checkers:
+            if not isinstance(checker, PostProcessingChecker):
+                raise TypeError(f"Checker {checker} does not implement PostProcessingChecker protocol")
         self.checkers = checkers
+        self.post_processing_checkers = post_processing_checkers
 
     def run_checks(self) -> None:
         self.run_breaking_change_diff_checks()
         self.check_parameter_ordering()  # not part of diff
-        self.run_async_cleanup(self.breaking_changes)
+        self.run_post_processing()
 
+    def run_post_processing(self) -> None:
+        # Remove duplicate reporting of changes that apply to both sync and async package components
+        self.run_async_cleanup(self.breaking_changes)
+        # Run user-defined post-processing checks
+        for checker in self.post_processing_checkers:
+            bc_list, fa_list = checker.run_check(
+                self.breaking_changes, self.features_added,
+                diff=self.diff, stable_nodes=self.stable, current_nodes=self.current
+            )
+            self.breaking_changes = bc_list
+            self.features_added = fa_list
+        
     # Remove duplicate reporting of changes that apply to both sync and async package components
     def run_async_cleanup(self, changes_list: List) -> None:
         # Create a list of all sync changes

scripts/breaking_changes_checker/checkers/unflattened_models_checker.py

@@ -0,0 +1,66 @@
+#!/usr/bin/env python
+
+# --------------------------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for license information.
+# --------------------------------------------------------------------------------------------
+
+# --------------------------------------------------------------------------------------------
+# This checker is designed to clean up reported changes related to libraries migrating from
+# swagger -> typespec where models that where previously flattened are no longer flattened.
+# The checker will check for the following:
+# 1. Check if a model has a new `properties`` field
+# 2. Check if there is a corresponding `<model name>Properties` model
+# 3. Remove entries related to the new `properties`` field from the diff
+# 4. Remove entries related to the new `<model name>Properties` model from the diff
+# 5. Remove entries related losing properties on the original model
+# --------------------------------------------------------------------------------------------
+
+import copy
+import sys
+import os
+sys.path.append(os.path.abspath("../../scripts/breaking_changes_checker"))
+
+
+class UnflattenedModelsChecker:
+    def run_check(self, breaking_changes: list, features_added: list, *, diff: dict, stable_nodes: dict, current_nodes: dict, **kwargs) -> tuple[list, list]:
+        properties_model_entry = ()
+        properties_field_entry = ()
+
+        breaking_changes_copy = copy.deepcopy(breaking_changes)
+        features_added_copy = copy.deepcopy(features_added)
+        # Look for new "*Properties" models and their corresponding properties field in the original model
+        for fa in features_added:
+            _, change_type, module_name, class_name, *args = fa
+            if change_type == "AddedClass" and class_name.endswith("Properties"):
+                properties_model_entry = fa
+                # Check if the corresponding model has a properties field
+                original_model_name = class_name[:-len("Properties")]
+                if original_model_name in stable_nodes[module_name]["class_nodes"]:
+                    original_model = stable_nodes[module_name]["class_nodes"][original_model_name]
+                    for fa2 in features_added:
+                        _, change_type2, module_name2, class_name2, *args2 = fa2
+                        # Check if the class name is the original model name
+                        if class_name2 and class_name2 == original_model_name:
+                            if change_type2 == "AddedClassProperty" and "properties" == args2[0]:
+                                properties_field_entry = fa2
+                                break
+                    if not properties_field_entry or not properties_model_entry:
+                        continue
+                    # Check the breaking changes list for entries about removing any of the properties that existed in the original model
+                    for prop in original_model["properties"]:
+                        for breaking_change in breaking_changes_copy:
+                            _, bc_change_type, bc_module_name, bc_class_name, *bc_args = breaking_change
+                            if (bc_change_type == "RemovedOrRenamedInstanceAttribute" and
+                                    bc_class_name == original_model_name and
+                                    bc_args[0] == prop):
+                                # If we find a breaking change about removing a property, delete it from the breaking changes list
+                                breaking_changes_copy.remove(breaking_change)
+                                break
+                if properties_model_entry and properties_field_entry:
+                    # If we found both the properties model and the properties field, remove them from the features added list
+                    # since this means the model was unflattened
+                    features_added_copy.remove(properties_model_entry)
+                    features_added_copy.remove(properties_field_entry)
+                properties_field_entry, properties_model_entry = (), ()
+        return breaking_changes_copy, features_added_copy

scripts/breaking_changes_checker/detect_breaking_changes.py

@@ -25,7 +25,7 @@
 from breaking_changes_tracker import BreakingChangesTracker
 from changelog_tracker import ChangelogTracker
 from pathlib import Path
-from supported_checkers import CHECKERS
+from supported_checkers import CHECKERS, POST_PROCESSING_CHECKERS
 
 root_dir = os.path.abspath(os.path.join(os.path.abspath(__file__), "..", "..", ".."))
 _LOGGER = logging.getLogger(__name__)
@@ -457,10 +457,11 @@ def test_compare_reports(pkg_dir: str, changelog: bool, source_report: str = "st
         current,
         package_name,
         checkers = CHECKERS,
-        ignore = IGNORE_BREAKING_CHANGES
+        ignore = IGNORE_BREAKING_CHANGES,
+        post_processing_checkers = POST_PROCESSING_CHECKERS
     )
     if changelog:
-        checker = ChangelogTracker(stable, current, package_name, checkers = CHECKERS, ignore = IGNORE_BREAKING_CHANGES)
+        checker = ChangelogTracker(stable, current, package_name, checkers = CHECKERS, ignore = IGNORE_BREAKING_CHANGES, post_processing_checkers = POST_PROCESSING_CHECKERS)
     checker.run_checks()
 
     remove_json_files(pkg_dir)

scripts/breaking_changes_checker/supported_checkers.py

@@ -7,8 +7,14 @@
 
 from checkers.removed_method_overloads_checker import RemovedMethodOverloadChecker
 from checkers.added_method_overloads_checker import AddedMethodOverloadChecker
+from checkers.unflattened_models_checker import UnflattenedModelsChecker
 
 CHECKERS = [
     RemovedMethodOverloadChecker(),
     AddedMethodOverloadChecker(),
 ]
+
+POST_PROCESSING_CHECKERS = [
+    # Add any post-processing checkers here
+    UnflattenedModelsChecker(),
+]