Skip to content

Revert "[pylint] Added in custom PyLint Checker for Aliasing, with tests" #23454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kristapratico merged 1 commit into from
Mar 10, 2022
Merged

Revert "[pylint] Added in custom PyLint Checker for Aliasing, with tests" #23454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions scripts/pylint_custom_plugin/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,16 @@ 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. |
| 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) |
| 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. |
63 changes: 0 additions & 63 deletions scripts/pylint_custom_plugin/pylint_guidelines_checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1706,68 +1706,6 @@ 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 @@ -1785,7 +1723,6 @@ 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: 0 additions & 7 deletions scripts/pylint_custom_plugin/tests/test_files/__init__.py
View file
Open in desktop

This file was deleted.

112 changes: 2 additions & 110 deletions scripts/pylint_custom_plugin/tests/test_pylint_custom_plugins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
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 @@ -2567,113 +2568,4 @@ def __init__(self):
msg_id="docstring-admonition-needs-newline", node=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)
self.checker.visit_classdef(class_node)