diff --git a/scripts/pylint_custom_plugin/README.md b/scripts/pylint_custom_plugin/README.md index a2df4d7749b6..d3b3192b20b4 100644 --- a/scripts/pylint_custom_plugin/README.md +++ b/scripts/pylint_custom_plugin/README.md @@ -59,4 +59,5 @@ In the case of a false positive, use the disable command to remove the pylint er | 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. | -| naming-mismatch | Do not alias models imported from the generated code. | # pylint:disable=naming-mismatch | [link](https://github.com/Azure/autorest/blob/main/docs/generate/built-in-directives.md) | \ No newline at end of file +| naming-mismatch | Do not alias models imported from the generated code. | # pylint:disable=naming-mismatch | [link](https://github.com/Azure/autorest/blob/main/docs/generate/built-in-directives.md) | +| client-accepts-api-version-keyword | Ensure that the client constructor accepts a keyword-only api_version argument. | # pylint:disable=client-accepts-api-version-keyword | [link](https://azure.github.io/azure-sdk/python_design.html#specifying-the-service-version) | \ No newline at end of file diff --git a/scripts/pylint_custom_plugin/pylint_guidelines_checker.py b/scripts/pylint_custom_plugin/pylint_guidelines_checker.py index 8498fc8cbc9a..4c16793e0ca6 100644 --- a/scripts/pylint_custom_plugin/pylint_guidelines_checker.py +++ b/scripts/pylint_custom_plugin/pylint_guidelines_checker.py @@ -1705,6 +1705,68 @@ def visit_functiondef(self, node): # this line makes it work for async functions visit_asyncfunctiondef = visit_functiondef +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 @@ -1786,6 +1848,8 @@ def register(linter): linter.register_checker(ServiceClientUsesNameWithClientSuffix(linter)) linter.register_checker(CheckDocstringAdmonitionNewline(linter)) linter.register_checker(CheckNamingMismatchGeneratedCode(linter)) + linter.register_checker(CheckAPIVersion(linter)) + # disabled by default, use pylint --enable=check-docstrings if you want to use it linter.register_checker(CheckDocstringParameters(linter)) diff --git a/scripts/pylint_custom_plugin/tests/test_files/api_version_checker_acceptable_class.py b/scripts/pylint_custom_plugin/tests/test_files/api_version_checker_acceptable_class.py new file mode 100644 index 000000000000..930f4a797856 --- /dev/null +++ b/scripts/pylint_custom_plugin/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/scripts/pylint_custom_plugin/tests/test_files/api_version_checker_acceptable_init.py b/scripts/pylint_custom_plugin/tests/test_files/api_version_checker_acceptable_init.py new file mode 100644 index 000000000000..6cddc4dfa810 --- /dev/null +++ b/scripts/pylint_custom_plugin/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/scripts/pylint_custom_plugin/tests/test_files/api_version_checker_violation.py b/scripts/pylint_custom_plugin/tests/test_files/api_version_checker_violation.py new file mode 100644 index 000000000000..d327d3fed542 --- /dev/null +++ b/scripts/pylint_custom_plugin/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/scripts/pylint_custom_plugin/tests/test_pylint_custom_plugins.py b/scripts/pylint_custom_plugin/tests/test_pylint_custom_plugins.py index 8d4070d16ba3..c779b9c375a7 100644 --- a/scripts/pylint_custom_plugin/tests/test_pylint_custom_plugins.py +++ b/scripts/pylint_custom_plugin/tests/test_pylint_custom_plugins.py @@ -2677,3 +2677,71 @@ def test_disable_pylint_alias(self): with self.assertNoMessages(): self.checker.visit_module(node) + +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])