Skip to content
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.

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

[pylint] Api checker #23500

Merged
merged 40 commits into from
Mar 24, 2022
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
d1cf546
setting remote
l0lawrence Mar 14, 2022
4331f45
Merge branch 'Azure:main' into api_checker
l0lawrence Mar 14, 2022
527d7e3
setting up api version checker
l0lawrence Mar 14, 2022
401f9e3
api checker
l0lawrence Mar 14, 2022
ad9b4c7
running core to see where error pops up
l0lawrence Mar 14, 2022
af85d72
trying to get tests to run
l0lawrence Mar 14, 2022
e24e7c8
python versioning and kwargs will change this
l0lawrence Mar 16, 2022
fa22fcb
checking docstring for api version keyword
l0lawrence Mar 16, 2022
d012520
added basic test for docstring api_version
l0lawrence Mar 16, 2022
50245cb
adding in a test file for api version checker
l0lawrence Mar 17, 2022
5463960
edit test file
l0lawrence Mar 17, 2022
1be5812
removing import that got added
l0lawrence Mar 17, 2022
6cffbb7
added newline:
l0lawrence Mar 17, 2022
218e3e9
remvoing scripts from pipeline
l0lawrence Mar 17, 2022
526cfda
added newline:
l0lawrence Mar 17, 2022
57825c4
added in check for init doc too
l0lawrence Mar 17, 2022
88fdaed
added in a test for init having the doc versus the init
l0lawrence Mar 17, 2022
618b02f
had to make tests go down to the class level to run test files
l0lawrence Mar 17, 2022
021ed9b
fixing merge conflicts with merging alias checker to feature branch
l0lawrence Mar 17, 2022
4eb225d
fixing naming schema and docs for api checker - renamed error
l0lawrence Mar 18, 2022
23c1c7d
instead of splitting the doc - directly check for the keyword api_ver…
l0lawrence Mar 21, 2022
c9d6883
renaming file to match other test files
l0lawrence Mar 21, 2022
d0b2ec3
switching around the order of keyword and param to see the effect on …
l0lawrence Mar 21, 2022
a806573
renaming changed file name in test
l0lawrence Mar 21, 2022
0dc7cfd
added api checker to the readme
l0lawrence Mar 21, 2022
c7ccf3f
refactoring - if class doc has api_verison, skip looking at the init …
l0lawrence Mar 21, 2022
3a95019
fixing docstring on bad test
l0lawrence Mar 21, 2022
e838d0a
changing credential type in test docstring
l0lawrence Mar 21, 2022
8fa3649
adding in endpoint as an arg in the init
l0lawrence Mar 21, 2022
abe4863
addind endpoint as arg in init
l0lawrence Mar 21, 2022
3987c9c
adding endpoint as arg
l0lawrence Mar 21, 2022
7be1ad6
updating test file names to follow acceptable and violation format
l0lawrence Mar 22, 2022
c2c0609
updating file names in tests, and updating test function names to fol…
l0lawrence Mar 22, 2022
79b7894
fixing eof newline errors
l0lawrence Mar 22, 2022
00982de
trying to get commits into pr
l0lawrence Mar 22, 2022
6dad18d
eof newline
l0lawrence Mar 22, 2022
679f8ca
changing class name to APIVersion instead of api
l0lawrence Mar 22, 2022
f9f052e
init file, also updated APIVersion name
l0lawrence Mar 22, 2022
b23866e
adding end colon to api_verion str checking
l0lawrence Mar 24, 2022
9b9e652
added in end colon to node.doc check too
l0lawrence Mar 24, 2022
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
3 changes: 2 additions & 1 deletion scripts/pylint_custom_plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,5 @@ In the case of a false positive, use the disable command to remove the pylint er
| 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. |
| 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) |
| 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) |
64 changes: 64 additions & 0 deletions scripts/pylint_custom_plugin/pylint_guidelines_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1705,6 +1705,68 @@ def visit_functiondef(self, node):
# this line makes it work for async functions
visit_asyncfunctiondef = visit_functiondef

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:
l0lawrence marked this conversation as resolved.
Show resolved Hide resolved
l0lawrence marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -1786,6 +1848,8 @@ def register(linter):
linter.register_checker(ServiceClientUsesNameWithClientSuffix(linter))
linter.register_checker(CheckDocstringAdmonitionNewline(linter))
linter.register_checker(CheckNamingMismatchGeneratedCode(linter))
linter.register_checker(CheckAPIVersion(linter))


# disabled by default, use pylint --enable=check-docstrings if you want to use it
linter.register_checker(CheckDocstringParameters(linter))
Expand Down
Original file line number Diff line number Diff line change
@@ -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



Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
68 changes: 68 additions & 0 deletions scripts/pylint_custom_plugin/tests/test_pylint_custom_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -2677,3 +2677,71 @@ def test_disable_pylint_alias(self):

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

class TestCheckAPIVersion(pylint.testutils.CheckerTestCase):
CHECKER_CLASS = checker.CheckAPIVersion

def test_api_version_violation(self):
class_node = astroid.extract_node(
"""
class SomeClient(object):
'''
:param str something: something
'''
def __init__(self, something, **kwargs):
pass
"""
)

with self.assertAddsMessages(
pylint.testutils.Message(
msg_id="client-accepts-api-version-keyword", node=class_node
)
):
self.checker.visit_classdef(class_node)

def test_api_version_acceptable(self):
class_node = astroid.extract_node(
"""
class SomeClient(object):
'''
:param str something: something
:keyword str api_version: api_version
'''
def __init__(self, something, **kwargs):
pass
"""
)

with self.assertNoMessages():
self.checker.visit_classdef(class_node)

def test_api_version_file_class_acceptable(self):
file = open("./test_files/api_version_checker_acceptable_class.py")
node = astroid.parse(file.read())
file.close()

with self.assertNoMessages():
self.checker.visit_classdef(node.body[0])

def test_api_version_file_init_acceptable(self):
file = open("./test_files/api_version_checker_acceptable_init.py")
node = astroid.parse(file.read())
file.close()


with self.assertNoMessages():
self.checker.visit_classdef(node.body[0])


def test_api_version_file_violation(self):
file = open("./test_files/api_version_checker_violation.py")
node = astroid.parse(file.read())
file.close()

with self.assertAddsMessages(
pylint.testutils.Message(
msg_id="client-accepts-api-version-keyword", node=node.body[0]
)
):
self.checker.visit_classdef(node.body[0])