Skip to content

Commit

Permalink
[pylint] Added in custom PyLint Checker for Aliasing, with tests (Azu…
Browse files Browse the repository at this point in the history 
…re#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
  • Loading branch information
@l0lawrence @rakshith91
l0lawrence authored and rakshith91 committed Apr 7, 2022
1 parent 2f5a348 commit 77c62ce
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 8 deletions.
13 changes: 7 additions & 6 deletions scripts/pylint_custom_plugin/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,17 @@ In the case of a false positive, use the disable command to remove the pylint er
| 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-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) |
| 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. |
| 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. |
| 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) |
63 changes: 63 additions & 0 deletions scripts/pylint_custom_plugin/pylint_guidelines_checker.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1706,6 +1706,68 @@ def visit_functiondef(self, node):
visit_asyncfunctiondef = visit_functiondef


class CheckNoAliasGeneratedCode(BaseChecker):
__implements__ = IAstroidChecker

name = "check-alias"
priority = -1
msgs = {
"C4745": (
"Exposing aliased 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",
"aliasing-generated-code",
"Do not alias models imported from the generated code.",
),
}
options = (
(
"ignore-aliasing-generated-code",
{
"default": False,
"type": "yn",
"metavar": "<y_or_n>",
"help": "Allow generated code to be aliased.",
},
),
)

def __init__(self, linter=None):
super(CheckNoAliasGeneratedCode, 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] != None:
aliased.append(name[1])

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="aliasing-generated-code", 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))
Expand All @@ -1723,6 +1785,7 @@ def register(linter):
linter.register_checker(PackageNameDoesNotUseUnderscoreOrPeriod(linter))
linter.register_checker(ServiceClientUsesNameWithClientSuffix(linter))
linter.register_checker(CheckDocstringAdmonitionNewline(linter))
linter.register_checker(CheckNoAliasGeneratedCode(linter))

# disabled by default, use pylint --enable=check-docstrings if you want to use it
linter.register_checker(CheckDocstringParameters(linter))
Expand Down
7 changes: 7 additions & 0 deletions scripts/pylint_custom_plugin/tests/test_files/__init__.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from something import Something
from something2 import something2 as somethingTwo

__all__ = (
Something,
somethingTwo, #pylint: disable=aliasing-generated-code
)
112 changes: 110 additions & 2 deletions scripts/pylint_custom_plugin/tests/test_pylint_custom_plugins.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from azure.core.configuration import Configuration
from pylint_custom_plugin import pylint_guidelines_checker as checker


class TestClientMethodsHaveTracingDecorators(pylint.testutils.CheckerTestCase):
CHECKER_CLASS = checker.ClientMethodsHaveTracingDecorators

Expand Down Expand Up @@ -2568,4 +2567,113 @@ def __init__(self):
msg_id="docstring-admonition-needs-newline", node=class_node
)
):
self.checker.visit_classdef(class_node)
self.checker.visit_classdef(class_node)


class TestCheckNoAliasGeneratedCode(pylint.testutils.CheckerTestCase):
CHECKER_CLASS = checker.CheckNoAliasGeneratedCode

def test_ignores_correct_alias_code(self):
module_node = astroid.extract_node(
"""
import something as somethingElse
"""
)

with self.assertNoMessages():
self.checker.visit_module(module_node)

def test_catches_incorrect_import_alias_code(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="aliasing-generated-code", node=err_node ,confidence=None
)
):
self.checker.visit_module(module_node)

def test_catches_incorrect_from_import_alias_code(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="aliasing-generated-code", node=err_node ,confidence=None
)
):
self.checker.visit_module(module_node)

def test_ignores_unaliased_import_init(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_disable_pylint_alias(self):

file = open("./test_files/__init__.py")
node = astroid.parse(file.read())
file.close()

with self.assertNoMessages():
self.checker.visit_module(node)

0 comments on commit 77c62ce

Please sign in to comment.