Skip to content

Commit deee2dc

Browse files
l0lawrencel0lawrence
authored
[pylint] Added in custom PyLint Checker for Aliasing, with tests (Azure#23383)
* intro patch for aliasing - needs better way to grab __all__ info * updated alias message * line number shows up with alias warning * working on testing this, slight mod to isinstance * tests for aliasing * updated docstring * removing random import * adding in a newline * adding in a newline * changing naming of error message * changing package to model in except * checking for only __all__ assign Node (cat) * added alias checker to README * added test file to test disable pylint warning * add from import test * removed unused imports * changing test names for clarity * added newline * added link from Izzy * fixed some issues with the links * fixed some issues with the links2 * fix for running core on pylint pr * removed cr
1 parent 1894a95 commit deee2dc

File tree

4 files changed

+187
-8
lines changed

4 files changed

+187
-8
lines changed

‎scripts/pylint_custom_plugin/README.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,17 @@ In the case of a false positive, use the disable command to remove the pylint er
4646
| 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) |
4747
| 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) |
4848
| 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) |
49-
| 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) |
50-
| 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) |
51-
| 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) |
49+
| 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) |
50+
| 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) |
51+
| 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) |
5252
| 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) |
5353
| 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) |
5454
| 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) |
5555
| 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) |
56-
| 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) |
57-
| 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) |
56+
| 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) |
57+
| 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) |
5858
| 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) |
5959
| 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) |
6060
| 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) |
61-
| 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. |
61+
| 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. |
62+
| aliasing-generated-code | Do not alias models imported from the generated code. | # pylint:disable=aliasing-generated-code | [link](https://github.com/Azure/autorest/blob/main/docs/generate/built-in-directives.md) |

‎scripts/pylint_custom_plugin/pylint_guidelines_checker.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,6 +1706,68 @@ def visit_functiondef(self, node):
17061706
visit_asyncfunctiondef = visit_functiondef
17071707

17081708

1709+
class CheckNoAliasGeneratedCode(BaseChecker):
1710+
__implements__ = IAstroidChecker
1711+
1712+
name = "check-alias"
1713+
priority = -1
1714+
msgs = {
1715+
"C4745": (
1716+
"Exposing aliased generated code."
1717+
"This messes up sphinx, intellisense, and apiview, so please modify the name of the generated code through"
1718+
" the swagger / directives, or code customizations",
1719+
"aliasing-generated-code",
1720+
"Do not alias models imported from the generated code.",
1721+
),
1722+
}
1723+
options = (
1724+
(
1725+
"ignore-aliasing-generated-code",
1726+
{
1727+
"default": False,
1728+
"type": "yn",
1729+
"metavar": "<y_or_n>",
1730+
"help": "Allow generated code to be aliased.",
1731+
},
1732+
),
1733+
)
1734+
1735+
def __init__(self, linter=None):
1736+
super(CheckNoAliasGeneratedCode, self).__init__(linter)
1737+
1738+
def visit_module(self, node):
1739+
"""Visits __init__.py and checks that there are not aliased models.
1740+
1741+
:param node: module node
1742+
:type node: ast.Module
1743+
:return: None
1744+
"""
1745+
try:
1746+
1747+
if node.file.endswith("__init__.py"):
1748+
aliased = []
1749+
1750+
for nod in node.body:
1751+
if isinstance(nod, astroid.ImportFrom) or isinstance(nod, astroid.Import):
1752+
# If the model has been aliased
1753+
for name in nod.names:
1754+
if name[1] != None:
1755+
aliased.append(name[1])
1756+
1757+
if isinstance(nod, astroid.Assign):
1758+
if nod.targets[0].as_string() == "__all__":
1759+
for models in nod.assigned_stmts():
1760+
for model_name in models.elts:
1761+
if model_name.value in aliased:
1762+
self.add_message(
1763+
msgid="aliasing-generated-code", node=model_name, confidence=None
1764+
)
1765+
1766+
except Exception:
1767+
logger.debug("Pylint custom checker failed to check if model is aliased.")
1768+
pass
1769+
1770+
17091771
# if a linter is registered in this function then it will be checked with pylint
17101772
def register(linter):
17111773
linter.register_checker(ClientsDoNotUseStaticMethods(linter))
@@ -1723,6 +1785,7 @@ def register(linter):
17231785
linter.register_checker(PackageNameDoesNotUseUnderscoreOrPeriod(linter))
17241786
linter.register_checker(ServiceClientUsesNameWithClientSuffix(linter))
17251787
linter.register_checker(CheckDocstringAdmonitionNewline(linter))
1788+
linter.register_checker(CheckNoAliasGeneratedCode(linter))
17261789

17271790
# disabled by default, use pylint --enable=check-docstrings if you want to use it
17281791
linter.register_checker(CheckDocstringParameters(linter))

‎scripts/pylint_custom_plugin/tests/test_files/__init__.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
from something import Something
2+
from something2 import something2 as somethingTwo
3+
4+
__all__ = (
5+
Something,
6+
somethingTwo, #pylint: disable=aliasing-generated-code
7+
)

‎scripts/pylint_custom_plugin/tests/test_pylint_custom_plugins.py

Lines changed: 110 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
from azure.core.configuration import Configuration
1111
from pylint_custom_plugin import pylint_guidelines_checker as checker
1212

13-
1413
class TestClientMethodsHaveTracingDecorators(pylint.testutils.CheckerTestCase):
1514
CHECKER_CLASS = checker.ClientMethodsHaveTracingDecorators
1615

@@ -2568,4 +2567,113 @@ def __init__(self):
25682567
msg_id="docstring-admonition-needs-newline", node=class_node
25692568
)
25702569
):
2571-
self.checker.visit_classdef(class_node)
2570+
self.checker.visit_classdef(class_node)
2571+
2572+
2573+
class TestCheckNoAliasGeneratedCode(pylint.testutils.CheckerTestCase):
2574+
CHECKER_CLASS = checker.CheckNoAliasGeneratedCode
2575+
2576+
def test_ignores_correct_alias_code(self):
2577+
module_node = astroid.extract_node(
2578+
"""
2579+
import something as somethingElse
2580+
"""
2581+
)
2582+
2583+
with self.assertNoMessages():
2584+
self.checker.visit_module(module_node)
2585+
2586+
def test_catches_incorrect_import_alias_code(self):
2587+
import_one = astroid.extract_node(
2588+
'import Something'
2589+
2590+
)
2591+
import_two = astroid.extract_node(
2592+
'import Something2 as SomethingTwo'
2593+
2594+
)
2595+
assign_one = astroid.extract_node(
2596+
"""
2597+
__all__ =(
2598+
"Something",
2599+
"SomethingTwo",
2600+
)
2601+
"""
2602+
)
2603+
2604+
module_node = astroid.Module(name = "node", file="__init__.py", doc = """ """)
2605+
module_node.body = [import_one,import_two,assign_one]
2606+
2607+
for name in module_node.body[-1].assigned_stmts():
2608+
err_node = name.elts[1]
2609+
2610+
with self.assertAddsMessages(
2611+
pylint.testutils.Message(
2612+
msg_id="aliasing-generated-code", node=err_node ,confidence=None
2613+
)
2614+
):
2615+
self.checker.visit_module(module_node)
2616+
2617+
def test_catches_incorrect_from_import_alias_code(self):
2618+
import_one = astroid.extract_node(
2619+
'import Something'
2620+
2621+
)
2622+
import_two = astroid.extract_node(
2623+
'from Something2 import SomethingToo as SomethingTwo'
2624+
2625+
)
2626+
assign_one = astroid.extract_node(
2627+
"""
2628+
__all__ =(
2629+
"Something",
2630+
"SomethingTwo",
2631+
)
2632+
"""
2633+
)
2634+
2635+
module_node = astroid.Module(name = "node", file="__init__.py", doc = """ """)
2636+
module_node.body = [import_one,import_two,assign_one]
2637+
2638+
for name in module_node.body[-1].assigned_stmts():
2639+
err_node = name.elts[1]
2640+
2641+
with self.assertAddsMessages(
2642+
pylint.testutils.Message(
2643+
msg_id="aliasing-generated-code", node=err_node ,confidence=None
2644+
)
2645+
):
2646+
self.checker.visit_module(module_node)
2647+
2648+
def test_ignores_unaliased_import_init(self):
2649+
import_one = astroid.extract_node(
2650+
'import Something'
2651+
2652+
)
2653+
import_two = astroid.extract_node(
2654+
'import Something2 as SomethingTwo'
2655+
2656+
)
2657+
assign_one = astroid.extract_node(
2658+
"""
2659+
__all__ =(
2660+
"Something",
2661+
"Something2",
2662+
)
2663+
"""
2664+
)
2665+
2666+
module_node = astroid.Module(name = "node", file="__init__.py", doc = """ """)
2667+
module_node.body = [import_one,import_two,assign_one]
2668+
2669+
with self.assertNoMessages():
2670+
self.checker.visit_module(module_node)
2671+
2672+
def test_disable_pylint_alias(self):
2673+
2674+
file = open("./test_files/__init__.py")
2675+
node = astroid.parse(file.read())
2676+
file.close()
2677+
2678+
with self.assertNoMessages():
2679+
self.checker.visit_module(node)

0 commit comments

Comments
 (0)