diff --git a/demisto_sdk/commands/validate/sdk_validation_config.toml b/demisto_sdk/commands/validate/sdk_validation_config.toml index beda871171..e1d8acb4e6 100644 --- a/demisto_sdk/commands/validate/sdk_validation_config.toml +++ b/demisto_sdk/commands/validate/sdk_validation_config.toml @@ -25,7 +25,7 @@ select = [ "BA100", "BA101", "BA105", "BA106", "BA110", "BA111", "BA114", "BA116", "BA118", "BA119", "BA124", "BA126", "PA100", "PA101", "PA102", "PA103", "PA104", "PA105", "PA107", "PA108", "PA109", "PA111", "PA113", "PA115", "PA117", "PA118", "PA119", "PA120", "PA121", "PA123", "PA125", "PA127", "PA128", "PA130", - "BC100", "BC101", "BC102", "BC103", "BC104", "BC105", "BC108", "BC110", + "BC100", "BC101", "BC102", "BC103", "BC104", "BC105", "BC108", "BC110", "BC111", "IN100", "IN101", "IN102", "IN104", "IN106", "IN107", "IN108", "IN109", "IN110", "IN112", "IN113", "IN114", "IN115", "IN117", "IN118", "IN121", "IN122", "IN123", "IN124", "IN125", "IN126", "IN127", "IN130", "IN131", "IN134", "IN135", "IN139", "IN141", "IN142", "IN144", "IN145", "IN146", "IN149", "IN150", "IN151", "IN152", "IN153", "IN154", "IN156", "IN158", "IN159", "IN160", "IN161", "IN162", diff --git a/demisto_sdk/commands/validate/tests/BC_validators_test.py b/demisto_sdk/commands/validate/tests/BC_validators_test.py index 46223275fe..90fa697eed 100644 --- a/demisto_sdk/commands/validate/tests/BC_validators_test.py +++ b/demisto_sdk/commands/validate/tests/BC_validators_test.py @@ -46,6 +46,9 @@ from demisto_sdk.commands.validate.validators.BC_validators.BC110_new_required_argument_integration import ( NewRequiredArgumentIntegrationValidator, ) +from demisto_sdk.commands.validate.validators.BC_validators.BC111_new_required_argument_script import ( + NewRequiredArgumentScriptValidator, +) from TestSuite.repo import ChangeCWD ALL_MARKETPLACES = list(MarketplaceVersions) @@ -1224,3 +1227,88 @@ def test_NewRequiredArgumentValidator__passes(): create_old_file_pointers([new_content_item], [old_content_item]) assert not NewRequiredArgumentIntegrationValidator().is_valid([new_content_item]) + + +@pytest.mark.parametrize( + "new_args, breaking_arg", + [ + pytest.param( + [{"name": "arg1", "required": False}, {"name": "arg2", "required": True}], + "arg2", + id="new required arg", + ), + pytest.param( + [ + {"name": "arg1", "required": True}, + ], + "arg1", + id="changed to required", + ), + ], +) +def test_NewRequiredArgumentScriptValidator__fails(new_args, breaking_arg): + """ + Given: + - An older version of a script that has one non-required argument. + - A newer version of the same script where an argument is now required. This can occur in two cases: + Case 1: The required argument is a new addition to the script. + Case 2: An existing argument from the older version has been updated to be required in the new version. + When: + - The NewRequiredArgumentScriptValidator is invoked to validate the changes in the script. + Then: + - The validation should fail because a non-required argument has been made required. The error message should correctly identify the argument that caused the validation to fail. + """ + new_content_item = create_script_object( + paths=["args"], + values=[new_args], + ) + old_content_item = create_script_object(paths=["args"], values=[[{"name": "arg1"}]]) + + create_old_file_pointers([new_content_item], [old_content_item]) + res = NewRequiredArgumentScriptValidator().is_valid([new_content_item]) + assert breaking_arg in res[0].message + + +@pytest.mark.parametrize( + "new_args", + [ + pytest.param( + [ + {"name": "arg1", "required": False}, + {"name": "arg2", "required": False}, + ], + id="non required args", + ), + pytest.param( + [ + {"name": "arg1", "required": True}, + {"name": "arg2", "required": True, "defaultvalue": "test"}, + ], + id="required with default value", + ), + ], +) +def test_NewRequiredArgumentScriptValidator__passes(new_args): + """ + Given: + - An older version of a script that has two arguments: one required and one not required. + - A newer version of the same script in two cases: + Case 1: Both arguments are not required. + Case 2: Both arguments are required, but the second one has a default value. + When: + - The NewRequiredArgumentScriptValidator is invoked to validate the changes in the script. + Then: + - The validation should pass in both cases: + Case 1: The first argument was changed to be not required, which is allowed. + Case 2: The first argument did not change, and the second argument was changed to be required but has a default value, which is allowed. + """ + new_content_item = create_script_object( + paths=["args"], + values=[new_args], + ) + old_content_item = create_script_object( + paths=["args"], values=[[{"name": "arg1", "required": True}, {"name": "arg2"}]] + ) + + create_old_file_pointers([new_content_item], [old_content_item]) + assert not NewRequiredArgumentScriptValidator().is_valid([new_content_item]) diff --git a/demisto_sdk/commands/validate/validators/BC_validators/BC111_new_required_argument_script.py b/demisto_sdk/commands/validate/validators/BC_validators/BC111_new_required_argument_script.py new file mode 100644 index 0000000000..fccec6fbef --- /dev/null +++ b/demisto_sdk/commands/validate/validators/BC_validators/BC111_new_required_argument_script.py @@ -0,0 +1,59 @@ +from __future__ import annotations + +from typing import Iterable, List + +from demisto_sdk.commands.common.constants import GitStatuses +from demisto_sdk.commands.content_graph.objects.script import Script +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + ValidationResult, +) + +ContentTypes = Script + +# This validation is similar to BC110, but specifically for scripts. +class NewRequiredArgumentScriptValidator(BaseValidator[ContentTypes]): + error_code = "BC111" + description = ( + "Ensure that no new *required* arguments are added to an existing script." + ) + rationale = "Adding a new required argument or changing a non-required one to required without specifying a default value breaks backward compatibility." + error_message = "Possible backward compatibility break: You have added the following new *required* arguments: {arg_list}. Please undo the changes." + related_field = "script.arguments" + is_auto_fixable = False + expected_git_statuses = [GitStatuses.MODIFIED] + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + results: List[ValidationResult] = [] + arg_list = [] + for content_item in content_items: + old_content_item = content_item.old_base_content_object + + arg_list = [ + arg.name + for arg in content_item.args # type: ignore + if arg.required + and not arg.defaultvalue + and ( + not next( + ( + old_arg + for old_arg in old_content_item.args # type: ignore + if old_arg.name == arg.name and old_arg.required + ), + None, + ) + ) + ] + if arg_list: + results.append( + ValidationResult( + validator=self, + message=self.error_message.format( + arg_list=", ".join(f'"{w}"' for w in arg_list) + ), + content_object=content_item, + ) + ) + + return results