Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
30 changes: 30 additions & 0 deletions scripts/breaking_changes_checker/breaking_changes_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class BreakingChangeType(str, Enum):
REMOVED_OR_RENAMED_MODULE = "RemovedOrRenamedModule"
REMOVED_FUNCTION_KWARGS = "RemovedFunctionKwargs"
REMOVED_OR_RENAMED_OPERATION_GROUP = "RemovedOrRenamedOperationGroup"
REQUIRED_PROPERTY = "RequiredProperty"


class BreakingChangesTracker:
Expand Down Expand Up @@ -90,6 +91,7 @@ class BreakingChangesTracker:
"Function `{}` changed from accepting keyword arguments to not accepting them"
REMOVED_OR_RENAMED_OPERATION_GROUP_MSG = \
"Deleted or renamed client operation group `{}.{}`"
REQUIRED_PROPERTY_MSG = "`{}.{}` is now required."

def __init__(self, stable: Dict, current: Dict, package_name: str, **kwargs: Any) -> None:
self.stable = stable
Expand Down Expand Up @@ -259,6 +261,7 @@ def run_class_level_diff_checks(self, module: Dict) -> None:
if class_deleted:
continue # class was deleted, abort other checks
self.check_class_instance_attribute_removed_or_renamed(class_components)
self.check_property_required(class_components)

for method_name, method_components in class_components.get("methods", {}).items():
self._function_name = method_name
Expand Down Expand Up @@ -659,6 +662,28 @@ def check_class_instance_attribute_removed_or_renamed(self, components: Dict) ->
if bc:
self.breaking_changes.append(bc)

def check_property_required(self, components: Dict) -> None:
for key, value in components.get("properties", {}).items():
if not isinstance(value, dict):
continue
stable_type = self.stable[self._module_name]["class_nodes"][self._class_name]["properties"].get(key, {}).get("attr_type")
current_type = value.get("attr_type")

if (
isinstance(stable_type, str)
and stable_type.startswith("Optional[")
and isinstance(current_type, str)
and not current_type.startswith("Optional[")
):
bc = (
self.REQUIRED_PROPERTY_MSG,
BreakingChangeType.REQUIRED_PROPERTY,
self._module_name,
self._class_name,
key,
)
self.breaking_changes.append(bc)

def check_class_removed_or_renamed(self, class_components: Dict) -> Union[bool, None]:
if isinstance(self._class_name, jsondiff.Symbol):
deleted_classes = []
Expand Down Expand Up @@ -767,6 +792,11 @@ def get_reportable_changes(self, ignore_changes: Dict, changes_list: List) -> Li

should_keep = True
for suppression in suppressions:
if (
bc_type == "AddedClassMethod"
and self.is_operation_group(module_name, class_name)
):
continue
if suppression.parameter_or_property_name is not None:
# If the ignore rule is for a property or parameter, we should check up to that level on the original change
Comment thread
ChenxiJiang333 marked this conversation as resolved.
if self.match((bc_type, module_name, class_name, function_name, parameter_name), suppression):
Expand Down
103 changes: 103 additions & 0 deletions scripts/breaking_changes_checker/tests/test_changelog.py
Original file line number Diff line number Diff line change
Expand Up @@ -1137,3 +1137,106 @@ def test_added_keyword_only_param_to_model_method_still_reported_as_class():
assert msg == ChangelogTracker.ADDED_CLASS_METHOD_PARAMETER_MSG
assert args == ["azure.contoso", "ContosoModel", "extra", "do_something"]

def test_added_update_method_for_operation_group():
stable = {
"azure.mgmt.contoso.operations": {
"class_nodes": {
"ContosoOperations": {
"type": None,
"methods": {
"list": {
"parameters": {
"self": {
"default": None,
"param_type": "positional_or_keyword"
}
},
"is_async": False
}
},
"properties": {}
}
}
}
}
current = {
"azure.mgmt.contoso.operations": {
"class_nodes": {
"ContosoOperations": {
"type": None,
"methods": {
"list": {
"parameters": {
"self": {
"default": None,
"param_type": "positional_or_keyword"
}
},
"is_async": False
},
"update": {
"parameters": {
"self": {
"default": None,
"param_type": "positional_or_keyword"
}
},
"is_async": False
}
},
"properties": {}
}
}
}
}
IGNORE = {
"azure-mgmt-contoso": [
("AddedClassMethod", "*", "*", "update")
]
}
bc = ChangelogTracker(stable, current, "azure-mgmt-contoso", ignore=IGNORE)
bc.run_checks()

assert len(bc.features_added) == 1
msg, _, *args = bc.features_added[0]
assert msg == ChangelogTracker.ADDED_CLASS_METHOD_MSG
assert args == ["azure.mgmt.contoso.operations", "ContosoOperations", "update"]

def test_class_property_is_required():
stable = {
"azure.contoso.models": {
"class_nodes": {
"ContosoModel": {
"type": None,
"methods": {},
"properties": {
"foo": {
"attr_type": "Optional[Foo]"
}
}
}
}
}
}
current = {
"azure.contoso.models": {
"class_nodes": {
"ContosoModel": {
"type": None,
"methods": {},
"properties": {
"foo": {
"attr_type": "Foo"
}
}
}
}
}
}
bc = ChangelogTracker(stable, current, "azure-contoso")
bc.run_checks()

assert len(bc.breaking_changes) == 1
msg, _, *args = bc.breaking_changes[0]
assert msg == BreakingChangesTracker.REQUIRED_PROPERTY_MSG
assert args == ["azure.contoso.models", "ContosoModel", "foo"]
Loading