From f236d0b7431f1d3d431fb5f959f8acb3773c93d7 Mon Sep 17 00:00:00 2001 From: Krista Pratico Date: Wed, 20 Apr 2022 14:30:37 -0700 Subject: [PATCH 1/4] Initial code move from Azure/azure-sdk-for-python --- .../pylint-guidelines-checker/__init__.py | 0 .../pylint_guidelines_checker.py | 1737 +++++++++++ .../pylint-guidelines-checker/setup.py | 12 + .../tests/test_pylint_custom_plugins.py | 2570 +++++++++++++++++ 4 files changed, 4319 insertions(+) create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/__init__.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/pylint_guidelines_checker.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/setup.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py diff --git a/tools/pylint-extensions/pylint-guidelines-checker/__init__.py b/tools/pylint-extensions/pylint-guidelines-checker/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tools/pylint-extensions/pylint-guidelines-checker/pylint_guidelines_checker.py b/tools/pylint-extensions/pylint-guidelines-checker/pylint_guidelines_checker.py new file mode 100644 index 00000000000..e62a6829625 --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/pylint_guidelines_checker.py @@ -0,0 +1,1737 @@ +# ------------------------------------ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. +# ------------------------------------ + +""" +Pylint custom checkers for SDK guidelines: C4717 - C4744 +""" + +import logging +import astroid +from pylint.checkers import BaseChecker +from pylint.interfaces import IAstroidChecker +logger = logging.getLogger(__name__) + + +class ClientConstructorTakesCorrectParameters(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-constructor" + priority = -1 + msgs = { + "C4717": ( + "Client constructor is missing a credential parameter. See details:" + " https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods", + "missing-client-constructor-parameter-credential", + "All client types should accept a credential parameter.", + ), + "C4718": ( + "Client constructor is missing a **kwargs parameter. See details:" + " https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods", + "missing-client-constructor-parameter-kwargs", + "All client types should accept a **kwargs parameter.", + ) + } + options = ( + ( + "ignore-missing-client-constructor-parameter-credential", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client constructors without a credential parameter", + }, + ), + ( + "ignore-missing-client-constructor-parameter-kwargs", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client constructors without a **kwargs parameter", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientConstructorTakesCorrectParameters, self).__init__(linter) + + def visit_functiondef(self, node): + """Visits the constructor within a client class and checks that it has + credential and kwargs parameters. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.name == "__init__" and node.parent.name.endswith("Client") and \ + node.parent.name not in self.ignore_clients: + arg_names = [argument.name for argument in node.args.args] + if "credential" not in arg_names: + self.add_message( + msgid="missing-client-constructor-parameter-credential", node=node, confidence=None + ) + if not node.args.kwarg: + self.add_message( + msgid="missing-client-constructor-parameter-kwargs", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if constructor has correct parameters.") + pass + + +class ClientHasKwargsInPoliciesForCreateConfigurationMethod(BaseChecker): + __implements__ = IAstroidChecker + + name = "configuration-policies-kwargs" + priority = -1 + msgs = { + "C4719": ( + "A policy in the create_configuration() function is missing a **kwargs argument. See details:" + " https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods", + "config-missing-kwargs-in-policy", + "All policies should take a **kwargs parameter.", + ) + } + options = ( + ( + "ignore-config-missing-kwargs-in-policy", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow clients instantiate a policy without a kwargs parameter.", + }, + ), + ) + + def __init__(self, linter=None): + super(ClientHasKwargsInPoliciesForCreateConfigurationMethod, self).__init__(linter) + + def visit_functiondef(self, node): + """Visits the any method called `create_configuration` or `create_config` and checks + that every policy in the method contains a kwargs parameter. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.name == "create_configuration" or node.name == "create_config": + node.decorators = None + for idx in range(len(node.body)): + # Gets each line of the method as a string + line = list(node.get_children())[idx].as_string() + if line.find("Policy") != -1: + if line.find("**kwargs") == -1: + self.add_message( + msgid="config-missing-kwargs-in-policy", + node=list(node.get_children())[idx], + confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if kwargs parameter in policies.") + pass + + +class ClientHasApprovedMethodNamePrefix(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-approved-method-name-prefix" + priority = -1 + msgs = { + "C4720": ( + "Client is not using an approved method name prefix. See details:" + " https://azure.github.io/azure-sdk/python_design.html#service-operations", + "unapproved-client-method-name-prefix", + "All clients should use the preferred verbs for method names.", + ) + } + options = ( + ( + "ignore-unapproved-client-method-name-prefix", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow clients to not use preferred method name prefixes", + }, + ), + ) + + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientHasApprovedMethodNamePrefix, self).__init__(linter) + + def visit_classdef(self, node): + """Visits every class in file and checks if it is a client. If it is a client, checks + that approved method name prefixes are present. + + :param node: class node + :type node: ast.ClassDef + :return: None + """ + try: + if node.name.endswith("Client") and node.name not in self.ignore_clients: + client_methods = [child for child in node.get_children() if child.is_function] + + approved_prefixes = ["get", "list", "create", "upsert", "set", "update", "replace", "append", "add", + "delete", "remove", "begin"] + for idx, method in enumerate(client_methods): + if method.name.startswith("__") or "_exists" in method.name or method.name.startswith("_") \ + or method.name.startswith("from"): + continue + prefix = method.name.split("_")[0] + if prefix.lower() not in approved_prefixes: + self.add_message( + msgid="unapproved-client-method-name-prefix", + node=client_methods[idx], + confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client has approved method name prefix.") + pass + + +class ClientMethodsUseKwargsWithMultipleParameters(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-method-multiple-parameters" + priority = -1 + msgs = { + "C4721": ( + "Client has too many positional arguments. Use keyword-only arguments." + " See details: https://azure.github.io/azure-sdk/python_introduction.html#method-signatures", + "client-method-has-more-than-5-positional-arguments", + "Client method should use keyword-only arguments for some parameters.", + ) + } + options = ( + ( + "ignore-client-method-has-more-than-5-positional-arguments", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client method to have more than 5 positional arguments", + }, + ), + ) + + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientMethodsUseKwargsWithMultipleParameters, self).__init__(linter) + + def visit_functiondef(self, node): + """Visits every method in the client and checks that it doesn't have more than 5 + positional arguments. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.parent.name.endswith("Client") and node.is_method() and node.parent.name not in self.ignore_clients: + # Only bother checking method signatures with > 6 parameters (don't include self/cls/etc) + if len(node.args.args) > 6: + positional_args = len(node.args.args) - len(node.args.defaults) + if positional_args > 6: + self.add_message( + msgid="client-method-has-more-than-5-positional-arguments", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if kwargs is used for multiple parameters.") + pass + + visit_asyncfunctiondef = visit_functiondef + + +class ClientMethodsHaveTypeAnnotations(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-method-type-annotations" + priority = -1 + msgs = { + "C4722": ( + "Client method is missing type annotations/comments, return type annotations/comments, or " + "mixing type annotations and comments. See details: " + " https://azure.github.io/azure-sdk/python_introduction.html#types-or-not", + "client-method-missing-type-annotations", + "Client method should use type annotations.", + ) + } + options = ( + ( + "ignore-client-method-missing-type-annotations", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client method without type annotations", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientMethodsHaveTypeAnnotations, self).__init__(linter) + + def visit_functiondef(self, node): + """Visits every method in the client and checks that all type comments/annotations + and type returns are present. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.parent.name.endswith("Client") and node.is_method() and node.parent.name not in self.ignore_clients: + if not node.name.startswith("_") or node.name == "__init__": + # Checks that method has python 2/3 type comments or annotations as shown here: + # https://www.python.org/dev/peps/pep-0484/#suggested-syntax-for-python-2-7-and-straddling-code + + # check for type comments + if node.type_comment_args is None or node.type_comment_returns is None: + + # type annotations default to a list of None when not present, + # so need extra logic here to check for any hints that may be present + type_annotations = [type_hint for type_hint in node.args.annotations if type_hint is not None] + + # check for type annotations + # node.args.args is a list of ast.AssignName arguments + # node.returns is the type annotation return + # Note that if the method returns nothing it will be of type ast.Const.NoneType + if (type_annotations == [] and len(node.args.args) > 1) or node.returns is None: + self.add_message( + msgid="client-method-missing-type-annotations", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client methods missing type annotations.") + pass + + visit_asyncfunctiondef = visit_functiondef + + +class ClientMethodsHaveTracingDecorators(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-method-has-tracing-decorator" + priority = -1 + msgs = { + "C4723": ( + "Client method is missing the distributed tracing decorator - `distributed_trace`. See details:" + " https://azure.github.io/azure-sdk/python_implementation.html#distributed-tracing", + "client-method-missing-tracing-decorator", + "Client method should support distributed tracing.", + ), + "C4724": ( + "Client async method is missing the distributed tracing decorator - `distributed_trace_async`. " + " See details: https://azure.github.io/azure-sdk/python_implementation.html#distributed-tracing", + "client-method-missing-tracing-decorator-async", + "Client method should support distributed tracing.", + ), + } + options = ( + ( + "ignore-client-method-missing-tracing-decorator", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client method without tracing decorator.", + }, + ), + ( + "ignore-client-method-missing-tracing-decorator-async", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client method without tracing decorator.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientMethodsHaveTracingDecorators, self).__init__(linter) + + def visit_functiondef(self, node): + """Visits every method in the client and checks that a distributed tracing decorator is present. + Ignores private methods, from_connection_string, and methods that retrieve child clients. + + node.decoratornames() returns a set of the method's decorator names. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.parent.name.endswith("Client") and node.is_method() and not node.name.startswith("_") and \ + node.parent.name not in self.ignore_clients: + if node.args.kwarg and "azure.core.tracing.decorator.distributed_trace" not in node.decoratornames() \ + and "builtins.classmethod" not in node.decoratornames(): + self.add_message( + msgid="client-method-missing-tracing-decorator", node=node, confidence=None + ) + except AttributeError: + pass + + def visit_asyncfunctiondef(self, node): + """Visits every method in the client and checks that a distributed tracing decorator is present. + Ignores private methods, from_connection_string, and methods that retrieve child clients. + + node.decoratornames() returns a set of the method's decorator names. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.parent.name.endswith("Client") and node.is_method() and not node.name.startswith("_") and \ + node.parent.name not in self.ignore_clients: + if node.args.kwarg and "azure.core.tracing.decorator_async.distributed_trace_async" not in \ + node.decoratornames() and "builtins.classmethod" not in node.decoratornames(): + self.add_message( + msgid="client-method-missing-tracing-decorator-async", node=node, confidence=None + ) + except AttributeError: + pass + + +class ClientsDoNotUseStaticMethods(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-does-not-use-static-methods" + priority = -1 + msgs = { + "C4725": ( + "Client should not use static methods (staticmethod). See details:" + " https://azure.github.io/azure-sdk/python_introduction.html#method-signatures", + "client-method-should-not-use-static-method", + "Client method should not use staticmethod.", + ), + } + options = ( + ( + "ignore-client-method-should-not-use-static-method", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client method to use staticmethod.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientsDoNotUseStaticMethods, self).__init__(linter) + + def visit_functiondef(self, node): + """Visits every method in the client and checks that it does not use staticmethod. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.parent.name.endswith("Client") and node.is_method() and node.parent.name not in self.ignore_clients: + # ignores private methods or methods that don't have any decorators + if not node.name.startswith("_") and node.decorators is not None: + if "builtins.staticmethod" in node.decoratornames(): + self.add_message( + msgid="client-method-should-not-use-static-method", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client methods do not use staticmethods.") + pass + + visit_asyncfunctiondef = visit_functiondef + + +class FileHasCopyrightHeader(BaseChecker): + __implements__ = IAstroidChecker + + name = "file-has-copyright-header" + priority = -1 + msgs = { + "C4726": ( + "File is missing a copyright header. See details:" + " https://azure.github.io/azure-sdk/policies_opensource.html", + "file-needs-copyright-header", + "Every source file should have a copyright header.", + ), + } + options = ( + ( + "ignore-file-needs-copyright-header", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow file without a copyright header.", + }, + ), + ) + + def __init__(self, linter=None): + super(FileHasCopyrightHeader, self).__init__(linter) + + def visit_module(self, node): + """Visits every file and checks that a copyright header is present. + + :param node: module node + :type node: ast.Module + :return: None + """ + try: + if not node.package: # don't throw an error on an __init__.py file + header = node.stream().read(200).lower() + if header.find(b'copyright') == -1: + self.add_message( + msgid="file-needs-copyright-header", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if file is missing a copyright header.") + pass + + +class ClientUsesCorrectNamingConventions(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-naming-conventions" + priority = -1 + msgs = { + "C4727": ( + "Client is using an incorrect naming convention. See details:" + " https://azure.github.io/azure-sdk/python_introduction.html#naming-conventions", + "client-incorrect-naming-convention", + "Client method should use correct naming conventions.", + ) + } + options = ( + ( + "ignore-client-incorrect-naming-convention", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client to use incorrect naming conventions.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientUsesCorrectNamingConventions, self).__init__(linter) + + def visit_classdef(self, node): + """Visits every class in file and checks if it is a client. + Checks that correct naming convention is used for the client. + Also checks that any class constants use uppercase. + + :param node: class node + :type node: ast.ClassDef + :return: None + """ + # check for correct capitalization for "Client" and whatever the first letter of the prefix is + if "_" in node.name or node.name.endswith("client") or node.name[0] != node.name[0].upper(): + if not node.name.startswith("_") and node.name not in self.ignore_clients: + self.add_message( + msgid="client-incorrect-naming-convention", node=node, confidence=None + ) + + # check for correct naming convention in any class constants + if node.name.endswith("Client"): + for idx in range(len(node.body)): + try: + const_name = node.body[idx].targets[0].name + if const_name != const_name.upper(): + self.add_message( + msgid="client-incorrect-naming-convention", node=node.body[idx], confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client uses correct naming conventions.") + pass + + # check that methods in client class do not use camelcase + try: + for func in node.body: + if func.name != func.name.lower() and not func.name.startswith("_"): + self.add_message( + msgid="client-incorrect-naming-convention", node=func, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client uses correct naming conventions.") + pass + + +class ClientMethodsHaveKwargsParameter(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-methods-have-kwargs" + priority = -1 + msgs = { + "C4728": ( + "Client method is missing a **kwargs parameter. See details:" + " https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods", + "client-method-missing-kwargs", + "All client methods should accept a kwargs parameter.", + ), + } + options = ( + ( + "ignore-client-method-missing-kwargs", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client method without a kwargs parameter", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientMethodsHaveKwargsParameter, self).__init__(linter) + + def visit_functiondef(self, node): + """Visits every method in the client and checks that it has a kwargs parameter. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.parent.name.endswith("Client") and node.is_method() and node.parent.name not in self.ignore_clients: + # avoid false positive with @property + if node.decorators is not None: + if "builtins.property" in node.decoratornames(): + return + if not node.name.startswith("_") and \ + ("azure.core.tracing.decorator.distributed_trace" in node.decoratornames() or + "azure.core.tracing.decorator_async.distributed_trace_async" in node.decoratornames()): + if not node.args.kwarg: + self.add_message( + msgid="client-method-missing-kwargs", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client uses kwargs parameter in method.") + pass + + visit_asyncfunctiondef = visit_functiondef + + +class ClientMethodNamesDoNotUseDoubleUnderscorePrefix(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-methods-no-double-underscore" + priority = -1 + msgs = { + "C4729": ( + "Client method name should not use a double underscore prefix. See details:" + " https://azure.github.io/azure-sdk/python_introduction.html#public-vs-private", + "client-method-name-no-double-underscore", + "Client method names should not use a leading double underscore prefix.", + ), + } + options = ( + ( + "ignore-client-method-name-no-double-underscore", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client method to have double underscore prefix.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + acceptable_names = ["__init__", "__enter__", "__exit__", "__aenter__", "__aexit__", "__repr__"] + + def __init__(self, linter=None): + super(ClientMethodNamesDoNotUseDoubleUnderscorePrefix, self).__init__(linter) + + def visit_functiondef(self, node): + """Visits every method in the client and checks that no name begins with a double underscore. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.parent.name.endswith("Client") and node.is_method() and node.parent.name not in self.ignore_clients: + if node.name.startswith("__") and node.name not in self.acceptable_names: + self.add_message( + msgid="client-method-name-no-double-underscore", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client method name does not use double underscore prefix.") + pass + + visit_asyncfunctiondef = visit_functiondef + + +class ClientDocstringUsesLiteralIncludeForCodeExample(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-docstring-literal-include" + priority = -1 + msgs = { + "C4730": ( + "Client docstring should use a literal include directive for the code example. See details:" + " https://azure.github.io/azure-sdk/python_documentation.html#code-snippets", + "client-docstring-use-literal-include", + "Client/methods should use literal include directives for code examples.", + ), + } + options = ( + ( + "ignore-client-docstring-use-literal-include", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client to use code block.", + }, + ), + ) + + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientDocstringUsesLiteralIncludeForCodeExample, self).__init__(linter) + + def visit_classdef(self, node): + """Visits every class in file and checks if it is a client. + Also checks that the class constructor uses literalinclude over a code-block for the code example. + + :param node: class node + :type node: ast.ClassDef + :return: None + """ + try: + if node.name.endswith("Client") and node.name not in self.ignore_clients: + if node.doc.find("code-block") != -1: + self.add_message( + msgid="client-docstring-use-literal-include", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client uses literalinclude over code-block.") + pass + + def visit_functiondef(self, node): + """Visits every method in the client class and checks that it uses literalinclude + over a code-block for the code example. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.parent.name.endswith("Client") and node.parent.name not in self.ignore_clients and node.is_method(): + if node.doc.find("code-block") != -1: + self.add_message( + msgid="client-docstring-use-literal-include", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client uses literalinclude over code-block.") + pass + + visit_asyncfunctiondef = visit_functiondef + + +class AsyncClientCorrectNaming(BaseChecker): + __implements__ = IAstroidChecker + + name = "async-client-correct-naming" + priority = -1 + msgs = { + "C4731": ( + "Async client should not include `Async` in the client name. See details:" + " https://azure.github.io/azure-sdk/python_design.html#async-support", + "async-client-bad-name", + "Async clients should not have async in the name.", + ), + } + options = ( + ( + "ignore-async-client-bad-name", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow async client to include async in its name.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(AsyncClientCorrectNaming, self).__init__(linter) + + def visit_classdef(self, node): + """Visits every class in file and checks that an async client does not use + async in its name. + + :param node: class node + :type node: ast.ClassDef + :return: None + """ + try: + # avoid false positive when async name is used with a base class. + if node.name.endswith("Client") and "async" in node.name.lower() and "base" not in node.name.lower(): + if not node.name.startswith("_") and node.name not in self.ignore_clients: + self.add_message( + msgid="async-client-bad-name", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if async client uses correct naming.") + pass + + +class SpecifyParameterNamesInCall(BaseChecker): + __implements__ = IAstroidChecker + + name = "specify-parameter-names" + priority = -1 + msgs = { + "C4732": ( + "Specify the parameter names when calling methods with more than 2 required positional parameters." + " See details: https://azure.github.io/azure-sdk/python_introduction.html#method-signatures", + "specify-parameter-names-in-call", + "You should specify the parameter names when the method has more than two positional arguments.", + ) + } + options = ( + ( + "ignore-specify-parameter-names-in-call", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Call the method without specifying parameter names.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(SpecifyParameterNamesInCall, self).__init__(linter) + + def visit_call(self, node): + """Visits every call in the client and checks that it specifies the parameter name in + the call if there are more than 2 require positional parameters. + + :param node: call node + :type node: ast.Call + :return: None + """ + try: + klass = node.parent.parent.parent + function = node.parent.parent + if klass.name.endswith("Client") and klass.name not in self.ignore_clients and function.is_method(): + # node.args represent positional arguments + if len(node.args) > 2 and node.func.attrname != "format": + self.add_message( + msgid="specify-parameter-names-in-call", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client methods specify parameters name in call.") + pass + + +class ClientListMethodsUseCorePaging(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-list-methods-use-paging" + priority = -1 + msgs = { + "C4733": ( + "Operations that return collections should return a value that implements the Paging protocol. See details:" + " https://azure.github.io/azure-sdk/python_design.html#response-formats", + "client-list-methods-use-paging", + "Client methods that return collections should use the Paging protocol.", + ), + } + options = ( + ( + "ignore-client-list-methods-use-paging", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow collections method to not use paging protocol.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientListMethodsUseCorePaging, self).__init__(linter) + + def visit_functiondef(self, node): + """Visits every method in the client and checks that any list_ methods return + an ItemPaged or AsyncItemPaged value. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.parent.name.endswith("Client") and node.parent.name not in self.ignore_clients and node.is_method(): + if node.name.startswith("list"): + try: + # infer_call_result gives the method return value as a string + returns = next(node.infer_call_result()).as_string() + if returns.find("ItemPaged") == -1 and returns.find("AsyncItemPaged") == -1: + self.add_message( + msgid="client-list-methods-use-paging", node=node, confidence=None + ) + except (astroid.exceptions.InferenceError, AttributeError): # astroid can't always infer the return + logger.debug("Pylint custom checker failed to check if client list method uses core paging.") + pass + except AttributeError: + logger.debug("Pylint custom checker failed to check if client list method uses core paging.") + pass + + +class ClientLROMethodsUseCorePolling(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-lro-methods-use-polling" + priority = -1 + msgs = { + "C4734": ( + "Long running operations should return a value that implements the Poller protocol. See details:" + " https://azure.github.io/azure-sdk/python_design.html#response-formats", + "client-lro-methods-use-polling", + "Long running operations should use the polling protocol.", + ), + } + options = ( + ( + "ignore-client-lro-methods-use-polling", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow LRO method to not use polling protocol.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientLROMethodsUseCorePolling, self).__init__(linter) + + def visit_functiondef(self, node): + """Visits every method in the client and checks that any begin_ methods return + an LROPoller value. + + :param node: function node + :type node: ast.FunctionDef + :return: None + """ + try: + if node.parent.name.endswith("Client") and node.parent.name not in self.ignore_clients and node.is_method(): + if node.name.startswith("begin"): + try: + # infer_call_result gives the method return value as a string + returns = next(node.infer_call_result()).as_string() + if returns.find("LROPoller") == -1: + self.add_message( + msgid="client-lro-methods-use-polling", node=node, confidence=None + ) + except (astroid.exceptions.InferenceError, AttributeError): # astroid can't always infer the return + logger.debug("Pylint custom checker failed to check if client begin method uses core polling.") + pass + except AttributeError: + logger.debug("Pylint custom checker failed to check if client begin method uses core polling.") + pass + + +class ClientLROMethodsUseCorrectNaming(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-lro-methods-use-correct-naming" + priority = -1 + msgs = { + "C4735": ( + "Methods that return an LROPoller should be prefixed with `begin_`. See details:" + " https://azure.github.io/azure-sdk/python_design.html#service-operations", + "lro-methods-use-correct-naming", + "Methods that return an LROPoller should be prefixed with `begin_`.", + ), + } + options = ( + ( + "ignore-client-lro-methods-use-correct-naming", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow LRO method to use a different name.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientLROMethodsUseCorrectNaming, self).__init__(linter) + self.is_client = [] + + def visit_classdef(self, node): + """Visits every class in file and checks if it is a client. + + :param node: class node + :type node: ast.ClassDef + :return: None + """ + if node.name.endswith("Client") and node.name not in self.ignore_clients: + self.is_client.append(True) + else: + self.is_client.append(False) + + def visit_return(self, node): + if self.is_client and self.is_client[-1]: + try: + # check for a return value of LROPoller in client class + if node.value.func.name == "LROPoller": + # get the method in which LROPoller is returned + method = node.value.func.scope() + if not method.name.startswith("begin") and not method.name.startswith("_"): + self.add_message( + msgid="lro-methods-use-correct-naming", node=method, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client method with polling uses correct naming.") + pass + + +class ClientConstructorDoesNotHaveConnectionStringParam(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-conn-str-not-in-constructor" + priority = -1 + msgs = { + "C4736": ( + "The constructor must not take a connection string. See details: " + "https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods", + "connection-string-should-not-be-constructor-param", + "Client should have a method to create the client with a connection string.", + ), + } + options = ( + ( + "ignore-connection-string-should-not-be-constructor-param", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow client to use connection string param in constructor.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(ClientConstructorDoesNotHaveConnectionStringParam, self).__init__(linter) + + def visit_classdef(self, node): + """Visits every class in file and checks if it is a client. + If it is a client, it checks that a connection string parameter is not used in the constructor. + + :param node: class node + :type node: ast.ClassDef + :return: None + """ + try: + if node.name.endswith("Client") and node.name not in self.ignore_clients: + for func in node.body: + if func.name == "__init__": + for argument in func.args.args: + if argument.name == "connection_string" or argument.name == "conn_str": + self.add_message( + msgid="connection-string-should-not-be-constructor-param", node=node, confidence=None + ) + except AttributeError: + logger.debug("Pylint custom checker failed to check if client uses connection string param in constructor.") + pass + + +class PackageNameDoesNotUseUnderscoreOrPeriod(BaseChecker): + __implements__ = IAstroidChecker + + name = "package-name-incorrect" + priority = -1 + msgs = { + "C4737": ( + "Package name should not use an underscore or period. Replace with dash (-). See details: " + "https://azure.github.io/azure-sdk/python_implementation.html#packaging", + "package-name-incorrect", + "Package name should use dashes instead of underscore or period.", + ), + } + options = ( + ( + "ignore-package-name-incorrect", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow package name to have a different naming convention.", + }, + ), + ) + + def __init__(self, linter=None): + super(PackageNameDoesNotUseUnderscoreOrPeriod, self).__init__(linter) + + def visit_module(self, node): + """Visits setup.py and checks that its package name follows correct naming convention. + + :param node: module node + :type node: ast.Module + :return: None + """ + try: + if node.file.endswith("setup.py"): + for nod in node.body: + if isinstance(nod, astroid.Assign): + if nod.targets[0].name == "PACKAGE_NAME": + package = nod.value + if package.value.find(".") != -1 or package.value.find("_") != -1: + self.add_message( + msgid="package-name-incorrect", node=node, confidence=None + ) + except Exception: + logger.debug("Pylint custom checker failed to check if package name is correct.") + pass + + +class ServiceClientUsesNameWithClientSuffix(BaseChecker): + __implements__ = IAstroidChecker + + name = "client-name-incorrect" + priority = -1 + msgs = { + "C4738": ( + "Service client types should use a `Client` suffix. See details: " + "https://azure.github.io/azure-sdk/python_design.html#clients", + "client-suffix-needed", + "Client should use the correct suffix.", + ), + } + options = ( + ( + "ignore-client-suffix-needed", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow the client to have a different suffix.", + }, + ), + ) + + def __init__(self, linter=None): + super(ServiceClientUsesNameWithClientSuffix, self).__init__(linter) + + def visit_module(self, node): + """Visits a file that has "client" in the file name and checks that the service client + uses a `Client` suffix. + + :param node: module node + :type node: ast.Module + :return: None + """ + try: + # ignore base clients + if node.file.endswith("base_client.py") or node.file.endswith("base_client_async.py"): + return + if node.file.endswith("client.py") or node.file.endswith("client_async.py"): + has_client_suffix = False + for idx in range(len(node.body)): + if isinstance(node.body[idx], astroid.ClassDef): + if node.body[idx].name.endswith("Client"): + has_client_suffix = True + if has_client_suffix is False: + self.add_message( + msgid="client-suffix-needed", node=node, confidence=None + ) + except Exception: + logger.debug("Pylint custom checker failed to check if service client has a client suffix.") + pass + + +class CheckDocstringParameters(BaseChecker): + __implements__ = IAstroidChecker + + name = "check-docstrings" + priority = -1 + msgs = { + "C4739": ( + 'Params missing in docstring: "%s". See details: ' + 'https://azure.github.io/azure-sdk/python_documentation.html#docstrings', + "docstring-missing-param", + "Docstring missing for param.", + ), + "C4740": ( + 'Param types missing in docstring: "%s". See details: ' + 'https://azure.github.io/azure-sdk/python_documentation.html#docstrings', + "docstring-missing-type", + "Docstring missing for param type.", + ), + "C4741": ( + "A return doc is missing in the docstring. See details: " + "https://azure.github.io/azure-sdk/python_documentation.html#docstrings", + "docstring-missing-return", + "Docstring missing for return doc.", + ), + "C4742": ( + "A return type is missing in the docstring. See details: " + "https://azure.github.io/azure-sdk/python_documentation.html#docstrings", + "docstring-missing-rtype", + "Docstring missing for return type.", + ), + "C4743": ( + '"%s" not found as a parameter. Use :keyword type myarg: if a keyword argument. See details: ' + 'https://azure.github.io/azure-sdk/python_documentation.html#docstrings', + "docstring-should-be-keyword", + "Docstring should use keywords.", + ), + } + options = ( + ( + "ignore-docstring-missing-param", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow a docstring param mismatch.", + }, + ), + ( + "ignore-docstring-missing-type", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow a docstring param type mismatch.", + }, + ), + ( + "ignore-docstring-missing-return", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow a docstring return doc mismatch", + }, + ), + ( + "ignore-docstring-missing-rtype", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow a docstring rtype mismatch", + }, + ), + ( + "ignore-docstring-should-be-keyword", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow a docstring to not use keyword for documentation.", + }, + ), + ) + + def __init__(self, linter=None): + super(CheckDocstringParameters, self).__init__(linter) + + def check_parameters(self, node): + """Parse the docstring for any params and types + and compares it to the function's parameters. + + Throws a pylint error if... + 1. Missing param in docstring. + 2. Missing a param type in the docstring. + 3. Missing a return doc in the docstring when a function returns something. + 4. Missing an rtype in the docstring when a function returns something. + 5. Extra params in docstring that aren't function parameters. Change to keywords. + + :param node: ast.ClassDef or ast.FunctionDef + :return: None + """ + arg_names = [] + # specific case for constructor where docstring found in class def + if isinstance(node, astroid.ClassDef): + for constructor in node.body: + if isinstance(constructor, astroid.FunctionDef) and constructor.name == "__init__": + arg_names = [arg.name for arg in constructor.args.args] + break + + if isinstance(node, astroid.FunctionDef): + arg_names = [arg.name for arg in node.args.args] + + try: + # not every method will have a docstring so don't crash here, just return + docstring = node.doc.split(":") + except AttributeError: + return + + docparams = {} + for idx, line in enumerate(docstring): + # this param has its type on a separate line + if line.startswith("param") and line.count(" ") == 1: + param = line.split("param ")[1] + docparams[param] = None + # this param has its type on the same line + if line.startswith("param") and line.count(" ") == 2: + _, param_type, param = line.split(" ") + docparams[param] = param_type + if line.startswith("type"): + param = line.split("type ")[1] + if param in docparams: + docparams[param] = docstring[idx+1] + + # check that all params are documented + missing_params = [] + for param in arg_names: + if param == "self" or param == "cls": + continue + if param not in docparams: + missing_params.append(param) + + if missing_params: + self.add_message( + msgid="docstring-missing-param", args=(", ".join(missing_params)), node=node, confidence=None + ) + + # check if we have a type for each param and check if documented params that should be keywords + missing_types = [] + should_be_keywords = [] + for param in docparams: + if docparams[param] is None: + missing_types.append(param) + if param not in arg_names: + should_be_keywords.append(param) + + if missing_types: + self.add_message( + msgid="docstring-missing-type", args=(", ".join(missing_types)), node=node, confidence=None + ) + + if should_be_keywords: + self.add_message( + msgid="docstring-should-be-keyword", + args=(", ".join(should_be_keywords)), + node=node, + confidence=None + ) + + def check_return(self, node): + """Checks if function returns anything. + If return found, checks that the docstring contains a return doc and rtype. + + :param node: ast.FunctionDef + :return: None + """ + try: + returns = next(node.infer_call_result()).as_string() + if returns == "None": + return + except (astroid.exceptions.InferenceError, AttributeError): + # this function doesn't return anything, just return + return + + try: + # not every method will have a docstring so don't crash here, just return + docstring = node.doc.split(":") + except AttributeError: + return + + has_return, has_rtype = False, False + for line in docstring: + if line.startswith("return"): + has_return = True + if line.startswith("rtype"): + has_rtype = True + + if has_return is False: + self.add_message( + msgid="docstring-missing-return", node=node, confidence=None + ) + if has_rtype is False: + self.add_message( + msgid="docstring-missing-rtype", node=node, confidence=None + ) + + def visit_classdef(self, node): + """Visits every class in the file and finds the constructor. + Makes a call to compare class docstring with constructor params. + + :param node: ast.ClassDef + :return: None + """ + try: + for func in node.body: + if isinstance(func, astroid.FunctionDef) and func.name == "__init__": + self.check_parameters(node) + except Exception: + logger.debug("Pylint custom checker failed to check docstrings.") + pass + + def visit_functiondef(self, node): + """Visits every function in the file and makes calls + to check docstring parameters and return statements. + + :param node: ast.FunctionDef + :return: None + """ + try: + if node.name == "__init__": + return + self.check_parameters(node) + self.check_return(node) + except Exception: + logger.debug("Pylint custom checker failed to check docstrings.") + pass + + # this line makes it work for async functions + visit_asyncfunctiondef = visit_functiondef + + +class CheckForPolicyUse(BaseChecker): + __implements__ = IAstroidChecker + + name = "check-for-policies" + priority = -1 + msgs = { + "C4739": ( + "You should include a UserAgentPolicy in your HTTP pipeline. See details: " + "https://azure.github.io/azure-sdk/python_implementation.html#network-operations", + "missing-user-agent-policy", + "You should include a UserAgentPolicy in the HTTP Pipeline.", + ), + "C4740": ( + "You should include a LoggingPolicy in your HTTP pipeline. See details: " + "https://azure.github.io/azure-sdk/python_implementation.html#network-operations", + "missing-logging-policy", + "You should include a LoggingPolicy in the HTTP Pipeline.", + ), + "C4741": ( + "You should include a RetryPolicy in your HTTP pipeline. See details: " + "https://azure.github.io/azure-sdk/python_implementation.html#network-operations", + "missing-retry-policy", + "You should include a RetryPolicy in the HTTP Pipeline.", + ), + "C4742": ( + "You should include a DistributedTracingPolicy in your HTTP pipeline. See details: " + "https://azure.github.io/azure-sdk/python_implementation.html#network-operations", + "missing-distributed-tracing-policy", + "You should include a DistributedTracingPolicy in the HTTP Pipeline.", + ), + } + options = ( + ( + "ignore-missing-user-agent-policy", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow the client to not have a UserAgentPolicy", + }, + ), + ( + "ignore-missing-logging-policy", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow the client to not have a LoggingPolicy", + }, + ), + ( + "ignore-missing-retry-policy", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow the client to not have a RetryPolicy", + }, + ), + ( + "ignore-missing-distributed-tracing-policy", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow the client to not have a DistributedTracingPolicy", + }, + ), + ) + + def __init__(self, linter=None): + super(CheckForPolicyUse, self).__init__(linter) + self.node_to_use = None + self.has_policies = set() + self.ran_at_package_level = False + self.disable_logging_error = False + self.disable_user_agent_error = False + self.disable_tracing_error = False + self.disable_retry_error = False + + def visit_function(self, node, policy): + """Visits the function and searches line by line for the policy being used. + Also searches for if the policy came from the azure.core.configuration object. + + :param node: ast.FunctionDef + :param policy: The policy imported in the file. + :return: None + """ + for func in node.body: + if isinstance(func, astroid.FunctionDef): + for idx, item in enumerate(func.body): + try: + line = list(node.get_children())[idx].as_string() + if line.find(policy) != -1: + self.has_policies.add(policy) + if line.find("config.logging_policy") != -1: + self.has_policies.add("NetworkTraceLoggingPolicy") + if line.find("config.retry_policy") != -1: + self.has_policies.add("RetryPolicy") + if line.find("config.user_agent_policy") != -1: + self.has_policies.add("UserAgentPolicy") + except IndexError: + pass + + def visit_class(self, klass, policy): + """Visits any classes in the file and then makes a call + to search its methods for the policy being used. + + :param klass: A class within the file + :param policy: The policy imported in the file. + :return: None + """ + for idx, node in enumerate(klass): + if isinstance(node, astroid.ClassDef): + self.visit_function(node, policy) + + def visit_module(self, node): + """Visits every file in the package and searches for policies as base classes + or custom policies. If a core policy is imported in a file in calls helper + methods to check that the policy was used in the code. + + This pylint checker is different from the others as it collects information across + many files and then reports any errors. Due to this difference, disable commands + must be searched for manually. + + :param node: ast.Module + :return: None + """ + # only throw the error if pylint was run at package level since it needs to check all the files + # infer run location based on the location of the init file highest in dir hierarchy + if node.package: # the init file + count = node.file.split("azure-sdk-for-python")[1].count("-") + if node.file.split("azure-sdk-for-python")[1].count("\\") <= (5 + count) and \ + node.file.split("azure-sdk-for-python")[1].count("/") <= (5 + count): + self.ran_at_package_level = True + + # not really a good place to throw the pylint error, so we'll do it on the init file. + # By running this checker on all the files first and then reporting errors, pylint disables need to be + # done manually for some reason + if node.file.endswith("__init__.py") and self.node_to_use is None: + header = node.stream().read(200).lower() + if header.find(b'disable') != -1: + if header.find(b'missing-logging-policy') != -1: + self.disable_logging_error = True + if header.find(b'missing-user-agent-policy') != -1: + self.disable_user_agent_error = True + if header.find(b'missing-distributed-tracing-policy') != -1: + self.disable_tracing_error = True + if header.find(b'missing-retry-policy') != -1: + self.disable_retry_error = True + self.node_to_use = node + + for idx in range(len(node.body)): + # Check if the core policy is the base class for some custom policy, or a custom policy is being used + # and we try our best to find it based on common naming conventions. + if isinstance(node.body[idx], astroid.ClassDef): + if "NetworkTraceLoggingPolicy" in node.body[idx].basenames: + self.has_policies.add("NetworkTraceLoggingPolicy") + if node.body[idx].name.find("LoggingPolicy") != -1: + self.has_policies.add("NetworkTraceLoggingPolicy") + if "RetryPolicy" in node.body[idx].basenames or "AsyncRetryPolicy" in node.body[idx].basenames: + self.has_policies.add("RetryPolicy") + if node.body[idx].name.find("RetryPolicy") != -1: + self.has_policies.add("RetryPolicy") + if "UserAgentPolicy" in node.body[idx].basenames: + self.has_policies.add("UserAgentPolicy") + if node.body[idx].name.find("UserAgentPolicy") != -1: + self.has_policies.add("UserAgentPolicy") + if "DistributedTracingPolicy" in node.body[idx].basenames: + self.has_policies.add("DistributedTracingPolicy") + if node.body[idx].name.find("TracingPolicy") != -1: + self.has_policies.add("DistributedTracingPolicy") + + # policy is imported in this file, let's check that it gets used in the code + if isinstance(node.body[idx], astroid.ImportFrom): + for imp, pol in enumerate(node.body[idx].names): + if node.body[idx].names[imp][0].endswith("Policy") and \ + node.body[idx].names[imp][0] not in self.has_policies: + self.visit_class(node.body, node.body[idx].names[imp][0]) + + def close(self): + """This method is inherited from BaseChecker and called at the very end of linting a module. + It reports any errors and does a final check for any pylint disable statements. + + :return: None + """ + if self.ran_at_package_level: + if self.disable_logging_error is False: + if "NetworkTraceLoggingPolicy" not in self.has_policies: + self.add_message( + msgid="missing-logging-policy", node=self.node_to_use, confidence=None + ) + if self.disable_retry_error is False: + if "RetryPolicy" not in self.has_policies: + self.add_message( + msgid="missing-retry-policy", node=self.node_to_use, confidence=None + ) + if self.disable_user_agent_error is False: + if "UserAgentPolicy" not in self.has_policies: + self.add_message( + msgid="missing-user-agent-policy", node=self.node_to_use, confidence=None + ) + if self.disable_tracing_error is False: + if "DistributedTracingPolicy" not in self.has_policies: + self.add_message( + msgid="missing-distributed-tracing-policy", node=self.node_to_use, confidence=None + ) + + +class CheckDocstringAdmonitionNewline(BaseChecker): + __implements__ = IAstroidChecker + + name = "check-admonition" + priority = -1 + msgs = { + "C4744": ( + "The .. literalinclude statement needs a blank line above it. ", + "docstring-admonition-needs-newline", + "Put a newline after the example and before the literalinclude.", + ), + } + options = ( + ( + "ignore-docstring-admonition-needs-newline", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow a docstring to not have newline after admonition example.", + }, + ), + ) + + def __init__(self, linter=None): + super(CheckDocstringAdmonitionNewline, self).__init__(linter) + + def check_for_admonition(self, node): + """Parse the docstring for an admonition statement. + If found, checks that the literalinclude statement has + two newlines above it. + + :param node: ast.ClassDef or ast.FunctionDef + :return: None + """ + + try: + # not every class/method will have a docstring so don't crash here, just return + if node.doc.find("admonition") != -1 and node.doc.find(".. literalinclude") != -1: + literal_include = node.doc.split(".. literalinclude")[0] + chars_list = list(reversed(literal_include)) + for idx, char in enumerate(chars_list): + if char == '\n': + if chars_list[idx+1] == '\n': + break + else: + self.add_message( + "docstring-admonition-needs-newline", node=node, confidence=None + ) + break + except Exception: + return + + def visit_classdef(self, node): + """Visits every class docstring. + + :param node: ast.ClassDef + :return: None + """ + try: + for func in node.body: + if isinstance(func, astroid.FunctionDef) and func.name == "__init__": + self.check_for_admonition(node) + except Exception: + logger.debug("Pylint custom checker failed to check docstrings.") + pass + + def visit_functiondef(self, node): + """Visits every method docstring. + + :param node: ast.FunctionDef + :return: None + """ + try: + if node.name == "__init__": + return + self.check_for_admonition(node) + except Exception: + logger.debug("Pylint custom checker failed to check docstrings.") + pass + + # this line makes it work for async functions + visit_asyncfunctiondef = visit_functiondef + + +# if a linter is registered in this function then it will be checked with pylint +def register(linter): + linter.register_checker(ClientsDoNotUseStaticMethods(linter)) + linter.register_checker(ClientConstructorTakesCorrectParameters(linter)) + linter.register_checker(ClientMethodsUseKwargsWithMultipleParameters(linter)) + linter.register_checker(ClientMethodsHaveTypeAnnotations(linter)) + linter.register_checker(ClientUsesCorrectNamingConventions(linter)) + linter.register_checker(ClientMethodsHaveKwargsParameter(linter)) + linter.register_checker(ClientHasKwargsInPoliciesForCreateConfigurationMethod(linter)) + linter.register_checker(AsyncClientCorrectNaming(linter)) + linter.register_checker(FileHasCopyrightHeader(linter)) + linter.register_checker(ClientMethodNamesDoNotUseDoubleUnderscorePrefix(linter)) + linter.register_checker(SpecifyParameterNamesInCall(linter)) + linter.register_checker(ClientConstructorDoesNotHaveConnectionStringParam(linter)) + linter.register_checker(PackageNameDoesNotUseUnderscoreOrPeriod(linter)) + linter.register_checker(ServiceClientUsesNameWithClientSuffix(linter)) + linter.register_checker(CheckDocstringAdmonitionNewline(linter)) + + # disabled by default, use pylint --enable=check-docstrings if you want to use it + linter.register_checker(CheckDocstringParameters(linter)) + + # Rules are disabled until false positive rate improved + # linter.register_checker(CheckForPolicyUse(linter)) + # linter.register_checker(ClientHasApprovedMethodNamePrefix(linter)) + # linter.register_checker(ClientMethodsHaveTracingDecorators(linter)) + # linter.register_checker(ClientDocstringUsesLiteralIncludeForCodeExample(linter)) + # linter.register_checker(ClientListMethodsUseCorePaging(linter)) + # linter.register_checker(ClientLROMethodsUseCorePolling(linter)) + # linter.register_checker(ClientLROMethodsUseCorrectNaming(linter)) diff --git a/tools/pylint-extensions/pylint-guidelines-checker/setup.py b/tools/pylint-extensions/pylint-guidelines-checker/setup.py new file mode 100644 index 00000000000..36d8d461c97 --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/setup.py @@ -0,0 +1,12 @@ +from setuptools import setup + +setup( + name="pylint-guidelines-checker", + version="0.0.1", + url='http://github.com/Azure/azure-sdk-tools', + license='MIT License', + description="A pylint plugin which enforces azure sdk guidelines.", + author='Microsoft Corporation', + author_email='azpysdkhelp@microsoft.com', + py_modules=['pylint_guidelines_checker'], +) \ No newline at end of file diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py new file mode 100644 index 00000000000..7527ec9ea3d --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py @@ -0,0 +1,2570 @@ +# ------------------------------------ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. +# ------------------------------------ + +import astroid +import pylint.testutils + +from azure.core import PipelineClient +from azure.core.configuration import Configuration +import pylint_guidelines_checker as checker + +class TestClientMethodsHaveTracingDecorators(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientMethodsHaveTracingDecorators + + def test_ignores_constructor(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_private_method(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def _private_method(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_private_method_async(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + async def _private_method(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(function_node) + + def test_ignores_methods_with_decorators(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + from azure.core.tracing.decorator import distributed_trace + class SomeClient(): #@ + @distributed_trace + def create_configuration(self, **kwargs): #@ + pass + @distributed_trace + def get_thing(self, **kwargs): #@ + pass + @distributed_trace + def list_thing(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_functiondef(func_node_b) + self.checker.visit_functiondef(func_node_c) + + def test_ignores_async_methods_with_decorators(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + from azure.core.tracing.decorator_async import distributed_trace_async + class SomeClient(): #@ + @distributed_trace_async + async def create_configuration(self, **kwargs): #@ + pass + @distributed_trace_async + async def get_thing(self, **kwargs): #@ + pass + @distributed_trace_async + async def list_thing(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + self.checker.visit_asyncfunctiondef(func_node_c) + + def test_finds_sync_decorator_on_async_method(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + from azure.core.tracing.decorator import distributed_trace + class SomeClient(): #@ + @distributed_trace + async def create_configuration(self, **kwargs): #@ + pass + @distributed_trace + async def get_thing(self, **kwargs): #@ + pass + @distributed_trace + async def list_thing(self, **kwargs): #@ + pass + """) + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-tracing-decorator-async", node=func_node_a + ), + pylint.testutils.Message( + msg_id="client-method-missing-tracing-decorator-async", node=func_node_b + ), + pylint.testutils.Message( + msg_id="client-method-missing-tracing-decorator-async", node=func_node_c + ), + ): + self.checker.visit_asyncfunctiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + self.checker.visit_asyncfunctiondef(func_node_c) + + def test_finds_async_decorator_on_sync_method(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + from azure.core.tracing.decorator_async import distributed_trace_async + class SomeClient(): #@ + @distributed_trace_async + def create_configuration(self, **kwargs): #@ + pass + @distributed_trace_async + def get_thing(self, **kwargs): #@ + pass + @distributed_trace_async + def list_thing(self, **kwargs): #@ + pass + """) + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-tracing-decorator", node=func_node_a + ), + pylint.testutils.Message( + msg_id="client-method-missing-tracing-decorator", node=func_node_b + ), + pylint.testutils.Message( + msg_id="client-method-missing-tracing-decorator", node=func_node_c + ), + ): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_functiondef(func_node_b) + self.checker.visit_functiondef(func_node_c) + + def test_ignores_other_decorators(self): + class_node, func_node_a, func_node_b = astroid.extract_node( + """ + from azure.core.tracing.decorator import distributed_trace + class SomeClient(): #@ + @classmethod + @distributed_trace + def download_thing(self, some, **kwargs): #@ + pass + + @distributed_trace + @decorator + def do_thing(self, some, **kwargs): #@ + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_functiondef(func_node_b) + + def test_ignores_other_decorators_async(self): + class_node, func_node_a, func_node_b = astroid.extract_node( + """ + from azure.core.tracing.decorator_async import distributed_trace_async + class SomeClient(): #@ + @classmethod + @distributed_trace_async + async def download_thing(self, some, **kwargs): #@ + pass + + @distributed_trace_async + @decorator + async def do_thing(self, some, **kwargs): #@ + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + + def test_ignores_non_client_method(self): + class_node, func_node_a, func_node_b = astroid.extract_node( + """ + class SomethingElse(): #@ + def download_thing(self, some, **kwargs): #@ + pass + + @classmethod + async def do_thing(self, some, **kwargs): #@ + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_implementation.html#distributed-tracing" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientsDoNotUseStaticMethods(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientsDoNotUseStaticMethods + + def test_ignores_constructor(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_private_method(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + @staticmethod + def _private_method(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_private_method_async(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + @staticmethod + async def _private_method(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(function_node) + + def test_ignores_methods_with_other_decorators(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + class SomeClient(): #@ + @distributed_trace + def create_configuration(self): #@ + pass + @distributed_trace + def get_thing(self): #@ + pass + @distributed_trace + def list_thing(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_functiondef(func_node_b) + self.checker.visit_functiondef(func_node_c) + + def test_ignores_async_methods_with_other_decorators(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + class SomeClient(): #@ + @distributed_trace_async + async def create_configuration(self): #@ + pass + @distributed_trace_async + async def get_thing(self): #@ + pass + @distributed_trace_async + async def list_thing(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + self.checker.visit_asyncfunctiondef(func_node_c) + + def test_finds_staticmethod_on_async_method(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + class SomeClient(): #@ + @staticmethod + async def create_configuration(self): #@ + pass + @staticmethod + async def get_thing(self): #@ + pass + @staticmethod + async def list_thing(self): #@ + pass + """) + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-should-not-use-static-method", node=func_node_a + ), + pylint.testutils.Message( + msg_id="client-method-should-not-use-static-method", node=func_node_b + ), + pylint.testutils.Message( + msg_id="client-method-should-not-use-static-method", node=func_node_c + ), + ): + self.checker.visit_asyncfunctiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + self.checker.visit_asyncfunctiondef(func_node_c) + + def test_finds_staticmethod_on_sync_method(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + class SomeClient(): #@ + @staticmethod + def create_configuration(self): #@ + pass + @staticmethod + def get_thing(self): #@ + pass + @staticmethod + def list_thing(self): #@ + pass + """) + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-should-not-use-static-method", node=func_node_a + ), + pylint.testutils.Message( + msg_id="client-method-should-not-use-static-method", node=func_node_b + ), + pylint.testutils.Message( + msg_id="client-method-should-not-use-static-method", node=func_node_c + ), + ): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_functiondef(func_node_b) + self.checker.visit_functiondef(func_node_c) + + def test_ignores_other_multiple_decorators(self): + class_node, func_node_a, func_node_b = astroid.extract_node( + """ + class SomeClient(): #@ + @classmethod + @distributed_trace + def download_thing(self, some, **kwargs): #@ + pass + + @distributed_trace + @decorator + def do_thing(self, some, **kwargs): #@ + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_functiondef(func_node_b) + + def test_ignores_other_multiple_decorators_async(self): + class_node, func_node_a, func_node_b = astroid.extract_node( + """ + class SomeClient(): #@ + @classmethod + @distributed_trace_async + async def download_thing(self, some, **kwargs): #@ + pass + + @distributed_trace_async + @decorator + async def do_thing(self, some, **kwargs): #@ + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + + def test_ignores_non_client_method(self): + class_node, func_node_a, func_node_b = astroid.extract_node( + """ + class SomethingElse(): #@ + @staticmethod + def download_thing(self, some, **kwargs): #@ + pass + + @staticmethod + async def do_thing(self, some, **kwargs): #@ + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_introduction.html#method-signatures" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientHasApprovedMethodNamePrefix(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientHasApprovedMethodNamePrefix + + def test_ignores_constructor(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_private_method(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def _private_method(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_if_exists_suffix(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def check_if_exists(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_from_prefix(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def from_connection_string(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_approved_prefix_names(self): + class_node, func_node_a, func_node_b, func_node_c, func_node_d, func_node_e, func_node_f, func_node_g, \ + func_node_h, func_node_i, func_node_j, func_node_k, func_node_l = astroid.extract_node(""" + class SomeClient(): #@ + def create_configuration(self): #@ + pass + def get_thing(self): #@ + pass + def list_thing(self): #@ + pass + def upsert_thing(self): #@ + pass + def set_thing(self): #@ + pass + def update_thing(self): #@ + pass + def replace_thing(self): #@ + pass + def append_thing(self): #@ + pass + def add_thing(self): #@ + pass + def delete_thing(self): #@ + pass + def remove_thing(self): #@ + pass + def begin_thing(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_non_client_with_unapproved_prefix_names(self): + class_node, function_node = astroid.extract_node( + """ + class SomethingElse(): #@ + def download_thing(self, some, **kwargs): #@ + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_nested_function_with_unapproved_prefix_names(self): + class_node, function_node = astroid.extract_node( + """ + class SomeClient(): #@ + def create_configuration(self, **kwargs): #@ + def nested(hello, world): + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_finds_unapproved_prefix_names(self): + class_node, func_node_a, func_node_b, func_node_c, func_node_d, func_node_e, func_node_f, func_node_g, \ + func_node_h, func_node_i, func_node_j, func_node_k, func_node_l, func_node_m, func_node_n, func_node_o, \ + func_node_p = astroid.extract_node(""" + class SomeClient(): #@ + @distributed_trace + def build_configuration(self): #@ + pass + def generate_thing(self): #@ + pass + def make_thing(self): #@ + pass + def insert_thing(self): #@ + pass + def put_thing(self): #@ + pass + def creates_configuration(self): #@ + pass + def gets_thing(self): #@ + pass + def lists_thing(self): #@ + pass + def upserts_thing(self): #@ + pass + def sets_thing(self): #@ + pass + def updates_thing(self): #@ + pass + def replaces_thing(self): #@ + pass + def appends_thing(self): #@ + pass + def adds_thing(self): #@ + pass + def deletes_thing(self): #@ + pass + def removes_thing(self): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_a + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_b + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_c + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_d + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_e + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_f + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_g + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_h + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_i + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_j + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_k + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_l + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_m + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_n + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_o + ), + pylint.testutils.Message( + msg_id="unapproved-client-method-name-prefix", node=func_node_p + ) + ): + self.checker.visit_classdef(class_node) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#service-operations" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientConstructorTakesCorrectParameters(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientConstructorTakesCorrectParameters + + def test_finds_correct_params(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, thing_url, credential, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_non_constructor_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def create_configuration(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_non_client_constructor_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomethingElse(): #@ + def __init__(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_finds_constructor_without_kwargs(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, thing_url, credential=None): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="missing-client-constructor-parameter-kwargs", node=function_node + ) + ): + self.checker.visit_functiondef(function_node) + + def test_finds_constructor_without_credentials(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, thing_url, **kwargs): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="missing-client-constructor-parameter-credential", node=function_node + ) + ): + self.checker.visit_functiondef(function_node) + + def test_finds_constructor_with_no_params(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="missing-client-constructor-parameter-credential", node=function_node + ), + pylint.testutils.Message( + msg_id="missing-client-constructor-parameter-kwargs", node=function_node + ) + ): + self.checker.visit_functiondef(function_node) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientMethodsUseKwargsWithMultipleParameters(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientMethodsUseKwargsWithMultipleParameters + + def test_ignores_method_abiding_to_guidelines(self): + class_node, function_node, function_node_a, function_node_b, function_node_c, function_node_d, \ + function_node_e, function_node_f, function_node_g, function_node_h, function_node_i, function_node_j, \ + function_node_k, function_node_l, function_node_m = astroid.extract_node(""" + class SomeClient(): #@ + @distributed_trace + def do_thing(): #@ + pass + def do_thing_a(self): #@ + pass + def do_thing_b(self, one): #@ + pass + def do_thing_c(self, one, two): #@ + pass + def do_thing_d(self, one, two, three): #@ + pass + def do_thing_e(self, one, two, three, four): #@ + pass + def do_thing_f(self, one, two, three, four, five): #@ + pass + def do_thing_g(self, one, two, three, four, five, six=6): #@ + pass + def do_thing_h(self, one, two, three, four, five, six=6, seven=7): #@ + pass + def do_thing_i(self, one, two, three, four, five, *, six=6, seven=7): #@ + pass + def do_thing_j(self, one, two, three, four, five, *, six=6, seven=7): #@ + pass + def do_thing_k(self, one, two, three, four, five, **kwargs): #@ + pass + def do_thing_l(self, one, two, three, four, five, *args, **kwargs): #@ + pass + def do_thing_m(self, one, two, three, four, five, *args, six, seven=7, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + self.checker.visit_functiondef(function_node_c) + self.checker.visit_functiondef(function_node_d) + self.checker.visit_functiondef(function_node_e) + self.checker.visit_functiondef(function_node_f) + self.checker.visit_functiondef(function_node_g) + self.checker.visit_functiondef(function_node_h) + self.checker.visit_functiondef(function_node_i) + self.checker.visit_functiondef(function_node_j) + self.checker.visit_functiondef(function_node_k) + self.checker.visit_functiondef(function_node_l) + self.checker.visit_functiondef(function_node_m) + + def test_ignores_method_abiding_to_guidelines_async(self): + class_node, function_node, function_node_a, function_node_b, function_node_c, function_node_d, \ + function_node_e, function_node_f, function_node_g, function_node_h, function_node_i, function_node_j, \ + function_node_k, function_node_l, function_node_m = astroid.extract_node(""" + class SomeClient(): #@ + @distributed_trace_async + async def do_thing(): #@ + pass + async def do_thing_a(self): #@ + pass + async def do_thing_b(self, one): #@ + pass + async def do_thing_c(self, one, two): #@ + pass + async def do_thing_d(self, one, two, three): #@ + pass + async def do_thing_e(self, one, two, three, four): #@ + pass + async def do_thing_f(self, one, two, three, four, five): #@ + pass + async def do_thing_g(self, one, two, three, four, five, six=6): #@ + pass + async def do_thing_h(self, one, two, three, four, five, six=6, seven=7): #@ + pass + async def do_thing_i(self, one, two, three, four, five, *, six=6, seven=7): #@ + pass + async def do_thing_j(self, one, two, three, four, five, *, six=6, seven=7): #@ + pass + async def do_thing_k(self, one, two, three, four, five, **kwargs): #@ + pass + async def do_thing_l(self, one, two, three, four, five, *args, **kwargs): #@ + pass + async def do_thing_m(self, one, two, three, four, five, *args, six, seven=7, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(function_node) + self.checker.visit_asyncfunctiondef(function_node_a) + self.checker.visit_asyncfunctiondef(function_node_b) + self.checker.visit_asyncfunctiondef(function_node_c) + self.checker.visit_asyncfunctiondef(function_node_d) + self.checker.visit_asyncfunctiondef(function_node_e) + self.checker.visit_asyncfunctiondef(function_node_f) + self.checker.visit_asyncfunctiondef(function_node_g) + self.checker.visit_asyncfunctiondef(function_node_h) + self.checker.visit_asyncfunctiondef(function_node_i) + self.checker.visit_asyncfunctiondef(function_node_j) + self.checker.visit_asyncfunctiondef(function_node_k) + self.checker.visit_asyncfunctiondef(function_node_l) + self.checker.visit_asyncfunctiondef(function_node_m) + + def test_finds_methods_with_too_many_positional_args(self): + class_node, function_node, function_node_a, function_node_b, function_node_c, function_node_d, \ + function_node_e, function_node_f = astroid.extract_node(""" + class SomeClient(): #@ + @distributed_trace + def do_thing(self, one, two, three, four, five, six): #@ + pass + def do_thing_a(self, one, two, three, four, five, six, seven=7): #@ + pass + def do_thing_b(self, one, two, three, four, five, six, *, seven): #@ + pass + def do_thing_c(self, one, two, three, four, five, six, *, seven, eight, nine): #@ + pass + def do_thing_d(self, one, two, three, four, five, six, **kwargs): #@ + pass + def do_thing_e(self, one, two, three, four, five, six, *args, seven, eight, nine): #@ + pass + def do_thing_f(self, one, two, three, four, five, six, *args, seven=7, eight=8, nine=9): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_b + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_c + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_d + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_e + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_f + ) + ): + self.checker.visit_functiondef(function_node) + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + self.checker.visit_functiondef(function_node_c) + self.checker.visit_functiondef(function_node_d) + self.checker.visit_functiondef(function_node_e) + self.checker.visit_functiondef(function_node_f) + + def test_finds_methods_with_too_many_positional_args_async(self): + class_node, function_node, function_node_a, function_node_b, function_node_c, function_node_d, \ + function_node_e, function_node_f = astroid.extract_node(""" + class SomeClient(): #@ + @distributed_trace_async + async def do_thing(self, one, two, three, four, five, six): #@ + pass + async def do_thing_a(self, one, two, three, four, five, six, seven=7): #@ + pass + async def do_thing_b(self, one, two, three, four, five, six, *, seven): #@ + pass + async def do_thing_c(self, one, two, three, four, five, six, *, seven, eight, nine): #@ + pass + async def do_thing_d(self, one, two, three, four, five, six, **kwargs): #@ + pass + async def do_thing_e(self, one, two, three, four, five, six, *args, seven, eight, nine): #@ + pass + async def do_thing_f(self, one, two, three, four, five, six, *args, seven=7, eight=8, nine=9): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_b + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_c + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_d + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_e + ), + pylint.testutils.Message( + msg_id="client-method-has-more-than-5-positional-arguments", node=function_node_f + ) + ): + self.checker.visit_asyncfunctiondef(function_node) + self.checker.visit_asyncfunctiondef(function_node_a) + self.checker.visit_asyncfunctiondef(function_node_b) + self.checker.visit_asyncfunctiondef(function_node_c) + self.checker.visit_asyncfunctiondef(function_node_d) + self.checker.visit_asyncfunctiondef(function_node_e) + self.checker.visit_asyncfunctiondef(function_node_f) + + def test_ignores_non_client_methods(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomethingElse(): #@ + def do_thing(self, one, two, three, four, five, six): #@ + pass + + @distributed_trace_async + async def do_thing(self, one, two, three, four, five, six): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_asyncfunctiondef(function_node_b) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_introduction.html#method-signatures" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientMethodsHaveTypeAnnotations(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientMethodsHaveTypeAnnotations + + def test_ignores_correct_type_annotations(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing(self, one: str, two: int, three: bool, four: Union[str, thing], five: dict) -> int: #@ + pass + async def do_thing(self, one: str, two: int, three: bool, four: Union[str, thing], five: dict) -> int: #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_asyncfunctiondef(function_node_b) + + def test_ignores_correct_type_comments(self): + class_node, function_node_a, function_node_b, function_node_c = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing_a(self, one, two, three, four, five): #@ + # type: (str, str, str, str, str) -> None + pass + + def do_thing_b(self, one, two): # type: (str, str) -> int #@ + pass + + def do_thing_c(self, #@ + one, # type: str + two, # type: str + three, # type: str + four, # type: str + five # type: str + ): + # type: (...) -> int + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + self.checker.visit_functiondef(function_node_c) + + def test_ignores_correct_type_comments_async(self): + class_node, function_node_a, function_node_b, function_node_c = astroid.extract_node(""" + class SomeClient(): #@ + async def do_thing_a(self, one, two, three, four, five): #@ + # type: (str, str, str, str, str) -> None + pass + + async def do_thing_b(self, one, two): # type: (str, str) -> int #@ + pass + + async def do_thing_c(self, #@ + one, # type: str + two, # type: str + three, # type: str + four, # type: str + five # type: str + ): + # type: (...) -> int + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(function_node_a) + self.checker.visit_asyncfunctiondef(function_node_b) + self.checker.visit_asyncfunctiondef(function_node_c) + + def test_ignores_no_parameter_method_with_annotations(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing_a(self): #@ + # type: () -> None + pass + + def do_thing_b(self) -> None: #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_ignores_no_parameter_method_with_annotations_async(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + async def do_thing_a(self): #@ + # type: () -> None + pass + + async def do_thing_b(self) -> None: #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(function_node_a) + self.checker.visit_asyncfunctiondef(function_node_b) + + def test_finds_no_parameter_method_without_annotations(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing(self): #@ + pass + async def do_thing(self): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node_b + ), + ): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_finds_method_missing_annotations(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing(self, one, two, three): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node + ) + ): + self.checker.visit_functiondef(function_node) + + def test_finds_method_missing_annotations_async(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + async def do_thing(self, one, two, three): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node + ) + ): + self.checker.visit_asyncfunctiondef(function_node) + + def test_finds_constructor_without_annotations(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, one, two, three, four, five): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node + ) + ): + self.checker.visit_functiondef(function_node) + + def test_finds_missing_return_annotation_but_has_type_hints(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing_a(self, one: str, two: int, three: bool, four: Union[str, thing], five: dict): #@ + pass + + def do_thing_b(self, one, two, three, four, five): #@ + # type: (str, str, str, str, str) + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node_b + ), + ): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_finds_missing_return_annotation_but_has_type_hints_async(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + async def do_thing_a(self, one: str, two: int, three: bool, four: Union[str, thing], five: dict): #@ + pass + + async def do_thing_b(self, one, two, three, four, five): #@ + # type: (str, str, str, str, str) + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node_b + ), + ): + self.checker.visit_asyncfunctiondef(function_node_a) + self.checker.visit_asyncfunctiondef(function_node_b) + + def test_finds_missing_annotations_but_has_return_hint(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing_a(self, one, two, three, four, five) -> None: #@ + pass + + def do_thing_b(self, one, two, three, four, five): #@ + # type: -> None + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node_b + ) + ): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_finds_missing_annotations_but_has_return_hint_async(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + async def do_thing_a(self, one, two, three, four, five) -> None: #@ + pass + + async def do_thing_b(self, one, two, three, four, five): #@ + # type: -> None + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-method-missing-type-annotations", node=function_node_b + ) + ): + self.checker.visit_asyncfunctiondef(function_node_a) + self.checker.visit_asyncfunctiondef(function_node_b) + + def test_ignores_non_client_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomethingElse(): #@ + def do_thing(self, one, two, three, four, five, six): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_private_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomethingElse(): #@ + def _do_thing(self, one, two, three, four, five, six): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_introduction.html#types-or-not" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientHasKwargsInPoliciesForCreateConfigurationMethod(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientHasKwargsInPoliciesForCreateConfigurationMethod + + def test_ignores_config_policies_with_kwargs(self): + function_node_a, function_node_b = astroid.extract_node(""" + def create_configuration(self, **kwargs): #@ + config = Configuration(**kwargs) + config.headers_policy = StorageHeadersPolicy(**kwargs) + config.user_agent_policy = StorageUserAgentPolicy(**kwargs) + config.retry_policy = kwargs.get('retry_policy') or ExponentialRetry(**kwargs) + config.redirect_policy = RedirectPolicy(**kwargs) + config.logging_policy = StorageLoggingPolicy(**kwargs) + config.proxy_policy = ProxyPolicy(**kwargs) + return config + + @staticmethod + def create_config(credential, api_version=None, **kwargs): #@ + # type: (TokenCredential, Optional[str], Mapping[str, Any]) -> Configuration + if api_version is None: + api_version = KeyVaultClient.DEFAULT_API_VERSION + config = KeyVaultClient.get_configuration_class(api_version, aio=False)(credential, **kwargs) + config.authentication_policy = ChallengeAuthPolicy(credential, **kwargs) + return config + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_finds_config_policies_without_kwargs(self): + function_node_a, policy_a, policy_b, policy_c, function_node_b, policy_d = astroid.extract_node(""" + def create_configuration(self, **kwargs): #@ + config = Configuration(**kwargs) + config.headers_policy = StorageHeadersPolicy(**kwargs) + config.user_agent_policy = StorageUserAgentPolicy() #@ + config.retry_policy = kwargs.get('retry_policy') or ExponentialRetry(**kwargs) + config.redirect_policy = RedirectPolicy(**kwargs) + config.logging_policy = StorageLoggingPolicy() #@ + config.proxy_policy = ProxyPolicy() #@ + return config + + @staticmethod + def create_config(credential, api_version=None, **kwargs): #@ + # type: (TokenCredential, Optional[str], Mapping[str, Any]) -> Configuration + if api_version is None: + api_version = KeyVaultClient.DEFAULT_API_VERSION + config = KeyVaultClient.get_configuration_class(api_version, aio=False)(credential, **kwargs) + config.authentication_policy = ChallengeAuthPolicy(credential) #@ + return config + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="config-missing-kwargs-in-policy", node=policy_a + ), + pylint.testutils.Message( + msg_id="config-missing-kwargs-in-policy", node=policy_b + ), + pylint.testutils.Message( + msg_id="config-missing-kwargs-in-policy", node=policy_c + ), + pylint.testutils.Message( + msg_id="config-missing-kwargs-in-policy", node=policy_d + ) + ): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_ignores_policies_outside_create_config(self): + function_node_a, function_node_b = astroid.extract_node(""" + def _configuration(self, **kwargs): #@ + config = Configuration(**kwargs) + config.headers_policy = StorageHeadersPolicy(**kwargs) + config.user_agent_policy = StorageUserAgentPolicy(**kwargs) + config.retry_policy = kwargs.get('retry_policy') or ExponentialRetry() + config.redirect_policy = RedirectPolicy() + config.logging_policy = StorageLoggingPolicy() + config.proxy_policy = ProxyPolicy() + return config + + @staticmethod + def some_other_method(credential, api_version=None, **kwargs): #@ + # type: (TokenCredential, Optional[str], Mapping[str, Any]) -> Configuration + if api_version is None: + api_version = KeyVaultClient.DEFAULT_API_VERSION + config = KeyVaultClient.get_configuration_class(api_version, aio=False)(credential) + config.authentication_policy = ChallengeAuthPolicy(credential) + return config + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientUsesCorrectNamingConventions(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientUsesCorrectNamingConventions + + def test_ignores_constructor(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_internal_client(self): + class_node, function_node = astroid.extract_node(""" + class _BaseSomeClient(): #@ + def __init__(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_private_method(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + def _private_method(self, **kwargs): #@ + pass + async def _another_private_method(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_correct_client(self): + class_node = astroid.extract_node(""" + class SomeClient(): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_non_client(self): + class_node, function_node = astroid.extract_node( + """ + class SomethingElse(): #@ + def download_thing(self, some, **kwargs): #@ + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_correct_method_names(self): + class_node, function_node_a, function_node_b, function_node_c = astroid.extract_node(""" + class SomeClient(): #@ + def from_connection_string(self, **kwargs): #@ + pass + def get_thing(self, **kwargs): #@ + pass + def delete_thing(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_correct_method_names_async(self): + class_node, function_node_a, function_node_b, function_node_c = astroid.extract_node(""" + class SomeClient(): #@ + def from_connection_string(self, **kwargs): #@ + pass + def get_thing(self, **kwargs): #@ + pass + def delete_thing(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_correct_class_constant(self): + class_node = astroid.extract_node(""" + class SomeClient(): #@ + MAX_SIZE = 14 + MIN_SIZE = 2 + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_finds_incorrectly_named_client(self): + class_node_a, class_node_b, class_node_c = astroid.extract_node(""" + class some_client(): #@ + pass + class Some_Client(): #@ + pass + class someClient(): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=class_node_a + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=class_node_b + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=class_node_c + ), + ): + self.checker.visit_classdef(class_node_a) + self.checker.visit_classdef(class_node_b) + self.checker.visit_classdef(class_node_c) + + def test_finds_incorrectly_named_methods(self): + class_node, func_node_a, func_node_b, func_node_c, func_node_d, func_node_e, func_node_f \ + = astroid.extract_node(""" + class SomeClient(): #@ + def Create_Config(self): #@ + pass + def getThing(self): #@ + pass + def List_thing(self): #@ + pass + def UpsertThing(self): #@ + pass + def set_Thing(self): #@ + pass + def Updatething(self): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_a + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_b + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_c + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_d + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_e + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_f + ), + ): + self.checker.visit_classdef(class_node) + + def test_finds_incorrectly_named_methods_async(self): + class_node, func_node_a, func_node_b, func_node_c, func_node_d, func_node_e, func_node_f \ + = astroid.extract_node(""" + class SomeClient(): #@ + async def Create_Config(self): #@ + pass + async def getThing(self): #@ + pass + async def List_thing(self): #@ + pass + async def UpsertThing(self): #@ + pass + async def set_Thing(self): #@ + pass + async def Updatething(self): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_a + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_b + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_c + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_d + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_e + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=func_node_f + ), + ): + self.checker.visit_classdef(class_node) + + def test_finds_incorrectly_named_class_constant(self): + class_node, const_a, const_b = astroid.extract_node(""" + class SomeClient(): #@ + max_size = 14 #@ + min_size = 2 #@ + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=const_a + ), + pylint.testutils.Message( + msg_id="client-incorrect-naming-convention", node=const_b + ), + ): + self.checker.visit_classdef(class_node) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_introduction.html#naming-conventions" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientMethodsHaveKwargsParameter(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientMethodsHaveKwargsParameter + + def test_ignores_private_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def _create_configuration(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_properties(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + @property + def key_id(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_properties_async(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + @property + async def key_id(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(function_node) + + def test_ignores_non_client_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomethingElse(): #@ + def create_configuration(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_methods_with_kwargs(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + def get_thing(self, **kwargs): #@ + pass + @distributed_trace + def remove_thing(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_finds_missing_kwargs(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + from azure.core.tracing.decorator import distributed_trace + + class SomeClient(): #@ + @distributed_trace + def get_thing(self): #@ + pass + @distributed_trace + def remove_thing(self): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-kwargs", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-method-missing-kwargs", node=function_node_b + ), + ): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_ignores_methods_with_kwargs_async(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + async def get_thing(self, **kwargs): #@ + pass + async def remove_thing(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(function_node_a) + self.checker.visit_asyncfunctiondef(function_node_b) + + def test_finds_missing_kwargs_async(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + from azure.core.tracing.decorator_async import distributed_trace_async + + class SomeClient(): #@ + @distributed_trace_async + async def get_thing(self): #@ + pass + @distributed_trace_async + async def remove_thing(self): #@ + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-missing-kwargs", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-method-missing-kwargs", node=function_node_b + ), + ): + self.checker.visit_asyncfunctiondef(function_node_a) + self.checker.visit_asyncfunctiondef(function_node_b) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestAsyncClientCorrectNaming(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.AsyncClientCorrectNaming + + def test_ignores_private_client(self): + class_node = astroid.extract_node(""" + class _AsyncBaseSomeClient(): #@ + def create_configuration(self): + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_correct_client(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def create_configuration(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_async_base_named_client(self): + class_node_a = astroid.extract_node(""" + class AsyncSomeClientBase(): #@ + def get_thing(self, **kwargs): + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node_a) + + def test_finds_incorrectly_named_client(self): + class_node_a = astroid.extract_node(""" + class AsyncSomeClient(): #@ + def get_thing(self, **kwargs): + pass + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="async-client-bad-name", node=class_node_a + ), + ): + self.checker.visit_classdef(class_node_a) + + def test_ignores_non_client(self): + class_node, function_node = astroid.extract_node(""" + class SomethingElse(): #@ + def create_configuration(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#async-support" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestFileHasCopyrightHeader(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.FileHasCopyrightHeader + + # Unable to use the astroid for this testcase. + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/policies_opensource.html" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestSpecifyParameterNamesInCall(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.SpecifyParameterNamesInCall + + def test_ignores_call_with_only_two_unnamed_params(self): + class_node, call_node = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing(self): + self._client.thing(one, two) #@ + """) + + with self.assertNoMessages(): + self.checker.visit_call(call_node) + + def test_ignores_call_with_two_unnamed_params_and_one_named(self): + class_node, call_node = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing(self): + self._client.thing(one, two, three=3) #@ + """) + + with self.assertNoMessages(): + self.checker.visit_call(call_node) + + def test_ignores_call_from_non_client(self): + class_node, call_node = astroid.extract_node(""" + class SomethingElse(): #@ + def do_thing(self): + self._other.thing(one, two, three) #@ + """) + + with self.assertNoMessages(): + self.checker.visit_call(call_node) + + def test_ignores_call_with_named_params(self): + class_node, call_node_a, call_node_b, call_node_c = astroid.extract_node(""" + class SomethingElse(): #@ + def do_thing_a(self): + self._other.thing(one=one, two=two, three=three) #@ + def do_thing_b(self): + self._other.thing(zero, number, one=one, two=two, three=three) #@ + def do_thing_c(self): + self._other.thing(zero, one=one, two=two, three=three) #@ + """) + + with self.assertNoMessages(): + self.checker.visit_call(call_node_a) + self.checker.visit_call(call_node_b) + self.checker.visit_call(call_node_c) + + def test_ignores_non_client_function_call(self): + call_node = astroid.extract_node(""" + def do_thing(): + self._client.thing(one, two, three) #@ + """) + + with self.assertNoMessages(): + self.checker.visit_call(call_node) + + def test_finds_call_with_more_than_two_unnamed_params(self): + class_node, call_node = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing(self): + self._client.thing(one, two, three) #@ + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="specify-parameter-names-in-call", node=call_node + ), + ): + self.checker.visit_call(call_node) + + def test_finds_call_with_more_than_two_unnamed_params_and_some_named(self): + class_node, call_node = astroid.extract_node(""" + class SomeClient(): #@ + def do_thing(self): + self._client.thing(one, two, three, four=4, five=5) #@ + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="specify-parameter-names-in-call", node=call_node + ), + ): + self.checker.visit_call(call_node) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_introduction.html#method-signatures" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientListMethodsUseCorePaging(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientListMethodsUseCorePaging + + def test_ignores_private_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def _list_thing(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_non_client_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomethingElse(): #@ + def list_things(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_methods_return_ItemPaged(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + from azure.core.paging import ItemPaged + + class SomeClient(): #@ + def list_thing(self): #@ + return ItemPaged() + @distributed_trace + def list_thing2(self): #@ + return ItemPaged( + command, prefix=name_starts_with, results_per_page=results_per_page, + page_iterator_class=BlobPropertiesPaged) + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_ignores_methods_return_AsyncItemPaged(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + from azure.core.async_paging import AsyncItemPaged + + class SomeClient(): #@ + async def list_thing(self): #@ + return AsyncItemPaged() + @distributed_trace + def list_thing2(self): #@ + return AsyncItemPaged( + command, prefix=name_starts_with, results_per_page=results_per_page, + page_iterator_class=BlobPropertiesPaged) + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_finds_method_returning_something_else(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + from azure.core.polling import LROPoller + + class SomeClient(): #@ + def list_thing(self): #@ + return list() + def list_thing2(self): #@ + return LROPoller() + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-list-methods-use-paging", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-list-methods-use-paging", node=function_node_b + ), + ): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_finds_method_returning_something_else_async(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + from azure.core.polling import LROPoller + + class SomeClient(): #@ + async def list_thing(self, **kwargs): #@ + return list() + async def list_thing2(self, **kwargs): #@ + from azure.core.polling import LROPoller + return LROPoller() + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-list-methods-use-paging", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-list-methods-use-paging", node=function_node_b + ), + ): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#response-formats" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientLROMethodsUseCorePolling(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientLROMethodsUseCorePolling + + def test_ignores_private_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def _begin_thing(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_non_client_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomethingElse(): #@ + def begin_things(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_methods_return_LROPoller(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + from azure.core.polling import LROPoller + + class SomeClient(): #@ + def begin_thing(self): #@ + return LROPoller() + @distributed_trace + def begin_thing2(self): #@ + return LROPoller(self._client, raw_result, get_long_running_output, polling_method) + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_finds_method_returning_something_else(self): + class_node, function_node_a, function_node_b = astroid.extract_node(""" + class SomeClient(): #@ + def begin_thing(self): #@ + return list() + def begin_thing2(self): #@ + return {} + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-lro-methods-use-polling", node=function_node_a + ), + pylint.testutils.Message( + msg_id="client-lro-methods-use-polling", node=function_node_b + ), + ): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#response-formats" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientLROMethodsUseCorrectNaming(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientLROMethodsUseCorrectNaming + + def test_ignores_private_methods(self): + class_node, return_node = astroid.extract_node(""" + from azure.core.polling import LROPoller + + class SomeClient(): #@ + def _do_thing(self): + return LROPoller(self._client, raw_result, get_long_running_output, polling_method) #@ + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + self.checker.visit_return(return_node) + + def test_ignores_non_client_methods(self): + class_node, return_node = astroid.extract_node(""" + from azure.core.polling import LROPoller + + class SomethingElse(): #@ + def begin_things(self): + return LROPoller(self._client, raw_result, get_long_running_output, polling_method) #@ + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + self.checker.visit_return(return_node) + + def test_ignores_methods_return_LROPoller_and_correctly_named(self): + class_node, return_node_a, return_node_b = astroid.extract_node(""" + from azure.core.polling import LROPoller + + class SomeClient(): #@ + def begin_thing(self): + return LROPoller() #@ + @distributed_trace + def begin_thing2(self): + return LROPoller(self._client, raw_result, get_long_running_output, polling_method) #@ + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + self.checker.visit_return(return_node_a) + self.checker.visit_return(return_node_b) + + def test_finds_incorrectly_named_method_returning_LROPoller(self): + class_node, function_node_a, return_node_a, function_node_b, return_node_b = astroid.extract_node(""" + from azure.core.polling import LROPoller + + class SomeClient(): #@ + def poller_thing(self): #@ + return LROPoller() #@ + @distributed_trace + def start_thing2(self): #@ + return LROPoller(self._client, raw_result, get_long_running_output, polling_method) #@ + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="lro-methods-use-correct-naming", node=function_node_a + ), + pylint.testutils.Message( + msg_id="lro-methods-use-correct-naming", node=function_node_b + ), + ): + self.checker.visit_classdef(class_node) + self.checker.visit_return(return_node_a) + self.checker.visit_return(return_node_b) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#service-operations" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientConstructorDoesNotHaveConnectionStringParam(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientConstructorDoesNotHaveConnectionStringParam + + def test_ignores_client_with_no_conn_str_in_constructor(self): + class_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self): + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_non_client_methods(self): + class_node, function_node = astroid.extract_node(""" + class SomethingElse(): #@ + def __init__(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_finds_client_method_using_conn_str_in_constructor_a(self): + class_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, connection_string): + return list() + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="connection-string-should-not-be-constructor-param", node=class_node + ), + ): + self.checker.visit_classdef(class_node) + + def test_finds_client_method_using_conn_str_in_constructor_b(self): + class_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, conn_str): + return list() + """) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="connection-string-should-not-be-constructor-param", node=class_node + ), + ): + self.checker.visit_classdef(class_node) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestClientMethodNamesDoNotUseDoubleUnderscorePrefix(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ClientMethodNamesDoNotUseDoubleUnderscorePrefix + + def test_ignores_repr(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __repr__(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_constructor(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + def __init__(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_other_dunder(self): + class_node, function_node_a, function_node_b, function_node_c, function_node_d = astroid.extract_node(""" + class SomeClient(): #@ + def __enter__(self): #@ + pass + def __exit__(self): #@ + pass + def __aenter__(self): #@ + pass + def __aexit__(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node_a) + self.checker.visit_functiondef(function_node_b) + self.checker.visit_functiondef(function_node_c) + self.checker.visit_functiondef(function_node_d) + + def test_ignores_private_method(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + @staticmethod + def _private_method(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_private_method_async(self): + class_node, function_node = astroid.extract_node(""" + class SomeClient(): #@ + @staticmethod + async def _private_method(self, **kwargs): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(function_node) + + def test_ignores_methods_with_decorators(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + class SomeClient(): #@ + @distributed_trace + def create_configuration(self): #@ + pass + @distributed_trace + def get_thing(self): #@ + pass + @distributed_trace + def list_thing(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_functiondef(func_node_b) + self.checker.visit_functiondef(func_node_c) + + def test_ignores_async_methods_with_decorators(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + class SomeClient(): #@ + @distributed_trace_async + async def create_configuration(self): #@ + pass + @distributed_trace_async + async def get_thing(self): #@ + pass + @distributed_trace_async + async def list_thing(self): #@ + pass + """) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + self.checker.visit_asyncfunctiondef(func_node_c) + + def test_finds_double_underscore_on_async_method(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + class SomeClient(): #@ + @staticmethod + async def __create_configuration(self): #@ + pass + @staticmethod + async def __get_thing(self): #@ + pass + @staticmethod + async def __list_thing(self): #@ + pass + """) + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-name-no-double-underscore", node=func_node_a + ), + pylint.testutils.Message( + msg_id="client-method-name-no-double-underscore", node=func_node_b + ), + pylint.testutils.Message( + msg_id="client-method-name-no-double-underscore", node=func_node_c + ), + ): + self.checker.visit_asyncfunctiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + self.checker.visit_asyncfunctiondef(func_node_c) + + def test_finds_double_underscore_on_sync_method(self): + class_node, func_node_a, func_node_b, func_node_c = astroid.extract_node(""" + class SomeClient(): #@ + @staticmethod + def __create_configuration(self): #@ + pass + @staticmethod + def __get_thing(self): #@ + pass + @staticmethod + def __list_thing(self): #@ + pass + """) + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-method-name-no-double-underscore", node=func_node_a + ), + pylint.testutils.Message( + msg_id="client-method-name-no-double-underscore", node=func_node_b + ), + pylint.testutils.Message( + msg_id="client-method-name-no-double-underscore", node=func_node_c + ), + ): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_functiondef(func_node_b) + self.checker.visit_functiondef(func_node_c) + + def test_ignores_non_client_method(self): + class_node, func_node_a, func_node_b = astroid.extract_node( + """ + class SomethingElse(): #@ + @staticmethod + def __download_thing(self, some, **kwargs): #@ + pass + + @staticmethod + async def __do_thing(self, some, **kwargs): #@ + pass + """ + ) + with self.assertNoMessages(): + self.checker.visit_functiondef(func_node_a) + self.checker.visit_asyncfunctiondef(func_node_b) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_introduction.html#public-vs-private" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + + +class TestCheckDocstringAdmonitionNewline(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.CheckDocstringAdmonitionNewline + + def test_ignores_correct_admonition_statement_in_function(self): + function_node = astroid.extract_node( + """ + def function_foo(x, y, z): + '''docstring + .. admonition:: Example: + + .. literalinclude:: ../samples/sample_detect_language.py + ''' + """ + ) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_ignores_correct_admonition_statement_in_function_with_comments(self): + function_node = astroid.extract_node( + """ + def function_foo(x, y, z): + '''docstring + .. admonition:: Example: + This is Example content. + Should support multi-line. + Can also include file: + + .. literalinclude:: ../samples/sample_detect_language.py + ''' + """ + ) + + with self.assertNoMessages(): + self.checker.visit_functiondef(function_node) + + def test_bad_admonition_statement_in_function(self): + function_node = astroid.extract_node( + """ + def function_foo(x, y, z): + '''docstring + .. admonition:: Example: + .. literalinclude:: ../samples/sample_detect_language.py + ''' + """ + ) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="docstring-admonition-needs-newline", node=function_node + ) + ): + self.checker.visit_functiondef(function_node) + + def test_bad_admonition_statement_in_function_with_comments(self): + function_node = astroid.extract_node( + """ + def function_foo(x, y, z): + '''docstring + .. admonition:: Example: + This is Example content. + Should support multi-line. + Can also include file: + .. literalinclude:: ../samples/sample_detect_language.py + ''' + """ + ) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="docstring-admonition-needs-newline", node=function_node + ) + ): + self.checker.visit_functiondef(function_node) + + def test_ignores_correct_admonition_statement_in_function_async(self): + function_node = astroid.extract_node( + """ + async def function_foo(x, y, z): + '''docstring + .. admonition:: Example: + + .. literalinclude:: ../samples/sample_detect_language.py + ''' + """ + ) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(function_node) + + def test_ignores_correct_admonition_statement_in_function_with_comments_async(self): + function_node = astroid.extract_node( + """ + async def function_foo(x, y, z): + '''docstring + .. admonition:: Example: + This is Example content. + Should support multi-line. + Can also include file: + + .. literalinclude:: ../samples/sample_detect_language.py + ''' + """ + ) + + with self.assertNoMessages(): + self.checker.visit_asyncfunctiondef(function_node) + + def test_bad_admonition_statement_in_function_async(self): + function_node = astroid.extract_node( + """ + async def function_foo(x, y, z): + '''docstring + .. admonition:: Example: + .. literalinclude:: ../samples/sample_detect_language.py + ''' + """ + ) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="docstring-admonition-needs-newline", node=function_node + ) + ): + self.checker.visit_asyncfunctiondef(function_node) + + def test_bad_admonition_statement_in_function_with_comments_async(self): + function_node = astroid.extract_node( + """ + async def function_foo(x, y, z): + '''docstring + .. admonition:: Example: + This is Example content. + Should support multi-line. + Can also include file: + .. literalinclude:: ../samples/sample_detect_language.py + ''' + """ + ) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="docstring-admonition-needs-newline", node=function_node + ) + ): + self.checker.visit_asyncfunctiondef(function_node) + + def test_ignores_correct_admonition_statement_in_class(self): + class_node = astroid.extract_node( + """ + class SomeClient(object): + '''docstring + .. admonition:: Example: + + .. literalinclude:: ../samples/sample_detect_language.py + ''' + def __init__(self): + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_ignores_correct_admonition_statement_in_class_with_comments(self): + class_node = astroid.extract_node( + """ + class SomeClient(object): + '''docstring + .. admonition:: Example: + This is Example content. + Should support multi-line. + Can also include file: + + .. literalinclude:: ../samples/sample_detect_language.py + ''' + def __init__(self): + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_bad_admonition_statement_in_class(self): + class_node = astroid.extract_node( + """ + class SomeClient(object): + '''docstring + .. admonition:: Example: + .. literalinclude:: ../samples/sample_detect_language.py + ''' + def __init__(self): + pass + """ + ) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="docstring-admonition-needs-newline", node=class_node + ) + ): + self.checker.visit_classdef(class_node) + + def test_bad_admonition_statement_in_class_with_comments(self): + class_node = astroid.extract_node( + """ + class SomeClient(object): + '''docstring + .. admonition:: Example: + This is Example content. + Should support multi-line. + Can also include file: + .. literalinclude:: ../samples/sample_detect_language.py + ''' + def __init__(self): + pass + """ + ) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="docstring-admonition-needs-newline", node=class_node + ) + ): + self.checker.visit_classdef(class_node) \ No newline at end of file From b763e980158c026f3415d0dd95e981198ab50d0c Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 20 Apr 2022 14:33:10 -0700 Subject: [PATCH 2/4] Adding necessary files to enable CI --- .../stages/archetype-sdk-tool-python.yml | 4 ++ .../python-packages/api-stub-generator/ci.yml | 4 -- .../pylint-guidelines-checker/README.md | 72 +++++++++++++++++++ .../pylint-guidelines-checker/ci.yml | 43 +++++++++++ .../dev_requirements.txt | 3 + 5 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/README.md create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/ci.yml create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/dev_requirements.txt diff --git a/eng/pipelines/templates/stages/archetype-sdk-tool-python.yml b/eng/pipelines/templates/stages/archetype-sdk-tool-python.yml index 0d89e9007b0..efe8a50a604 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-tool-python.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-tool-python.yml @@ -62,6 +62,10 @@ stages: vmImage: MMSUbuntu20.04 steps: + - template: /eng/pipelines/templates/steps/use-python-version.yml + parameters: + versionSpec: '${{ parameters.PythonVersion }}' + - ${{ parameters.TestSteps }} - ${{if and(eq(variables['Build.Reason'], 'Manual'), eq(variables['System.TeamProject'], 'internal'))}}: diff --git a/packages/python-packages/api-stub-generator/ci.yml b/packages/python-packages/api-stub-generator/ci.yml index c7481018f7b..07ad50a4e1a 100644 --- a/packages/python-packages/api-stub-generator/ci.yml +++ b/packages/python-packages/api-stub-generator/ci.yml @@ -30,10 +30,6 @@ extends: ArtifactName: 'apiviewparserpython' PackageName: 'Python API View Parser' TestSteps: - - template: /eng/pipelines/templates/steps/use-python-version.yml - parameters: - versionSpec: '3.10.2' - - script: | python $(Build.SourcesDirectory)/eng/scripts/python_version_check.py displayName: 'Verify Python APIView version consistency' diff --git a/tools/pylint-extensions/pylint-guidelines-checker/README.md b/tools/pylint-extensions/pylint-guidelines-checker/README.md new file mode 100644 index 00000000000..51cd3e2d4c7 --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/README.md @@ -0,0 +1,72 @@ +# Linting the Guidelines + +The code presented in this repository is a lift-and-shift of code [originally written by @kristapratico](https://github.com/Azure/azure-sdk-for-python/blob/ae2948416956938c908dcfb570a7456b23482149/scripts/pylint_custom_plugin/) + +## Running guidelines checks in Azure/azure-sdk-for-python repository + +In order to lint for the guidelines, you must make sure you are enabling the `pylint-guidelines-checker` in your pylint invocation. Using the rcfile at the [root of the repo](https://github.com/Azure/azure-sdk-for-python/blob/main/pylintrc) will ensure that the plugin is properly activated + +It is recommended you run pylint at the library package level to be consistent with how the CI runs pylint. + +Check that you are running pylint version >=2.5.2 and astroid version >=2.4.1. + + +0. Install the pylint checker from the azure-sdk development feed. + ```bash + pip install --index-url="https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-python/pypi/simple/" pylint-guidelines-checker + ``` + +1. Run pylint at the root of the repo and it will automatically find the pylintrc: + ```bash + C:\azure-sdk-for-python>pylint sdk/storage/azure-storage-blob/azure + ``` +2. Add the --rcfile command line argument with a relative path to the pylintrc from your current directory: + ```bash + C:\azure-sdk-for-python\sdk\storage>pylint --rcfile="../../pylintrc" azure-storage-blob + ``` +3. Set the environment variable PYLINTRC to the absolute path of the pylintrc file: + ```bash + set PYLINTRC=C:\azure-sdk-for-python\pylintrc + ``` + Run pylint: + ```bash + C:\azure-sdk-for-python\sdk\storage>pylint azure-storage-blob + ``` +4. Run pylint at the package level using tox and it will find the pylintrc file: + ```bash + pip install tox tox-monorepo + C:\azure-sdk-for-python\sdk\storage\azure-storage-blob>tox -e lint -c ../../../eng/tox/tox.ini + ``` +5. If you use the pylint extension for VS code or Pycharm it *should* find the pylintrc automatically. + +## How to disable a pylint error + +```bash +# pylint:disable=connection-string-should-not-be-constructor-param +``` + +The pylint custom checkers for SDK guidelines fall into messages range C4717 - C4738. +You will know you came across a custom checker if it contains a link to the guidelines. + +In the case of a false positive, use the disable command to remove the pylint error. + +## Rules List + +| Pylint checker name | How to fix this | How to disable this rule | Link to python guideline | +|----------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------| +| client-method-should-not-use-static-method | Use module level functions instead. | # pylint:disable=connection-string-should-not-be-constructor-param | [link](https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods) | +| missing-client-constructor-parameter-credential | Add a credential parameter to the client constructor. Do not use plural form "credentials". | # pylint:disable=missing-client-constructor-parameter-credential | [link](https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods) | +| missing-client-constructor-parameter-kwargs | Add a **kwargs parameter to the client constructor. | # pylint:disable=missing-client-constructor-parameter-kwargs | [link](https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods) | +| client-method-has-more-than-5-positional-arguments | Use keyword arguments to reduce number of positional arguments. | # pylint:disable=client-method-has-more-than-5-positional-arguments | [link]((https://azure.github.io/azure-sdk/python_design.html#method-signatures) | +| client-method-missing-type-annotations | Check that param/return type comments are present or that param/return type annotations are present. Check that you did not mix type comments with type annotations. | # pylint:disable=client-method-missing-type-annotations | [link]((https://azure.github.io/azure-sdk/python_design.html#types-or-not) | +| client-incorrect-naming-convention | Check that you use... snake_case for variable, function, and method names. Pascal case for types. ALL CAPS for constants. | # pylint:disable=client-incorrect-naming-convention | [link]((https://azure.github.io/azure-sdk/python_design.html#naming-conventions) | +| client-method-missing-kwargs | Check that any methods that make network calls have a **kwargs parameter. | # pylint:disable=client-method-missing-kwargs | [link](https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods) | +| config-missing-kwargs-in-policy | Check that the policies in your configuration function contain a **kwargs parameter. | # pylint:disable=config-missing-kwargs-in-policy | [link](https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods) | +| async-client-bad-name | Remove "Async" from your service client's name. | # pylint:disable=async-client-bad-name | [link](https://azure.github.io/azure-sdk/python_design.html#async-support) | +| file-needs-copyright-header | Add a copyright header to the top of your file. | # pylint:disable=file-needs-copyright-header | [link](https://azure.github.io/azure-sdk/policies_opensource.html) | +| client-method-name-no-double-underscore | Don't use method names prefixed with "__". | # pylint:disable=client-method-name-no-double-underscore | [link]((https://azure.github.io/azure-sdk/python_design.html#public-vs-private) | +| specify-parameter-names-in-call | Specify the parameter names when calling methods with more than 2 required positional parameters. e.g. self.get_foo(one, two, three=three, four=four, five=five) | # pylint:disable=specify-parameter-names-in-call | [link]((https://azure.github.io/azure-sdk/python_design.html#method-signatures) | +| connection-string-should-not-be-constructor-param | Remove connection string parameter from client constructor. Create a method that creates the client using a connection string. | # pylint:disable=connection-string-should-not-be-constructor-param | [link](https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods) | +| package-name-incorrect | Change your distribution package name to only include dashes, e.g. azure-storage-file-share | # pylint:disable=package-name-incorrect | [link](https://azure.github.io/azure-sdk/python_implementation.html#packaging) | +| client-suffix-needed | Service client types should use a "Client" suffix, e.g. BlobClient. | # pylint:disable=client-suffix-needed | [link](https://azure.github.io/azure-sdk/python_design.html#clients) | +| docstring-admonition-needs-newline | Add a blank newline above the .. literalinclude statement. | # pylint:disable=docstring-admonition-needs-newline | No guideline, just helps our docs get built correctly for microsoft docs. | \ No newline at end of file diff --git a/tools/pylint-extensions/pylint-guidelines-checker/ci.yml b/tools/pylint-extensions/pylint-guidelines-checker/ci.yml new file mode 100644 index 00000000000..94057ce6a35 --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/ci.yml @@ -0,0 +1,43 @@ +# NOTE: Please refer to https://aka.ms/azsdk/engsys/ci-yaml before editing this file. +trigger: + branches: + include: + - main + - feature/* + - release/* + - hotfix/* + paths: + include: + - tools/pylint-extensions/pylint_guidelines_checker + +pr: + branches: + include: + - main + - feature/* + - release/* + - hotfix/* + paths: + include: + - tools/pylint-extensions/pylint_guidelines_checker + +extends: + template: /eng/pipelines/templates/stages/archetype-sdk-tool-python.yml + parameters: + PythonVersion: '3.10.2' + PackagePath: 'tools/pylint-extensions/pylint_guidelines_checker' + FeedName: 'public/azure-sdk-for-python' + ArtifactName: 'azsdkpylintplugin' + PackageName: 'Azure SDK Pylint Plugin' + TestSteps: + - script: | + pip install -r dev_requirements.txt + pip install -e . + displayName: 'Install Test Requirements' + workingDirectory: $(Build.SourcesDirectory)/tools/pylint-extensions/pylint_guidelines_checker + + - script: | + pytest . + displayName: 'Test Python Linter' + workingDirectory: $(Build.SourcesDirectory)/tools/pylint-extensions/pylint_guidelines_checker + continueOnError: true diff --git a/tools/pylint-extensions/pylint-guidelines-checker/dev_requirements.txt b/tools/pylint-extensions/pylint-guidelines-checker/dev_requirements.txt new file mode 100644 index 00000000000..91ae092b89b --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/dev_requirements.txt @@ -0,0 +1,3 @@ +pylint==2.5.2 +azure-core==1.23.1 +pytest==7.1.1 \ No newline at end of file From fded0e9e2f255ecc825c5e18123f105c034c6af3 Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Wed, 20 Apr 2022 14:50:40 -0700 Subject: [PATCH 3/4] Moving pylint checker updates from sdk-for-python into sdk-tools. --- .../pylint_guidelines_checker.py | 243 +++++++++- .../pylint-guidelines-checker/setup.py | 2 +- .../tests/__init__.py | 0 .../tests/test_files/__init__.py | 9 + .../api_version_checker_acceptable_class.py | 24 + .../api_version_checker_acceptable_init.py | 17 + .../api_version_checker_violation.py | 13 + .../test_files/copyright_header_acceptable.py | 9 + .../test_files/copyright_header_violation.py | 3 + .../test_files/enum_checker_acceptable.py | 8 + .../test_files/enum_checker_violation.py | 5 + .../tests/test_pylint_custom_plugins.py | 426 +++++++++++++++++- 12 files changed, 732 insertions(+), 27 deletions(-) create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/tests/__init__.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/__init__.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_acceptable_class.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_acceptable_init.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_violation.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/copyright_header_acceptable.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/copyright_header_violation.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/enum_checker_acceptable.py create mode 100644 tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/enum_checker_violation.py diff --git a/tools/pylint-extensions/pylint-guidelines-checker/pylint_guidelines_checker.py b/tools/pylint-extensions/pylint-guidelines-checker/pylint_guidelines_checker.py index e62a6829625..3ee93d8c3e9 100644 --- a/tools/pylint-extensions/pylint-guidelines-checker/pylint_guidelines_checker.py +++ b/tools/pylint-extensions/pylint-guidelines-checker/pylint_guidelines_checker.py @@ -22,13 +22,13 @@ class ClientConstructorTakesCorrectParameters(BaseChecker): msgs = { "C4717": ( "Client constructor is missing a credential parameter. See details:" - " https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods", + " https://azure.github.io/azure-sdk/python_design.html#client-configuration", "missing-client-constructor-parameter-credential", "All client types should accept a credential parameter.", ), "C4718": ( "Client constructor is missing a **kwargs parameter. See details:" - " https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods", + " https://azure.github.io/azure-sdk/python_design.html#client-configuration", "missing-client-constructor-parameter-kwargs", "All client types should accept a **kwargs parameter.", ) @@ -91,7 +91,7 @@ class ClientHasKwargsInPoliciesForCreateConfigurationMethod(BaseChecker): msgs = { "C4719": ( "A policy in the create_configuration() function is missing a **kwargs argument. See details:" - " https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods", + " https://azure.github.io/azure-sdk/python_design.html#client-configuration", "config-missing-kwargs-in-policy", "All policies should take a **kwargs parameter.", ) @@ -205,7 +205,7 @@ class ClientMethodsUseKwargsWithMultipleParameters(BaseChecker): msgs = { "C4721": ( "Client has too many positional arguments. Use keyword-only arguments." - " See details: https://azure.github.io/azure-sdk/python_introduction.html#method-signatures", + " See details: https://azure.github.io/azure-sdk/python_implementation.html#method-signatures", "client-method-has-more-than-5-positional-arguments", "Client method should use keyword-only arguments for some parameters.", ) @@ -260,7 +260,7 @@ class ClientMethodsHaveTypeAnnotations(BaseChecker): "C4722": ( "Client method is missing type annotations/comments, return type annotations/comments, or " "mixing type annotations and comments. See details: " - " https://azure.github.io/azure-sdk/python_introduction.html#types-or-not", + " https://azure.github.io/azure-sdk/python_implementation.html#types-or-not", "client-method-missing-type-annotations", "Client method should use type annotations.", ) @@ -412,7 +412,7 @@ class ClientsDoNotUseStaticMethods(BaseChecker): msgs = { "C4725": ( "Client should not use static methods (staticmethod). See details:" - " https://azure.github.io/azure-sdk/python_introduction.html#method-signatures", + " https://azure.github.io/azure-sdk/python_implementation.html#method-signatures", "client-method-should-not-use-static-method", "Client method should not use staticmethod.", ), @@ -463,7 +463,7 @@ class FileHasCopyrightHeader(BaseChecker): msgs = { "C4726": ( "File is missing a copyright header. See details:" - " https://azure.github.io/azure-sdk/policies_opensource.html", + " https://azure.github.io/azure-sdk/policies_opensource.html#", "file-needs-copyright-header", "Every source file should have a copyright header.", ), @@ -510,7 +510,7 @@ class ClientUsesCorrectNamingConventions(BaseChecker): msgs = { "C4727": ( "Client is using an incorrect naming convention. See details:" - " https://azure.github.io/azure-sdk/python_introduction.html#naming-conventions", + " https://azure.github.io/azure-sdk/python_implementation.html#naming-conventions", "client-incorrect-naming-convention", "Client method should use correct naming conventions.", ) @@ -636,7 +636,7 @@ class ClientMethodNamesDoNotUseDoubleUnderscorePrefix(BaseChecker): msgs = { "C4729": ( "Client method name should not use a double underscore prefix. See details:" - " https://azure.github.io/azure-sdk/python_introduction.html#public-vs-private", + " https://azure.github.io/azure-sdk/python_implementation.html#public-vs-private", "client-method-name-no-double-underscore", "Client method names should not use a leading double underscore prefix.", ), @@ -804,7 +804,7 @@ class SpecifyParameterNamesInCall(BaseChecker): msgs = { "C4732": ( "Specify the parameter names when calling methods with more than 2 required positional parameters." - " See details: https://azure.github.io/azure-sdk/python_introduction.html#method-signatures", + " See details: https://azure.github.io/azure-sdk/python_implementation.html#python-codestyle-positional-params", "specify-parameter-names-in-call", "You should specify the parameter names when the method has more than two positional arguments.", ) @@ -1023,7 +1023,7 @@ class ClientConstructorDoesNotHaveConnectionStringParam(BaseChecker): msgs = { "C4736": ( "The constructor must not take a connection string. See details: " - "https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods", + "https://azure.github.io/azure-sdk/python_design.html#python-client-connection-string", "connection-string-should-not-be-constructor-param", "Client should have a method to create the client with a connection string.", ), @@ -1074,7 +1074,7 @@ class PackageNameDoesNotUseUnderscoreOrPeriod(BaseChecker): msgs = { "C4737": ( "Package name should not use an underscore or period. Replace with dash (-). See details: " - "https://azure.github.io/azure-sdk/python_implementation.html#packaging", + "https://azure.github.io/azure-sdk/python_design.html#packaging", "package-name-incorrect", "Package name should use dashes instead of underscore or period.", ), @@ -1124,7 +1124,7 @@ class ServiceClientUsesNameWithClientSuffix(BaseChecker): msgs = { "C4738": ( "Service client types should use a `Client` suffix. See details: " - "https://azure.github.io/azure-sdk/python_design.html#clients", + "https://azure.github.io/azure-sdk/python_design.html#service-client", "client-suffix-needed", "Client should use the correct suffix.", ), @@ -1706,6 +1706,217 @@ def visit_functiondef(self, node): visit_asyncfunctiondef = visit_functiondef +class CheckEnum(BaseChecker): + __implements__ = IAstroidChecker + + name = "check-enum" + priority = -1 + msgs = { + "C4746": ( + "The enum must use uppercase naming. " + "https://azure.github.io/azure-sdk/python_design.html#enumerations", + "enum-must-be-uppercase", + "Capitalize enum name.", + ), + "C4747":( + "The enum must inherit from CaseInsensitiveEnumMeta. " + "https://azure.github.io/azure-sdk/python_implementation.html#extensible-enumerations", + "enum-must-inherit-case-insensitive-enum-meta", + "Inherit CaseInsensitiveEnumMeta.", + ), + } + options = ( + ( + "ignore-enum-must-be-uppercase", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow an enum to not be capitalized.", + }, + ), + ( + "ignore-enum-must-inherit-case-insensitive-enum-meta", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow an enum to not inherit CaseInsensitiveEnumMeta.", + }, + ), + ) + + def __init__(self, linter=None): + super(CheckEnum, self).__init__(linter) + + def visit_classdef(self, node): + """Visits every enum class. + + :param node: ast.ClassDef + :return: None + """ + try: + + # If it has a metaclass, and is an enum class, check the capitalization + if node.declared_metaclass(): + if node.declared_metaclass().name == "CaseInsensitiveEnumMeta": + self._enum_uppercase(node) + # Else if it does not have a metaclass, but it is an enum class + # Check both capitalization and throw pylint error for metaclass + elif node.bases[0].name == "str" and node.bases[1].name == "Enum": + self.add_message( + "enum-must-inherit-case-insensitive-enum-meta", node=node, confidence=None + ) + self._enum_uppercase(node) + + except Exception: + logger.debug("Pylint custom checker failed to check enum.") + pass + + def _enum_uppercase(self, node): + """Visits every enum within the class. + Checks if the enum is uppercase, if it isn't it + adds a pylint error message. + + :param node: ast.ClassDef + :return: None + """ + + # Check capitalization of enums assigned in the class + for nod in node.body: + if isinstance(nod, astroid.Assign): + if not nod.targets[0].name.isupper(): + self.add_message( + "enum-must-be-uppercase", node=nod.targets[0], confidence=None + ) + + +class CheckAPIVersion(BaseChecker): + __implements__ = IAstroidChecker + + name = "check-api-version-kwarg" + priority = -1 + msgs = { + "C4748": ( + "The client constructor needs to take in an optional keyword-only api_version argument. " + "https://azure.github.io/azure-sdk/python_design.html#specifying-the-service-version", + "client-accepts-api-version-keyword", + "Accept a keyword argument called api_version.", + ), + } + options = ( + ( + "ignore-client-accepts-api-version-keyword", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow for no keyword api version.", + }, + ), + ) + ignore_clients = ["PipelineClient", "AsyncPipelineClient", "ARMPipelineClient", "AsyncARMPipelineClient"] + + def __init__(self, linter=None): + super(CheckAPIVersion, self).__init__(linter) + + def visit_classdef(self, node): + """Visits every class in file and checks if it is a client. + If it is a client, it checks that there is an api_version keyword. + + :param node: class node + :type node: ast.ClassDef + :return: None + """ + + try: + api_version = False + + if node.name.endswith("Client") and node.name not in self.ignore_clients: + if node.doc: + if ":keyword api_version:" in node.doc or ":keyword str api_version:" in node.doc: + api_version = True + if not api_version: + for func in node.body: + if isinstance(func, astroid.FunctionDef): + if func.name == '__init__': + if func.doc: + if ":keyword api_version:" in func.doc or ":keyword str api_version:" in func.doc: + api_version = True + if not api_version: + self.add_message( + msgid="client-accepts-api-version-keyword", node=node, confidence=None + ) + + + except AttributeError: + logger.debug("Pylint custom checker failed to check if client takes in an optional keyword-only api_version argument.") + pass + + +class CheckNamingMismatchGeneratedCode(BaseChecker): + __implements__ = IAstroidChecker + + name = "check-naming-mismatch" + priority = -1 + msgs = { + "C4745": ( + "Do not alias generated code. " + "This messes up sphinx, intellisense, and apiview, so please modify the name of the generated code through" + " the swagger / directives, or code customizations. See Details: " + "https://github.com/Azure/autorest/blob/main/docs/generate/built-in-directives.md", + "naming-mismatch", + "Do not alias models imported from the generated code.", + ), + } + options = ( + ( + "ignore-naming-mismatch", + { + "default": False, + "type": "yn", + "metavar": "", + "help": "Allow generated code to be aliased.", + }, + ), + ) + + def __init__(self, linter=None): + super(CheckNamingMismatchGeneratedCode, self).__init__(linter) + + def visit_module(self, node): + """Visits __init__.py and checks that there are not aliased models. + + :param node: module node + :type node: ast.Module + :return: None + """ + try: + if node.file.endswith("__init__.py"): + aliased = [] + + for nod in node.body: + if isinstance(nod, astroid.ImportFrom) or isinstance(nod, astroid.Import): + # If the model has been aliased + for name in nod.names: + if name[1] is not None: + aliased.append(name[1]) + + for nod in node.body: + if isinstance(nod, astroid.Assign): + if nod.targets[0].as_string() == "__all__": + for models in nod.assigned_stmts(): + for model_name in models.elts: + if model_name.value in aliased: + self.add_message( + msgid="naming-mismatch", node=model_name, confidence=None + ) + + except Exception: + logger.debug("Pylint custom checker failed to check if model is aliased.") + pass + + # if a linter is registered in this function then it will be checked with pylint def register(linter): linter.register_checker(ClientsDoNotUseStaticMethods(linter)) @@ -1723,6 +1934,10 @@ def register(linter): linter.register_checker(PackageNameDoesNotUseUnderscoreOrPeriod(linter)) linter.register_checker(ServiceClientUsesNameWithClientSuffix(linter)) linter.register_checker(CheckDocstringAdmonitionNewline(linter)) + linter.register_checker(CheckNamingMismatchGeneratedCode(linter)) + linter.register_checker(CheckAPIVersion(linter)) + linter.register_checker(CheckEnum(linter)) + # disabled by default, use pylint --enable=check-docstrings if you want to use it linter.register_checker(CheckDocstringParameters(linter)) @@ -1735,3 +1950,5 @@ def register(linter): # linter.register_checker(ClientListMethodsUseCorePaging(linter)) # linter.register_checker(ClientLROMethodsUseCorePolling(linter)) # linter.register_checker(ClientLROMethodsUseCorrectNaming(linter)) + + diff --git a/tools/pylint-extensions/pylint-guidelines-checker/setup.py b/tools/pylint-extensions/pylint-guidelines-checker/setup.py index 36d8d461c97..b4b89126e95 100644 --- a/tools/pylint-extensions/pylint-guidelines-checker/setup.py +++ b/tools/pylint-extensions/pylint-guidelines-checker/setup.py @@ -3,7 +3,7 @@ setup( name="pylint-guidelines-checker", version="0.0.1", - url='http://github.com/Azure/azure-sdk-tools', + url='http://github.com/Azure/azure-sdk-for-python', license='MIT License', description="A pylint plugin which enforces azure sdk guidelines.", author='Microsoft Corporation', diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/__init__.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/__init__.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/__init__.py new file mode 100644 index 00000000000..d75603086ed --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/__init__.py @@ -0,0 +1,9 @@ +# Test for CheckNamingMismatchGeneratedCode + +from something import Something +from something2 import something2 as somethingTwo + +__all__ = ( + Something, + somethingTwo, #pylint: disable=naming-mismatch +) diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_acceptable_class.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_acceptable_class.py new file mode 100644 index 00000000000..930f4a79785 --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_acceptable_class.py @@ -0,0 +1,24 @@ +#Test file for api_verison checker + + +class SomeClient(): + + """ + :param str endpoint: Something. + :keyword api_version: + The API version of the service to use for requests. It defaults to API version v2.1. + Setting to an older version may result in reduced feature compatibility. To use the + latest supported API version and features, instantiate a DocumentAnalysisClient instead. + :paramtype api_version: str + :param credential: Something. + :type credential: : TokenCredential. + :keyword key : Something2. + + + """ + + def __init__(self, endpoint, credential, **kwargs): + pass + + + diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_acceptable_init.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_acceptable_init.py new file mode 100644 index 00000000000..6cddc4dfa81 --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_acceptable_init.py @@ -0,0 +1,17 @@ +#Test file for api_verison checker + +class SomeClient(): + + def __init__(self, endpoint, credential, **kwargs): + """ + :param str endpoint: Something. + :param credential: Something. + :type credential: : Something. + :keyword api_version: + The API version of the service to use for requests. It defaults to API version v2.1. + Setting to an older version may result in reduced feature compatibility. To use the + latest supported API version and features, instantiate a DocumentAnalysisClient instead. + :paramtype api_version: str + + """ + pass diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_violation.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_violation.py new file mode 100644 index 00000000000..d327d3fed54 --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/api_version_checker_violation.py @@ -0,0 +1,13 @@ +# Test file for api version checker + + +class SomeClient(): + + def __init__(self, endpoint, credential, **kwargs): + """ + :param str endpoint: Something. + :param credential: Something. + :type credential: TokenCredential. + + """ + pass diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/copyright_header_acceptable.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/copyright_header_acceptable.py new file mode 100644 index 00000000000..984a3f7aca2 --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/copyright_header_acceptable.py @@ -0,0 +1,9 @@ +# ------------------------------------ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. +# ------------------------------------ + + +class Something(): + def __init__(self): + pass diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/copyright_header_violation.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/copyright_header_violation.py new file mode 100644 index 00000000000..d37cab410d8 --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/copyright_header_violation.py @@ -0,0 +1,3 @@ +class Something(): + def __init__(self): + pass diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/enum_checker_acceptable.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/enum_checker_acceptable.py new file mode 100644 index 00000000000..1701bada680 --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/enum_checker_acceptable.py @@ -0,0 +1,8 @@ +# Test file for enum checker +from enum import Enum +from six import with_metaclass +from azure.core import CaseInsensitiveEnumMeta + +class EnumPython2(with_metaclass(CaseInsensitiveEnumMeta, str, Enum)): + ONE = "one" + TWO = "two" diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/enum_checker_violation.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/enum_checker_violation.py new file mode 100644 index 00000000000..a12b64442df --- /dev/null +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_files/enum_checker_violation.py @@ -0,0 +1,5 @@ +# Test file for enum checker +from enum import Enum + +class EnumPython(str, Enum): + One = "one" diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py index 7527ec9ea3d..91ef6c3b1b2 100644 --- a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py @@ -5,10 +5,11 @@ import astroid import pylint.testutils +import requests from azure.core import PipelineClient from azure.core.configuration import Configuration -import pylint_guidelines_checker as checker +from pylint_custom_plugin import pylint_guidelines_checker as checker class TestClientMethodsHaveTracingDecorators(pylint.testutils.CheckerTestCase): CHECKER_CLASS = checker.ClientMethodsHaveTracingDecorators @@ -397,7 +398,7 @@ async def do_thing(self, some, **kwargs): #@ self.checker.visit_asyncfunctiondef(func_node_b) def test_guidelines_link_active(self): - url = "https://azure.github.io/azure-sdk/python_introduction.html#method-signatures" + url = "https://azure.github.io/azure-sdk/python_implementation.html#method-signatures" config = Configuration() client = PipelineClient(url, config=config) request = client.get(url) @@ -686,7 +687,7 @@ def __init__(self): #@ self.checker.visit_functiondef(function_node) def test_guidelines_link_active(self): - url = "https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods" + url = "https://azure.github.io/azure-sdk/python_design.html#client-configuration" config = Configuration() client = PipelineClient(url, config=config) request = client.get(url) @@ -921,7 +922,7 @@ async def do_thing(self, one, two, three, four, five, six): #@ self.checker.visit_asyncfunctiondef(function_node_b) def test_guidelines_link_active(self): - url = "https://azure.github.io/azure-sdk/python_introduction.html#method-signatures" + url = "https://azure.github.io/azure-sdk/python_implementation.html#method-signatures" config = Configuration() client = PipelineClient(url, config=config) request = client.get(url) @@ -1198,7 +1199,7 @@ def _do_thing(self, one, two, three, four, five, six): #@ self.checker.visit_functiondef(function_node) def test_guidelines_link_active(self): - url = "https://azure.github.io/azure-sdk/python_introduction.html#types-or-not" + url = "https://azure.github.io/azure-sdk/python_implementation.html#types-or-not" config = Configuration() client = PipelineClient(url, config=config) request = client.get(url) @@ -1301,7 +1302,7 @@ def some_other_method(credential, api_version=None, **kwargs): #@ self.checker.visit_functiondef(function_node_b) def test_guidelines_link_active(self): - url = "https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods" + url = "https://azure.github.io/azure-sdk/python_design.html#client-configuration" config = Configuration() client = PipelineClient(url, config=config) request = client.get(url) @@ -1526,7 +1527,7 @@ class SomeClient(): #@ self.checker.visit_classdef(class_node) def test_guidelines_link_active(self): - url = "https://azure.github.io/azure-sdk/python_introduction.html#naming-conventions" + url = "https://azure.github.io/azure-sdk/python_implementation.html#naming-conventions" config = Configuration() client = PipelineClient(url, config=config) request = client.get(url) @@ -1732,7 +1733,25 @@ def test_guidelines_link_active(self): class TestFileHasCopyrightHeader(pylint.testutils.CheckerTestCase): CHECKER_CLASS = checker.FileHasCopyrightHeader - # Unable to use the astroid for this testcase. + def test_copyright_header_acceptable(self): + file = open("./test_files/copyright_header_acceptable.py") + node = astroid.parse(file.read()) + file.close() + + with self.assertNoMessages(): + self.checker.visit_module(node) + + def test_copyright_header_violation(self): + file = open("./test_files/copyright_header_violation.py") + node = astroid.parse(file.read()) + file.close() + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="file-needs-copyright-header", node=node + ) + ): + self.checker.visit_module(node) def test_guidelines_link_active(self): url = "https://azure.github.io/azure-sdk/policies_opensource.html" @@ -1830,7 +1849,7 @@ def do_thing(self): self.checker.visit_call(call_node) def test_guidelines_link_active(self): - url = "https://azure.github.io/azure-sdk/python_introduction.html#method-signatures" + url = "https://azure.github.io/azure-sdk/python_implementation.html#python-codestyle-positional-params" config = Configuration() client = PipelineClient(url, config=config) request = client.get(url) @@ -2150,7 +2169,90 @@ def __init__(self, conn_str): self.checker.visit_classdef(class_node) def test_guidelines_link_active(self): - url = "https://azure.github.io/azure-sdk/python_design.html#constructors-and-factory-methods" + url = "https://azure.github.io/azure-sdk/python_design.html#python-client-connection-string" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + +class TestPackageNameDoesNotUseUnderscoreOrPeriod(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.PackageNameDoesNotUseUnderscoreOrPeriod + + def test_package_name_acceptable(self): + + package_name = astroid.extract_node( + """ + PACKAGE_NAME = "correct-package-name" + """ + ) + module_node = astroid.Module(name = "node", file="setup.py", doc = """ """) + module_node.body = [package_name] + + with self.assertNoMessages(): + self.checker.visit_module(module_node) + + def test_package_name_violation(self): + + package_name = astroid.extract_node( + """ + PACKAGE_NAME = "incorrect.package-name" + """ + ) + module_node = astroid.Module(name = "node", file="setup.py", doc = """ """) + module_node.body = [package_name] + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="package-name-incorrect", node=module_node + ) + ): + self.checker.visit_module(module_node) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#packaging" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + +class TestServiceClientUsesNameWithClientSuffix(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.ServiceClientUsesNameWithClientSuffix + + def test_client_suffix_acceptable(self): + client_node = astroid.extract_node( + """ + class MyClient(): + def __init__(self): + pass + """ + ) + module_node = astroid.Module(name = "node", file="_my_client.py", doc = """ """) + module_node.body = [client_node] + + with self.assertNoMessages(): + self.checker.visit_module(module_node) + + def test_client_suffix_violation(self): + client_node = astroid.extract_node( + """ + class Violation(): + def __init__(self): + pass + """ + ) + module_node = astroid.Module(name = "node", file="_my_client.py", doc = """ """) + module_node.body = [client_node] + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-suffix-needed", node=module_node + ) + ): + self.checker.visit_module(module_node) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#service-client" config = Configuration() client = PipelineClient(url, config=config) request = client.get(url) @@ -2334,14 +2436,13 @@ async def __do_thing(self, some, **kwargs): #@ self.checker.visit_asyncfunctiondef(func_node_b) def test_guidelines_link_active(self): - url = "https://azure.github.io/azure-sdk/python_introduction.html#public-vs-private" + url = "https://azure.github.io/azure-sdk/python_implementation.html#public-vs-private" config = Configuration() client = PipelineClient(url, config=config) request = client.get(url) response = client._pipeline.run(request) assert response.http_response.status_code == 200 - class TestCheckDocstringAdmonitionNewline(pylint.testutils.CheckerTestCase): CHECKER_CLASS = checker.CheckDocstringAdmonitionNewline @@ -2567,4 +2668,303 @@ def __init__(self): msg_id="docstring-admonition-needs-newline", node=class_node ) ): - self.checker.visit_classdef(class_node) \ No newline at end of file + self.checker.visit_classdef(class_node) + + +class TestCheckNamingMismatchGeneratedCode(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.CheckNamingMismatchGeneratedCode + + def test_import_naming_mismatch_violation(self): + import_one = astroid.extract_node( + 'import Something' + + ) + import_two = astroid.extract_node( + 'import Something2 as SomethingTwo' + + ) + assign_one = astroid.extract_node( + """ + __all__ =( + "Something", + "SomethingTwo", + ) + """ + ) + + module_node = astroid.Module(name = "node", file="__init__.py", doc = """ """) + module_node.body = [import_one,import_two,assign_one] + + for name in module_node.body[-1].assigned_stmts(): + err_node = name.elts[1] + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="naming-mismatch", node=err_node ,confidence=None + ) + ): + self.checker.visit_module(module_node) + + def test_import_from_naming_mismatch_violation(self): + import_one = astroid.extract_node( + 'import Something' + + ) + import_two = astroid.extract_node( + 'from Something2 import SomethingToo as SomethingTwo' + + ) + assign_one = astroid.extract_node( + """ + __all__ =( + "Something", + "SomethingTwo", + ) + """ + ) + + module_node = astroid.Module(name = "node", file="__init__.py", doc = """ """) + module_node.body = [import_one,import_two,assign_one] + + for name in module_node.body[-1].assigned_stmts(): + err_node = name.elts[1] + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="naming-mismatch", node=err_node ,confidence=None + ) + ): + self.checker.visit_module(module_node) + + def test_naming_mismatch_acceptable(self): + import_one = astroid.extract_node( + 'import Something' + + ) + import_two = astroid.extract_node( + 'import Something2 as SomethingTwo' + + ) + assign_one = astroid.extract_node( + """ + __all__ =( + "Something", + "Something2", + ) + """ + ) + + module_node = astroid.Module(name = "node", file="__init__.py", doc = """ """) + module_node.body = [import_one,import_two,assign_one] + + with self.assertNoMessages(): + self.checker.visit_module(module_node) + + def test_naming_mismatch_pylint_disable(self): + + file = open("./test_files/__init__.py") + node = astroid.parse(file.read()) + file.close() + + with self.assertNoMessages(): + self.checker.visit_module(node) + + def test_guidelines_link_active(self): + url = "https://github.com/Azure/autorest/blob/main/docs/generate/built-in-directives.md" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 + +class TestCheckEnum(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.CheckEnum + + def test_ignore_normal_class(self): + class_node = astroid.extract_node( + """ + class SomeClient(object): + my_list = [] + """ + ) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_enum_capitalized_violation_python_two(self): + class_node = astroid.extract_node( + """ + from enum import Enum + from six import with_metaclass + from azure.core import CaseInsensitiveEnumMeta + + class MyBadEnum(with_metaclass(CaseInsensitiveEnumMeta, str, Enum)): + One = "one" + """ + ) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="enum-must-be-uppercase", node=class_node.body[0].targets[0] + ) + ): + self.checker.visit_classdef(class_node) + + def test_enum_capitalized_violation_python_three(self): + class_node = astroid.extract_node( + """ + from enum import Enum + from azure.core import CaseInsensitiveEnumMeta + + class MyBadEnum(str, Enum, metaclass=CaseInsensitiveEnumMeta): + One = "one" + + """ + ) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="enum-must-be-uppercase", node=class_node.body[0].targets[0] + ) + ): + self.checker.visit_classdef(class_node) + + def test_inheriting_case_insensitive_violation(self): + class_node = astroid.extract_node( + """ + from enum import Enum + + class MyGoodEnum(str, Enum): + ONE = "one" + """ + ) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="enum-must-inherit-case-insensitive-enum-meta", node=class_node ) + ): + self.checker.visit_classdef(class_node) + + def test_acceptable_python_three(self): + class_node = astroid.extract_node( + """ + from enum import Enum + from azure.core import CaseInsensitiveEnumMeta + + class MyGoodEnum(str, Enum, metaclass=CaseInsensitiveEnumMeta): + ONE = "one" + """ + ) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_enum_file_acceptable_python_two(self): + file = open("./test_files/enum_checker_acceptable.py") + node = astroid.parse(file.read()) + file.close() + + with self.assertNoMessages(): + self.checker.visit_classdef(node.body[3]) + + def test_enum_file_both_violation(self): + file = open("./test_files/enum_checker_violation.py") + node = astroid.parse(file.read()) + file.close() + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="enum-must-inherit-case-insensitive-enum-meta", node=node.body[1] + ), + pylint.testutils.Message( + msg_id="enum-must-be-uppercase", node=node.body[1].body[0].targets[0] + ) + ): + + self.checker.visit_classdef(node.body[1]) + + def test_guidelines_link_active(self): + self._create_url_pipeline("https://azure.github.io/azure-sdk/python_design.html#enumerations") + self._create_url_pipeline("https://azure.github.io/azure-sdk/python_implementation.html#extensible-enumerations") + + + def _create_url_pipeline(self,url): + resp = requests.get(url) + assert resp.status_code == 200 + + + +class TestCheckAPIVersion(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = checker.CheckAPIVersion + + def test_api_version_violation(self): + class_node = astroid.extract_node( + """ + class SomeClient(object): + ''' + :param str something: something + ''' + def __init__(self, something, **kwargs): + pass + """ + ) + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-accepts-api-version-keyword", node=class_node + ) + ): + self.checker.visit_classdef(class_node) + + def test_api_version_acceptable(self): + class_node = astroid.extract_node( + """ + class SomeClient(object): + ''' + :param str something: something + :keyword str api_version: api_version + ''' + def __init__(self, something, **kwargs): + pass + """ + ) + + with self.assertNoMessages(): + self.checker.visit_classdef(class_node) + + def test_api_version_file_class_acceptable(self): + file = open("./test_files/api_version_checker_acceptable_class.py") + node = astroid.parse(file.read()) + file.close() + + with self.assertNoMessages(): + self.checker.visit_classdef(node.body[0]) + + def test_api_version_file_init_acceptable(self): + file = open("./test_files/api_version_checker_acceptable_init.py") + node = astroid.parse(file.read()) + file.close() + + with self.assertNoMessages(): + self.checker.visit_classdef(node.body[0]) + + + def test_api_version_file_violation(self): + file = open("./test_files/api_version_checker_violation.py") + node = astroid.parse(file.read()) + file.close() + + with self.assertAddsMessages( + pylint.testutils.Message( + msg_id="client-accepts-api-version-keyword", node=node.body[0] + ) + ): + self.checker.visit_classdef(node.body[0]) + + def test_guidelines_link_active(self): + url = "https://azure.github.io/azure-sdk/python_design.html#specifying-the-service-version" + config = Configuration() + client = PipelineClient(url, config=config) + request = client.get(url) + response = client._pipeline.run(request) + assert response.http_response.status_code == 200 From 3e31d07b0348c6cd338ee35b51738579b598df46 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 20 Apr 2022 14:56:00 -0700 Subject: [PATCH 4/4] Update pylint-guidelines-plugin tests to properly join test path with targeted files. Update targeted folder in guidelines checker ci.yml. --- .../pylint-guidelines-checker/ci.yml | 11 +++++----- .../tests/test_pylint_custom_plugins.py | 22 ++++++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/tools/pylint-extensions/pylint-guidelines-checker/ci.yml b/tools/pylint-extensions/pylint-guidelines-checker/ci.yml index 94057ce6a35..f8bcb67bfcf 100644 --- a/tools/pylint-extensions/pylint-guidelines-checker/ci.yml +++ b/tools/pylint-extensions/pylint-guidelines-checker/ci.yml @@ -8,7 +8,7 @@ trigger: - hotfix/* paths: include: - - tools/pylint-extensions/pylint_guidelines_checker + - tools/pylint-extensions/pylint-guidelines-checker pr: branches: @@ -19,13 +19,13 @@ pr: - hotfix/* paths: include: - - tools/pylint-extensions/pylint_guidelines_checker + - tools/pylint-extensions/pylint-guidelines-checker extends: template: /eng/pipelines/templates/stages/archetype-sdk-tool-python.yml parameters: PythonVersion: '3.10.2' - PackagePath: 'tools/pylint-extensions/pylint_guidelines_checker' + PackagePath: 'tools/pylint-extensions/pylint-guidelines-checker' FeedName: 'public/azure-sdk-for-python' ArtifactName: 'azsdkpylintplugin' PackageName: 'Azure SDK Pylint Plugin' @@ -34,10 +34,9 @@ extends: pip install -r dev_requirements.txt pip install -e . displayName: 'Install Test Requirements' - workingDirectory: $(Build.SourcesDirectory)/tools/pylint-extensions/pylint_guidelines_checker + workingDirectory: $(Build.SourcesDirectory)/tools/pylint-extensions/pylint-guidelines-checker - script: | pytest . displayName: 'Test Python Linter' - workingDirectory: $(Build.SourcesDirectory)/tools/pylint-extensions/pylint_guidelines_checker - continueOnError: true + workingDirectory: $(Build.SourcesDirectory)/tools/pylint-extensions/pylint-guidelines-checker diff --git a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py index 91ef6c3b1b2..119746b7756 100644 --- a/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py +++ b/tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py @@ -6,10 +6,13 @@ import astroid import pylint.testutils import requests +import os from azure.core import PipelineClient from azure.core.configuration import Configuration -from pylint_custom_plugin import pylint_guidelines_checker as checker +import pylint_guidelines_checker as checker + +TEST_FOLDER = os.path.abspath(os.path.join(__file__, "..")) class TestClientMethodsHaveTracingDecorators(pylint.testutils.CheckerTestCase): CHECKER_CLASS = checker.ClientMethodsHaveTracingDecorators @@ -1734,7 +1737,7 @@ class TestFileHasCopyrightHeader(pylint.testutils.CheckerTestCase): CHECKER_CLASS = checker.FileHasCopyrightHeader def test_copyright_header_acceptable(self): - file = open("./test_files/copyright_header_acceptable.py") + file = open(os.path.join(TEST_FOLDER, "test_files", "copyright_header_acceptable.py")) node = astroid.parse(file.read()) file.close() @@ -1742,7 +1745,7 @@ def test_copyright_header_acceptable(self): self.checker.visit_module(node) def test_copyright_header_violation(self): - file = open("./test_files/copyright_header_violation.py") + file = open(os.path.join(TEST_FOLDER, "test_files", "copyright_header_violation.py")) node = astroid.parse(file.read()) file.close() @@ -2761,8 +2764,7 @@ def test_naming_mismatch_acceptable(self): self.checker.visit_module(module_node) def test_naming_mismatch_pylint_disable(self): - - file = open("./test_files/__init__.py") + file = open(os.path.join(TEST_FOLDER, "test_files", "__init__.py")) node = astroid.parse(file.read()) file.close() @@ -2860,7 +2862,7 @@ class MyGoodEnum(str, Enum, metaclass=CaseInsensitiveEnumMeta): self.checker.visit_classdef(class_node) def test_enum_file_acceptable_python_two(self): - file = open("./test_files/enum_checker_acceptable.py") + file = open(os.path.join(TEST_FOLDER, "test_files", "enum_checker_acceptable.py")) node = astroid.parse(file.read()) file.close() @@ -2868,7 +2870,7 @@ def test_enum_file_acceptable_python_two(self): self.checker.visit_classdef(node.body[3]) def test_enum_file_both_violation(self): - file = open("./test_files/enum_checker_violation.py") + file = open(os.path.join(TEST_FOLDER, "test_files", "enum_checker_violation.py")) node = astroid.parse(file.read()) file.close() @@ -2933,7 +2935,7 @@ def __init__(self, something, **kwargs): self.checker.visit_classdef(class_node) def test_api_version_file_class_acceptable(self): - file = open("./test_files/api_version_checker_acceptable_class.py") + file = open(os.path.join(TEST_FOLDER, "test_files", "api_version_checker_acceptable_class.py")) node = astroid.parse(file.read()) file.close() @@ -2941,7 +2943,7 @@ def test_api_version_file_class_acceptable(self): self.checker.visit_classdef(node.body[0]) def test_api_version_file_init_acceptable(self): - file = open("./test_files/api_version_checker_acceptable_init.py") + file = open(os.path.join(TEST_FOLDER, "test_files", "api_version_checker_acceptable_init.py")) node = astroid.parse(file.read()) file.close() @@ -2950,7 +2952,7 @@ def test_api_version_file_init_acceptable(self): def test_api_version_file_violation(self): - file = open("./test_files/api_version_checker_violation.py") + file = open(os.path.join(TEST_FOLDER, "test_files", "api_version_checker_violation.py")) node = astroid.parse(file.read()) file.close()