[pylint] Feature/pylint plugins merging in new pylint checkers

pylintrc

@@ -4,6 +4,7 @@ reports=no
 
 # PYLINT DIRECTORY BLACKLIST.
 ignore=_vendor,_generated,samples,examples,test,tests,doc,.tox
+ignore-paths=azure\\mixedreality\\remoterendering\\_api_version.py,azure/mixedreality/remoterendering/_api_version.py
 
 init-hook='import sys; import os; sys.path.insert(0, os.path.abspath(os.getcwd().rsplit("azure-sdk-for-python", 1)[0] + "azure-sdk-for-python/scripts/pylint_custom_plugin"))'
 load-plugins=pylint_guidelines_checker

scripts/pylint_custom_plugin/README.md

@@ -58,4 +58,8 @@ In the case of a false positive, use the disable command to remove the pylint er
 | 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#python-client-connection-string) |
 | 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_design.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#service-client)                          |
-| 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.                     |
+| 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)                     |
+| 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)                     |
+| enum-must-be-uppercase                 |  The enum name must be all uppercase.                                                                                                           | # pylint:disable=enum-must-be-uppercase                                      | [link](https://azure.github.io/azure-sdk/python_design.html#enumerations)                     |
+| enum-must-inherit-case-insensitive-enum-meta                 |  The enum should inherit from CaseInsensitiveEnumMeta.                                                                                                           | # pylint:disable=enum-must-inherit-case-insensitive-enum-meta                                      | [link](https://azure.github.io/azure-sdk/python_implementation.html#extensible-enumerations)                     |

scripts/pylint_custom_plugin/pylint_guidelines_checker.py

@@ -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.",
         )
@@ -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": "<y_or_n>",
+                "help": "Allow an enum to not be capitalized.",
+            },
+        ),
+        (
+            "ignore-enum-must-inherit-case-insensitive-enum-meta",
+            {
+                "default": False,
+                "type": "yn",
+                "metavar": "<y_or_n>",
+                "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": "<y_or_n>",
+                "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": "<y_or_n>",
+                "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))
+
+

scripts/pylint_custom_plugin/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
+)

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

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

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

scripts/pylint_custom_plugin/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

scripts/pylint_custom_plugin/tests/test_files/copyright_header_violation.py

@@ -0,0 +1,3 @@
+class Something():
+    def __init__(self):
+        pass

scripts/pylint_custom_plugin/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"

scripts/pylint_custom_plugin/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"