Skip to content

Approved prefix #12

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
merged 20 commits into from
Oct 6, 2024
Merged

Approved prefix #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
95fe015
refactor checker and tests
MJoshuaB Aug 28, 2024
e2e664f
fix error with non-builtins decorators
MJoshuaB Aug 28, 2024
b402a5d
add pylint report
MJoshuaB Aug 29, 2024
3d051d7
add ranked listing of reports
MJoshuaB Aug 29, 2024
fdf6db7
format report as table
MJoshuaB Aug 29, 2024
fb160d5
add new verbs
MJoshuaB Sep 1, 2024
f06c915
update report
MJoshuaB Sep 2, 2024
db697b2
update reportcounts.md
MJoshuaB Sep 2, 2024
ae66d2c
fix formatting for reportcounts.md
MJoshuaB Sep 2, 2024
233f536
update reportcounts.md
MJoshuaB Sep 2, 2024
b10a3cd
add TODOs
MJoshuaB Sep 21, 2024
35094ba
exclue private namespaces and classes
MJoshuaB Sep 26, 2024
ed5e211
update reports
MJoshuaB Sep 27, 2024
ae88cee
add new prefixes
MJoshuaB Oct 1, 2024
0bf7340
Merge branch 'working-main' into approved_prefix
MJoshuaB Oct 2, 2024
cfaa891
update unit tests
MJoshuaB Oct 2, 2024
a6f341a
remove reports
MJoshuaB Oct 2, 2024
5947f31
remove commented code
MJoshuaB Oct 2, 2024
65e431f
add checker to README
MJoshuaB Oct 2, 2024
6d262e3
Merge branch 'working-main' into approved_prefix
MJoshuaB Oct 6, 2024
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
7 changes: 4 additions & 3 deletions tools/pylint-extensions/azure-pylint-guidelines-checker/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ In the case of a false positive, use the disable command to remove the pylint er
| no-typing-import-in-type-check | Do not import typing under TYPE_CHECKING. | pylint:disable=no-typing-import-in-type-check | No Link. |
| invalid-use-of-overload | Do not mix async and synchronous overloads | pylint:disable=invalid-use-of-overload | No Link. |
| do-not-log-raised-errors | Do not log errors at `error` or `warning` level when error is raised in an exception block. | pylint:disable=do-not-log-raised-errors | No Link. |
| do-not-use-legacy-typing | Do not use legacy (<Python 3.8) type hinting comments | pylint:disable=do-not-use-legacy-typing | No Link.
| do-not-use-legacy-typing | Do not use legacy (<Python 3.8) type hinting comments | pylint:disable=do-not-use-legacy-typing | No Link.
| do-not-import-asyncio | Do not import asyncio directly. | pylint:disable=do-not-import-asyncio | No Link. |
| TODO | custom linter check for invalid use of @overload #3229 | | |
| TODO | Custom Linter check for Exception Logging #3227 | | |
| TODO | Address Commented out Pylint Custom Plugin Checkers #3228 | | |
| unapproved-client-method-name-prefix | Clients should use preferred verbs for method names | pylint:disable=unapproved-client-method-name-prefix | [link](https://azure.github.io/azure-sdk/python_design.html#naming) |
| do-not-hardcode-connection-verify | Do not hardcode a boolean value to connection_verify | pylint:disable=do-not-hardcode-connection-verify | No LInk. |

| TODO | Address Commented out Pylint Custom Plugin Checkers #3228 | | |
132 changes: 73 additions & 59 deletions tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,79 +161,93 @@ class ClientHasApprovedMethodNamePrefix(BaseChecker):
" https://azure.github.io/azure-sdk/python_design.html#service-operations",
"unapproved-client-method-name-prefix",
"All clients should use the preferred verbs for method names.",
)
}
options = (
(
"ignore-unapproved-client-method-name-prefix",
{
"default": False,
"type": "yn",
"metavar": "<y_or_n>",
"help": "Allow clients to not use preferred method name prefixes",
},
),
)
}

ignore_clients = [
ignore_clients = [
"PipelineClient",
"AsyncPipelineClient",
"ARMPipelineClient",
"AsyncARMPipelineClient",
]

approved_prefixes = [
"get",
"list",
"create",
"upsert",
"set",
"update",
"replace",
"append",
"add",
"delete",
"remove",
"begin",
"upload",
"download", # standard verbs
"close", # very common verb
"cancel",
"clear",
"subscribe",
"send",
"query", # common verbs
"analyze",
"train",
"detect", # future proofing
"from", # special case
]

ignored_decorators = [
"property",
]

def __init__(self, linter=None):
super(ClientHasApprovedMethodNamePrefix, self).__init__(linter)
self.process_class = None
self.namespace = None

def _check_decorators(self, node):
if not node.decorators:
return False
for decorator in node.decorators.nodes:
if isinstance(decorator, astroid.nodes.Name) and decorator.name in self.ignored_decorators:
return True
return False

def visit_module(self, node):
self.namespace = node.name

def visit_classdef(self, node):
"""Visits every class in file and checks if it is a client. If it is a client, checks
that approved method name prefixes are present.
if all((
node.name.endswith("Client"),
node.name not in self.ignore_clients,
not node.name.startswith("_"),
not '._' in self.namespace,
)):
self.process_class = node

:param node: class node
:type node: ast.ClassDef
:return: None
"""
try:
if node.name.endswith("Client") and node.name not in self.ignore_clients:
client_methods = [
child for child in node.get_children() if child.is_function
]

approved_prefixes = [
"get",
"list",
"create",
"upsert",
"set",
"update",
"replace",
"append",
"add",
"delete",
"remove",
"begin",
]
for idx, method in enumerate(client_methods):
if (
method.name.startswith("__")
or "_exists" in method.name
or method.name.startswith("_")
or method.name.startswith("from")
):
continue
prefix = method.name.split("_")[0]
if prefix.lower() not in approved_prefixes:
self.add_message(
msgid="unapproved-client-method-name-prefix",
node=client_methods[idx],
confidence=None,
)
except AttributeError:
logger.debug(
"Pylint custom checker failed to check if client has approved method name prefix."
def visit_functiondef(self, node):
if any((
self.process_class is None, # not in a client class
node.name.startswith("_"), # private method
node.name.endswith("_exists"), # special case
self._check_decorators(node), # property
node.parent != self.process_class, # nested method
)):
return

# check for approved prefix
parts = node.name.split("_")
if parts[0].lower() not in self.approved_prefixes:
self.add_message(
msgid="unapproved-client-method-name-prefix",
node=node,
confidence=None,
)
pass

def leave_classdef(self, node):
self.process_class = None

class ClientMethodsUseKwargsWithMultipleParameters(BaseChecker):
name = "client-method-multiple-parameters"
Expand Down Expand Up @@ -3057,7 +3071,7 @@ def register(linter):

# Rules are disabled until false positive rate improved
# linter.register_checker(CheckForPolicyUse(linter))
# linter.register_checker(ClientHasApprovedMethodNamePrefix(linter))
linter.register_checker(ClientHasApprovedMethodNamePrefix(linter))

# linter.register_checker(ClientDocstringUsesLiteralIncludeForCodeExample(linter))
# linter.register_checker(ClientLROMethodsUseCorePolling(linter))
Expand Down
Loading