Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bct] Support class level properties #36007

Merged
merged 20 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 97 additions & 17 deletions scripts/breaking_changes_checker/breaking_changes_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ class ChangeType(str, Enum):
ADDED_CLIENT_METHOD = "AddedClientMethod"
ADDED_CLASS = "AddedClass"
ADDED_CLASS_METHOD = "AddedClassMethod"
ADDED_CLASS_METHOD_PROPERTY = "AddedClassMethodProperty"
catalinaperalta marked this conversation as resolved.
Show resolved Hide resolved
ADDED_CLASS_PROPERTY = "AddedClassProperty"
ADDED_FUNCTION_PARAMETER = "AddedFunctionParameter"
catalinaperalta marked this conversation as resolved.
Show resolved Hide resolved

class BreakingChangesTracker:
REMOVED_OR_RENAMED_CLIENT_MSG = \
Expand Down Expand Up @@ -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_PROPERTY_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:
Expand Down Expand Up @@ -183,24 +192,49 @@ 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
if self.parameter_name in class_components.get("properties", {}).keys():
continue
catalinaperalta marked this conversation as resolved.
Show resolved Hide resolved
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:
Expand Down Expand Up @@ -504,6 +538,52 @@ 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:
if self.function_name == "__init__":
# This is a new class property defined only through the init and not at the class level
self.features_added.append(
(
self.ADDED_CLASS_PROPERTY_MSG, ChangeType.ADDED_CLASS_PROPERTY,
catalinaperalta marked this conversation as resolved.
Show resolved Hide resolved
self.module_name, self.class_name, self.parameter_name
)
)
else:
self.features_added.append(
(
self.ADDED_CLASS_METHOD_PROPERTY_MSG, ChangeType.ADDED_CLASS_METHOD_PROPERTY,
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_added(self, current_parameters_node: Dict) -> None:
catalinaperalta marked this conversation as resolved.
Show resolved Hide resolved
if current_parameters_node["param_type"] == "positional_or_keyword" and \
current_parameters_node["default"] != "none":
if self.class_name:
self.breaking_changes.append(
(
self.ADDED_POSITIONAL_PARAM_TO_METHOD_MSG, BreakingChangeType.ADDED_POSITIONAL_PARAM,
self.module_name, self.class_name, self.function_name,
current_parameters_node["param_type"], self.parameter_name
)
)
else:
self.breaking_changes.append(
(
self.ADDED_POSITIONAL_PARAM_TO_FUNCTION_MSG, BreakingChangeType.ADDED_POSITIONAL_PARAM,
self.module_name, self.function_name,
current_parameters_node["param_type"], self.parameter_name
)
)

def check_positional_parameter_removed_or_renamed(
self, param_type: str,
deleted: str,
Expand Down
17 changes: 17 additions & 0 deletions scripts/breaking_changes_checker/detect_breaking_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,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
catalinaperalta marked this conversation as resolved.
Show resolved Hide resolved
# 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))]
Expand Down
Loading