wip - support non-breaking changes

scripts/breaking_changes_checker/breaking_changes_tracker.py

@@ -29,6 +29,11 @@ class BreakingChangeType(str, Enum):
     REMOVED_OR_RENAMED_MODULE = "RemovedOrRenamedModule"
     REMOVED_FUNCTION_KWARGS = "RemovedFunctionKwargs"
 
+    # ----------------- General Changes -----------------
+    ADDED_CLIENT = "AddedClient"
+    ADDED_CLIENT_METHOD = "AddedClientMethod"
+    ADDED_CLASS = "AddedClass"
+    ADDED_CLASS_METHOD = "AddedClassMethod"
 
 class BreakingChangesTracker:
     REMOVED_OR_RENAMED_CLIENT_MSG = \
@@ -88,11 +93,23 @@ class BreakingChangesTracker:
         "({}): The function '{}.{}' changed from accepting keyword arguments to not accepting them in " \
         "the current version"
 
+    # ----------------- General Changes -----------------
+    ADDED_CLIENT_MSG = \
+        "({}): The client '{}.{}' was added in the current version"
+    ADDED_CLIENT_METHOD_MSG = \
+        "({}): The '{}.{}' client method '{}' was added in the current version"
+    ADDED_CLASS_MSG = \
+        "({}): The model or publicly exposed class '{}.{}' was added in the current version"
+    ADDED_CLASS_METHOD_MSG = \
+        "({}): The '{}.{}' method '{}' was added in the current version"
+
+
     def __init__(self, stable: Dict, current: Dict, diff: Dict, package_name: str, **kwargs: Any) -> None:
         self.stable = stable
         self.current = current
         self.diff = diff
         self.breaking_changes = []
+        self.features_added = []
         self.package_name = package_name
         self.module_name = None
         self.class_name = None
@@ -102,6 +119,9 @@ def __init__(self, stable: Dict, current: Dict, diff: Dict, package_name: str, *
 
     def __str__(self):
         formatted = "\n"
+        for fa in self.features_added:
+            formatted += fa + "\n"
+
         for bc in self.breaking_changes:
             formatted += bc + "\n"
 
@@ -110,6 +130,8 @@ def __str__(self):
         return formatted
 
     def run_checks(self) -> None:
+        # TODO: this should be conditional based on what folks want reported
+        self.run_non_breaking_change_diff_checks()
         self.run_breaking_change_diff_checks()
         self.check_parameter_ordering()  # not part of diff
         self.report_breaking_changes()
@@ -127,6 +149,62 @@ def run_breaking_change_diff_checks(self) -> None:
             self.run_class_level_diff_checks(module)
             self.run_function_level_diff_checks(module)
 
+    def run_non_breaking_change_diff_checks(self) -> None:
+        for module_name, module in self.diff.items():
+            self.module_name = module_name
+            if self.module_name not in self.stable and not isinstance(self.module_name, jsondiff.Symbol):
+                continue  # TODO add this to reported changes
+
+            self.run_non_breaking_class_level_diff_checks(module)
+
+    def run_non_breaking_class_level_diff_checks(self, module: Dict) -> None:
+        for class_name, class_components in module.get("class_nodes", {}).items():
+            self.class_name = class_name
+            stable_class_nodes = self.stable[self.module_name]["class_nodes"]
+            if not isinstance(class_name, jsondiff.Symbol):
+                if self.class_name not in stable_class_nodes:
+                    if self.class_name.endswith("Client"):
+                        # This is a new client
+                        fa = (
+                            self.ADDED_CLIENT_MSG,
+                            BreakingChangeType.ADDED_CLIENT,
+                            self.module_name, class_name
+                        )
+                        self.features_added.append(fa)
+                    else:
+                        # This is a new class
+                        fa = (
+                            self.ADDED_CLASS_MSG,
+                            BreakingChangeType.ADDED_CLASS,
+                            self.module_name, class_name
+                        )
+                        self.features_added.append(fa)
+                else:
+                    # Check existing class for new methods
+                    stable_methods_node = stable_class_nodes[self.class_name]["methods"]
+                    for method_name, method_components in class_components.get("methods", {}).items():
+                        self.function_name = method_name
+                        # current_methods_node = self.current[self.module_name]["class_nodes"][self.class_name]["methods"]
+                        if self.function_name not in stable_methods_node and \
+                                not isinstance(self.function_name, jsondiff.Symbol):
+                            if self.class_name.endswith("Client"):
+                                # This is a new client method
+                                fa = (
+                                    self.ADDED_CLIENT_METHOD_MSG,
+                                    BreakingChangeType.ADDED_CLIENT_METHOD,
+                                    self.module_name, self.class_name, method_name
+                                )
+                                self.features_added.append(fa)
+                            else:
+                                # This is a new class method
+                                fa = (
+                                    self.ADDED_CLASS_METHOD_MSG,
+                                    BreakingChangeType.ADDED_CLASS_METHOD,
+                                    self.module_name, class_name, method_name
+                                )
+                                self.features_added.append(fa)
+
+
     def run_class_level_diff_checks(self, module: Dict) -> None:
         for class_name, class_components in module.get("class_nodes", {}).items():
             self.class_name = class_name
@@ -568,6 +646,7 @@ def check_module_level_function_removed_or_renamed(self, function_components: Di
                 )
             return True
 
+    # ----------------------------------- Report methods -----------------------------------
     def get_reportable_breaking_changes(self, ignore_changes: Dict) -> List:
         reportable_changes = []
         ignored = ignore_changes[self.package_name]
@@ -582,11 +661,36 @@ def get_reportable_breaking_changes(self, ignore_changes: Dict) -> List:
             reportable_changes.append(bc)
         return reportable_changes
 
-    def report_breaking_changes(self) -> None:
+    def report_changelog(self) -> None:
+        def _build_md(content, title, buffer):
+            buffer.append(title)
+            buffer.append("")
+            for _, bc in enumerate(content):
+                msg, *args = bc
+                buffer.append(msg.format(*args))
+            buffer.append("")
+            return buffer
+
+        buffer = []
+        _build_md([], "# Release history", buffer)
+        _build_md([], f"## current", buffer)
+        _build_md(self.breaking_changes, "### Breaking Changes", buffer)
+        _build_md(self.features_added, "### Features Added", buffer)
+        content =  "\n".join(buffer).strip()
+        with open("changelog.md", "rw") as fd:
+            content.append(fd.read())
+            fd.write(content)
+
+    def report_breaking_changes(self, changelog: bool = False) -> None:
         ignore_changes = self.ignore if self.ignore else IGNORE_BREAKING_CHANGES
         if self.package_name in ignore_changes:
             self.breaking_changes = self.get_reportable_breaking_changes(ignore_changes)
 
         for idx, bc in enumerate(self.breaking_changes):
             msg, *args = bc
             self.breaking_changes[idx] = msg.format(*args)
+
+        # TODO - need to do this separately for changelog
+        for idx, fa in enumerate(self.features_added):
+            msg, *args = fa
+            self.features_added[idx] = msg.format(*args)

scripts/breaking_changes_checker/detect_breaking_changes.py

@@ -274,9 +274,9 @@ def test_compare_reports(pkg_dir: str, version: str) -> None:
     bc = BreakingChangesTracker(stable, current, diff, package_name)
     bc.run_checks()
 
-    remove_json_files(pkg_dir)
+    # remove_json_files(pkg_dir)
 
-    if bc.breaking_changes:
+    if bc.features_added or bc.breaking_changes:
         print(bc)
         exit(1)
 
@@ -344,7 +344,7 @@ def main(package_name: str, target_module: str, version: str, in_venv: Union[boo
 
     except Exception as err:  # catch any issues with capturing the public API and building the report
         print("\n*****See aka.ms/azsdk/breaking-changes-tool to resolve any build issues*****\n")
-        remove_json_files(pkg_dir)
+        # remove_json_files(pkg_dir)
         raise err
 
 
@@ -391,10 +391,10 @@ def main(package_name: str, target_module: str, version: str, in_venv: Union[boo
     pkg_dir = os.path.abspath(args.target_package)
     package_name = os.path.basename(pkg_dir)
     logging.basicConfig(level=logging.INFO)
-    if package_name not in RUN_BREAKING_CHANGES_PACKAGES:
-        _LOGGER.info(f"{package_name} opted out of breaking changes checks. "
-                     f"See http://aka.ms/azsdk/breaking-changes-tool to opt-in.")
-        exit(0)
+    # if package_name not in RUN_BREAKING_CHANGES_PACKAGES:
+    #     _LOGGER.info(f"{package_name} opted out of breaking changes checks. "
+    #                  f"See http://aka.ms/azsdk/breaking-changes-tool to opt-in.")
+    #     exit(0)
 
     if not target_module:
         from ci_tools.parsing import ParsedSetup