Do not hardcode boolean connection verify v2 (#9118)

* Version 2 Checker Duplicated

* Function Positiona lArguments Added

* positional arg check removed

Author: 16234397

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                                                                                                            |                                                                     |                                                                                                         |
+| TODO                                               | Address Commented out Pylint Custom Plugin Checkers #3228                                                                                                            |                                                                     |                                                                                                         |

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

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

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
+
+

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