Feature/import warnings

tools/pylint-extensions/pylint-guidelines-checker/README.md

@@ -73,4 +73,5 @@ In the case of a false positive, use the disable command to remove the pylint er
 | 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)                     |
+| 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)                     |
+| networking-import-outside-azure-core-transport                 | This import is only allowed in azure.core.pipeline.transport.                                                                                                          | # pylint:disable=networking-import-outside-azure-core-transport                                     | [link](https://github.com/Azure/azure-sdk-for-python/issues/24989)                     |

tools/pylint-extensions/pylint-guidelines-checker/pylint_guidelines_checker.py

@@ -4,7 +4,7 @@
 # ------------------------------------
 
 """
-Pylint custom checkers for SDK guidelines: C4717 - C4748
+Pylint custom checkers for SDK guidelines: C4717 - C4749
 """
 
 import logging
@@ -1916,7 +1916,45 @@ def visit_module(self, node):
 
         except Exception:
                 logger.debug("Pylint custom checker failed to check if model is aliased.")
-                pass
+
+class NonCoreNetworkImport(BaseChecker):
+    """There are certain imports that should only occur in the core package.
+    For example, instead of using `requests` to make requests, clients should
+    take a `azure.core.pipeline.Pipeline` as input to make requests.
+    """
+    name = "networking-import-outside-azure-core-transport"
+    priority = -1
+    msgs = {
+        "C4749": (
+            "This import is not allowed here. Consider using an abstract"
+            " alternative from azure.core.pipeline.transport.",
+            "networking-import-outside-azure-core-transport",
+            "This import is only allowed in azure.core.pipeline.transport.",
+        ),
+    }
+    BLOCKED_MODULES = ["aiohttp", "requests", "trio"]
+    AZURE_CORE_TRANSPORT_NAME = "azure.core.pipeline.transport"
+
+    def visit_import(self, node):
+        """Check that we dont have blocked imports."""
+        if node.root().name.startswith(self.AZURE_CORE_TRANSPORT_NAME):
+            return
+        for import_, _ in node.names:
+            self._check_import(import_, node)
+
+    def visit_importfrom(self, node):
+        """Check that we aren't import from a blocked package."""
+        if node.root().name.startswith(self.AZURE_CORE_TRANSPORT_NAME): 
+            return
+        self._check_import(node.modname, node)
+
+    def _check_import(self, name, node):
+        """Check if an import is blocked."""
+        for blocked in self.BLOCKED_MODULES:
+            if name.startswith(blocked):
+                self.add_message(
+                    msgid=f"networking-import-outside-azure-core-transport", node=node, confidence=None
+                )
 
 
 # if a linter is registered in this function then it will be checked with pylint
@@ -1939,6 +1977,7 @@ def register(linter):
     linter.register_checker(CheckNamingMismatchGeneratedCode(linter))
     linter.register_checker(CheckAPIVersion(linter))
     linter.register_checker(CheckEnum(linter))
+    linter.register_checker(NonCoreNetworkImport(linter))
     linter.register_checker(ClientListMethodsUseCorePaging(linter))
 
 
@@ -1953,5 +1992,3 @@ def register(linter):
     # linter.register_checker(ClientDocstringUsesLiteralIncludeForCodeExample(linter))
     # linter.register_checker(ClientLROMethodsUseCorePolling(linter))
     # linter.register_checker(ClientLROMethodsUseCorrectNaming(linter))
-
-

tools/pylint-extensions/pylint-guidelines-checker/tests/test_pylint_custom_plugins.py

@@ -3017,3 +3017,59 @@ def test_guidelines_link_active(self):
         request = client.get(url)
         response = client._pipeline.run(request)
         assert response.http_response.status_code == 200
+
+
+class TestCheckNonCoreNetworkImport(pylint.testutils.CheckerTestCase):
+    """Test that we are blocking disallowed imports and allowing allowed imports."""
+    CHECKER_CLASS = checker.NonCoreNetworkImport
+
+    def test_disallowed_imports(self):
+        """Check that illegal imports raise warnings"""
+        # Blocked import ouside of core.
+        import_node = astroid.extract_node("import requests")
+        with self.assertAddsMessages(
+                pylint.testutils.MessageTest(
+                    msg_id="networking-import-outside-azure-core-transport",
+                    line=1,
+                    node=import_node,
+                    col_offset=0,
+                )
+        ):
+            self.checker.visit_import(import_node)
+
+        # blocked import from outside of core.
+        importfrom_node = astroid.extract_node("from aiohttp import get")
+        with self.assertAddsMessages(
+                pylint.testutils.MessageTest(
+                    msg_id="networking-import-outside-azure-core-transport",
+                    line=1,
+                    node=importfrom_node,
+                    col_offset=0,
+                )
+        ):
+            self.checker.visit_importfrom(importfrom_node)
+
+
+    def test_allowed_imports(self):
+        """Check that allowed imports don't raise warnings."""
+        # import not in the blocked list.
+        import_node = astroid.extract_node("import math")
+        with self.assertNoMessages():
+            self.checker.visit_import(import_node)
+
+        # from import not in the blocked list.
+        importfrom_node = astroid.extract_node("from azure.core.pipeline import Pipeline")
+        with self.assertNoMessages():
+            self.checker.visit_importfrom(importfrom_node)
+
+        # blocked import, but in core.
+        import_node = astroid.extract_node("import requests")
+        import_node.root().name = "azure.core.pipeline.transport"
+        with self.assertNoMessages():
+            self.checker.visit_import(import_node)
+
+        # blocked from import, but in core.
+        importfrom_node = astroid.extract_node("from requests.exceptions import HttpException")
+        importfrom_node.root().name = "azure.core.pipeline.transport._private_module"
+        with self.assertNoMessages():
+            self.checker.visit_importfrom(importfrom_node)