[bct] Support class level properties (#36007)

* wip - support non-breaking changes

* refactor to support changelog functionality

* print changelog or breaking changes

* support changelog flag

* detect and register class level attributes

* diff check for class properties and method params

* revert temp changes

* fix property checking logic

* add some init handling

* cleanup

* add comment

* add tests

* rename added class method parameter

* cleanup

* revert init exception

* update tests

* comment

---------

Co-authored-by: Catalina Peralta <[email protected]>

Author: catalinaperalta

scripts/breaking_changes_checker/breaking_changes_tracker.py

@@ -35,6 +35,9 @@ class ChangeType(str, Enum):
     ADDED_CLIENT_METHOD = "AddedClientMethod"
     ADDED_CLASS = "AddedClass"
     ADDED_CLASS_METHOD = "AddedClassMethod"
+    ADDED_CLASS_METHOD_PARAMETER = "AddedClassMethodParameter"
+    ADDED_CLASS_PROPERTY = "AddedClassProperty"
+    ADDED_FUNCTION_PARAMETER = "AddedFunctionParameter"
 
 class BreakingChangesTracker:
     REMOVED_OR_RENAMED_CLIENT_MSG = \
@@ -103,6 +106,12 @@ class BreakingChangesTracker:
         "The model or publicly exposed class '{}.{}' was added in the current version"
     ADDED_CLASS_METHOD_MSG = \
         "The '{}.{}' method '{}' was added in the current version"
+    ADDED_CLASS_METHOD_PARAMETER_MSG = \
+        "The model or publicly exposed class '{}.{}' had property '{}' added in the {} method in the current version"
+    ADDED_FUNCTION_PARAMETER_MSG = \
+        "The function '{}.{}' had parameter '{}' added in the current version"
+    ADDED_CLASS_PROPERTY_MSG = \
+        "The model or publicly exposed class '{}.{}' had property '{}' added in the current version"
 
 
     def __init__(self, stable: Dict, current: Dict, diff: Dict, package_name: str, **kwargs: Any) -> None:
@@ -183,24 +192,51 @@ def run_non_breaking_class_level_diff_checks(self, module: Dict) -> None:
                     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
-                        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,
-                                    ChangeType.ADDED_CLIENT_METHOD,
-                                    self.module_name, self.class_name, method_name
-                                )
-                                self.features_added.append(fa)
+                        if not isinstance(self.function_name, jsondiff.Symbol):
+                            if self.function_name not in stable_methods_node:
+                                if self.class_name.endswith("Client"):
+                                    # This is a new client method
+                                    fa = (
+                                        self.ADDED_CLIENT_METHOD_MSG,
+                                        ChangeType.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,
+                                        ChangeType.ADDED_CLASS_METHOD,
+                                        self.module_name, class_name, method_name
+                                    )
+                                    self.features_added.append(fa)
                             else:
-                                # This is a new class method
-                                fa = (
-                                    self.ADDED_CLASS_METHOD_MSG,
-                                    ChangeType.ADDED_CLASS_METHOD,
-                                    self.module_name, class_name, method_name
-                                )
-                                self.features_added.append(fa)
+                                # Check existing methods for new parameters
+                                stable_parameters_node = stable_methods_node[self.function_name]["parameters"]
+                                current_parameters_node = self.current[self.module_name]["class_nodes"][self.class_name]["methods"][self.function_name]["parameters"]
+                                for param_name, param_components in method_components.get("parameters", {}).items():
+                                    self.parameter_name = param_name
+                                    if self.parameter_name not in stable_parameters_node and \
+                                            not isinstance(self.parameter_name, jsondiff.Symbol):
+                                        if self.function_name == "__init__":
+                                            # If this is a new class property skip reporting it here and let the class properties check handle it.
+                                            # This is because we'll get multiple reports for the same new property if it's a parameter in __init__
+                                            # and a class level attribute.
+                                            if self.parameter_name in class_components.get("properties", {}).keys():
+                                                continue
+                                        self.check_non_positional_parameter_added(
+                                            current_parameters_node[param_name]
+                                        )
+                    stable_property_node = stable_class_nodes[self.class_name]["properties"]
+                    for property_name, property_components in class_components.get("properties", {}).items():
+                        if property_name not in stable_property_node and \
+                                not isinstance(property_name, jsondiff.Symbol):
+                            fa = (
+                                self.ADDED_CLASS_PROPERTY_MSG,
+                                ChangeType.ADDED_CLASS_PROPERTY,
+                                self.module_name, class_name, property_name
+                            )
+                            self.features_added.append(fa)
 
 
     def run_class_level_diff_checks(self, module: Dict) -> None:
@@ -504,6 +540,23 @@ def check_positional_parameter_added(self, current_parameters_node: Dict) -> Non
                     )
                 )
 
+    def check_non_positional_parameter_added(self, current_parameters_node: Dict) -> None:
+        if current_parameters_node["param_type"] != "positional_or_keyword":
+            if self.class_name:
+                self.features_added.append(
+                    (
+                        self.ADDED_CLASS_METHOD_PARAMETER_MSG, ChangeType.ADDED_CLASS_METHOD_PARAMETER,
+                        self.module_name, self.class_name, self.parameter_name, self.function_name
+                    )
+                )
+            else:
+                self.features_added.append(
+                    (
+                        self.ADDED_FUNCTION_PARAMETER_MSG, ChangeType.ADDED_FUNCTION_PARAMETER,
+                        self.module_name, self.function_name, self.parameter_name
+                    )
+                )
+
     def check_positional_parameter_removed_or_renamed(
         self, param_type: str,
         deleted: str,

scripts/breaking_changes_checker/detect_breaking_changes.py

@@ -101,6 +101,23 @@ def get_parameter_default(param: inspect.Parameter) -> None:
 
 
 def get_property_names(node: ast.AST, attribute_names: Dict) -> None:
+    assign_nodes = [node for node in node.body if isinstance(node, ast.AnnAssign)]
+    # Check for class level attributes that follow the pattern: foo: List["_models.FooItem"] = rest_field(name="foo")
+    for assign in assign_nodes:
+        if hasattr(assign, "target"):
+            if hasattr(assign.target, "id") and not assign.target.id.startswith("_"):
+                attr = assign.target.id
+                attr_type = None
+                # FIXME: This can get the type hint for a limited set attributes. We need to address more complex
+                # type hints in the future.
+                # Build type hint for the attribute
+                if hasattr(assign.annotation, "value") and isinstance(assign.annotation.value, ast.Name):
+                    attr_type = assign.annotation.value.id
+                    if attr_type == "List" and hasattr(assign.annotation, "slice"):
+                        if isinstance(assign.annotation.slice, ast.Constant):
+                            attr_type = f"{attr_type}[{assign.annotation.slice.value}]"
+                attribute_names.update({attr: attr_type})
+
     func_nodes = [node for node in node.body if isinstance(node, ast.FunctionDef)]
     if func_nodes:
         assigns = [node for node in func_nodes[0].body if isinstance(node, (ast.Assign, ast.AnnAssign))]