diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/README.md b/tools/pylint-extensions/azure-pylint-guidelines-checker/README.md index d06ab5a24f2..e9185913260 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/README.md +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/README.md @@ -96,8 +96,8 @@ In the case of a false positive, use the disable command to remove the pylint er | do-not-log-raised-errors | Do not log errors at `error` or `warning` level when error is raised in an exception block. | pylint:disable=do-not-log-raised-errors | No Link. | | do-not-use-legacy-typing | Do not use legacy (<Python 3.8) type hinting comments | pylint:disable=do-not-use-legacy-typing | No Link. | | do-not-import-asyncio | Do not import asyncio directly. | pylint:disable=do-not-import-asyncio | No Link. | +| do-not-hardcode-connection-verify | Do not hardcode a boolean value to connection_verify | pylint:disable=do-not-hardcode-connection-verify | No LInk. | | TODO | custom linter check for invalid use of @overload #3229 | | | | do-not-log-exceptions | Do not log exceptions in levels other than debug, otherwise it can reveal sensitive information | pylint:disable=do-not-log-exceptions | [link](https://azure.github.io/azure-sdk/python_implementation.html#python-logging-sensitive-info) | | unapproved-client-method-name-prefix | Clients should use preferred verbs for method names | pylint:disable=unapproved-client-method-name-prefix | [link](https://azure.github.io/azure-sdk/python_design.html#naming) | -| TODO | Add a check for connection_verify hardcoded settings #35355 | | | -| TODO | Address Commented out Pylint Custom Plugin Checkers #3228 | | | \ No newline at end of file +| TODO | Address Commented out Pylint Custom Plugin Checkers #3228 | | | diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py b/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py index 20ba2166989..4f3d125f01a 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py @@ -2959,7 +2959,80 @@ def check_for_logging(self, node, exception_name): # [Pylint] Address Commented out Pylint Custom Plugin Checkers #3228 -# [Pylint] Add a check for connection_verify hardcoded settings #35355 + + +class DoNotHardcodeConnectionVerify(BaseChecker): + + """Rule to check that developers do not hardcode a boolean to connection_verify.""" + + name = "do-not-hardcode-connection-verify" + priority = -1 + msgs = { + "C4767": ( + "Do not hardcode a boolean value to connection_verify", + "do-not-hardcode-connection-verify", + "Do not hardcode a boolean value to connection_verify. It's up to customers who use the code to be able to set it", + ), + } + + def visit_call(self, node): + """Visit function calls to ensure it isn't used as a keyword parameter""" + if len(node.keywords) > 0: + for keyword in node.keywords: + if keyword.arg == "connection_verify": + if type(keyword.value.value) == bool: + self.add_message( + msgid=f"do-not-hardcode-connection-verify", + node=keyword, + confidence=None, + ) + + def visit_assign(self, node): + """Visiting variable Assignments""" + try: # self.connection_verify = True + if node.targets[0].attrname == "connection_verify": + if type(node.value.value) == bool: + self.add_message( + msgid=f"do-not-hardcode-connection-verify", + node=node, + confidence=None, + ) + except: + try: # connection_verify = True + if node.targets[0].name == "connection_verify": + if type(node.value.value) == bool: + self.add_message( + msgid=f"do-not-hardcode-connection-verify", + node=node, + confidence=None, + ) + except: + pass # typically lists + + def visit_annassign(self, node): + """Visiting variable annotated assignments""" + try: # self.connection_verify: bool = True + if node.target.attrname == "connection_verify": + if type(node.value.value) == bool: + self.add_message( + msgid=f"do-not-hardcode-connection-verify", + node=node, + confidence=None, + ) + except: # Picks up connection_verify: bool = True + try: + if node.target.name == "connection_verify": + if type(node.value.value) == bool: + self.add_message( + msgid=f"do-not-hardcode-connection-verify", + node=node, + confidence=None, + ) + except: + pass + + + # [Pylint] Investigate pylint rule around missing dependency #3231 @@ -3002,7 +3075,7 @@ def register(linter): # [Pylint] custom linter check for invalid use of @overload #3229 # [Pylint] Address Commented out Pylint Custom Plugin Checkers #3228 - # [Pylint] Add a check for connection_verify hardcoded settings #35355 + linter.register_checker(DoNotHardcodeConnectionVerify(linter)) # [Pylint] Investigate pylint rule around missing dependency #3231 # disabled by default, use pylint --enable=check-docstrings if you want to use it diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_files/do_not_hardcode_connection_verify_acceptable.py b/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_files/do_not_hardcode_connection_verify_acceptable.py new file mode 100644 index 00000000000..095a537b0f6 --- /dev/null +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_files/do_not_hardcode_connection_verify_acceptable.py @@ -0,0 +1,54 @@ +class InstanceVariable: + def __init__(self): + self.connection_verify = None + self.self = self + + +class Variable: + connection_verify = None + + +class FunctionKeywordArgumentsErrors: + def create(connection_verify): + pass + + client = create(connection_verify=None) + + +class FunctionArgumentsInstanceErrors: + def __init__(self): + client = self.create_client_from_credential(connection_verify=None) + + +class ReturnErrorFunctionArgument: + def send(connection_verify): + pass + + def sampleFunction(self): + return self.send(connection_verify=None) + + +class ReturnErrorDict: + def returnDict(self): + + return dict( + connection_verify=None, + ) + + +class AnnotatedAssignment: + connection_verify: bool = None + + +class AnnotatedSelfAssignment: + def __init__(self): + self.connection_verify: bool = None + + +class VisitAssignPass: + connection_verify = ["apple", "banana", "cherry"] + + +class VisitAnnassignPass: + connection_verify = [0] + connection_verify[0]: int = 0 \ No newline at end of file diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_files/do_not_hardcode_connection_verify_violation.py b/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_files/do_not_hardcode_connection_verify_violation.py new file mode 100644 index 00000000000..fbf787931cc --- /dev/null +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_files/do_not_hardcode_connection_verify_violation.py @@ -0,0 +1,46 @@ +class InstanceVariableError: + def __init__(self): + self.connection_verify = True + self.self = self + + +class VariableError: + connection_verify = True + + +class FunctionKeywordArgumentsErrors: + def create(x, connection_verify): + pass + + client = create(connection_verify=False, x=0) + + +class FunctionArgumentsInstanceErrors: + def __init__(self): + client = self.create_client_from_credential(connection_verify=False) + + +class ReturnErrorFunctionArgument: + def send(connection_verify): + pass + + def sample_function(self): + return self.send(connection_verify=True) + + +class ReturnErrorDict: + def return_dict(self): + + return dict( + connection_verify=False, + ) + +class AnnotatedAssignment: + connection_verify: bool = True + + +class AnnotatedSelfAssignment: + def __init__(self): + self.connection_verify: bool = True + + diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py b/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py index c5423829feb..5b2f0133d5f 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py @@ -3724,7 +3724,144 @@ def test_guidelines_link_active(self): # [Pylint] Address Commented out Pylint Custom Plugin Checkers #3228 -# [Pylint] Add a check for connection_verify hardcoded settings #35355 + + +class TestDoNotHardcodeConnectionVerify(pylint.testutils.CheckerTestCase): + """Test that we are not hard-coding a True or False to connection_verify""" + + CHECKER_CLASS = checker.DoNotHardcodeConnectionVerify + + def test_valid_connection_verify(self): + """Check that valid connection_verify hard coding does not raise warnings""" + file = open( + os.path.join( + TEST_FOLDER, "test_files", "do_not_hardcode_connection_verify_acceptable.py" + ) + ) + node = astroid.parse(file.read()) + file.close() + + nodes = node.body + instance_variable_error = nodes[0].body[0].body[0] + variable_error = nodes[1].body[0] + function_arguments_errors = nodes[2].body[1].value + function_arguments_instance_errors = nodes[3].body[0].body[0].value + return_error_function_argument = nodes[4].body[1].body[0].value + return_error_dict = nodes[5].body[0].body[0].value + annotated_assignment = nodes[6].body[0] + annotated_self_assignment = nodes[7].body[0].body[0] + visit_assign_pass = nodes[8].body[0] + visit_annassign_pass = nodes[9].body[1] + + with self.assertNoMessages(): + self.checker.visit_assign(instance_variable_error) + self.checker.visit_assign(variable_error) + self.checker.visit_call(function_arguments_errors) + self.checker.visit_call(function_arguments_instance_errors) + self.checker.visit_call(return_error_function_argument) + self.checker.visit_call(return_error_dict) + self.checker.visit_annassign(annotated_assignment) + self.checker.visit_annassign(annotated_self_assignment) + self.checker.visit_assign(visit_assign_pass) + self.checker.visit_annassign(visit_annassign_pass) + + def test_invalid_connection_verify(self): + """Check that hard-coding connection_verify to a bool raise warnings""" + file = open( + os.path.join( + TEST_FOLDER, "test_files", "do_not_hardcode_connection_verify_violation.py" + ) + ) + node = astroid.parse(file.read()) + file.close() + + nodes = node.body + instance_variable_error = nodes[0].body[0].body[0] + variable_error = nodes[1].body[0] + function_keyword_arguments = nodes[2].body[1].value + function_arguments_instance = nodes[3].body[0].body[0].value + return_error_function_argument = nodes[4].body[1].body[0].value + return_error_dict = nodes[5].body[0].body[0].value + annotated_assignment = nodes[6].body[0] + annotated_self_assignment = nodes[7].body[0].body[0] + + with self.assertAddsMessages( + pylint.testutils.MessageTest( + msg_id="do-not-hardcode-connection-verify", + line=3, + node=instance_variable_error, + col_offset=8, + end_line=3, + end_col_offset=37, + ), + pylint.testutils.MessageTest( + msg_id="do-not-hardcode-connection-verify", + line=8, + node=variable_error, + col_offset=4, + end_line=8, + end_col_offset=28, + ), + pylint.testutils.MessageTest( + msg_id="do-not-hardcode-connection-verify", + line=15, + node=function_keyword_arguments.keywords[0], + col_offset=20, + end_line=15, + end_col_offset=43, + ), + pylint.testutils.MessageTest( + msg_id="do-not-hardcode-connection-verify", + line=20, + node=function_arguments_instance.keywords[0], + col_offset=52, + end_line=20, + end_col_offset=75, + ), + pylint.testutils.MessageTest( + msg_id="do-not-hardcode-connection-verify", + line=28, + node=return_error_function_argument.keywords[0], + col_offset=25, + end_line=28, + end_col_offset=47, + ), + pylint.testutils.MessageTest( + msg_id="do-not-hardcode-connection-verify", + line=35, + node=return_error_dict.keywords[0], + col_offset=12, + end_line=35, + end_col_offset=35, + ), + pylint.testutils.MessageTest( + msg_id="do-not-hardcode-connection-verify", + line=39, + node=annotated_assignment, + col_offset=4, + end_line=39, + end_col_offset=34, + ), + pylint.testutils.MessageTest( + msg_id="do-not-hardcode-connection-verify", + line=44, + node=annotated_self_assignment, + col_offset=8, + end_line=44, + end_col_offset=43, + ), + ): + self.checker.visit_assign(instance_variable_error) + self.checker.visit_assign(variable_error) + self.checker.visit_call(function_keyword_arguments) + self.checker.visit_call(function_arguments_instance) + self.checker.visit_call(return_error_function_argument) + self.checker.visit_call(return_error_dict) + self.checker.visit_annassign(annotated_assignment) + self.checker.visit_annassign(annotated_self_assignment) + + + # [Pylint] Refactor test suite for custom pylint checkers to use files instead of docstrings #3233 # [Pylint] Investigate pylint rule around missing dependency #3231