Skip to content

Commit 18ea2fa

Browse files
MJoshuaBMJoshuaB
authored
Merge pull request #12 from MJoshuaB/approved_prefix
Address merge conflicts and finalize merge.
2 parents 8cdce20 + 6d262e3 commit 18ea2fa

4 files changed

Lines changed: 312 additions & 315 deletions

File tree

‎tools/pylint-extensions/azure-pylint-guidelines-checker/README.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,10 @@ In the case of a false positive, use the disable command to remove the pylint er
9595
| no-typing-import-in-type-check | Do not import typing under TYPE_CHECKING. | pylint:disable=no-typing-import-in-type-check | No Link. |
9696
| invalid-use-of-overload | Do not mix async and synchronous overloads | pylint:disable=invalid-use-of-overload | No Link. |
9797
| 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. |
98-
| do-not-use-legacy-typing | Do not use legacy (<Python 3.8) type hinting comments | pylint:disable=do-not-use-legacy-typing | No Link.
98+
| do-not-use-legacy-typing | Do not use legacy (<Python 3.8) type hinting comments | pylint:disable=do-not-use-legacy-typing | No Link.
9999
| do-not-import-asyncio | Do not import asyncio directly. | pylint:disable=do-not-import-asyncio | No Link. |
100+
| TODO | custom linter check for invalid use of @overload #3229 | | |
100101
| TODO | Custom Linter check for Exception Logging #3227 | | |
101-
| TODO | Address Commented out Pylint Custom Plugin Checkers #3228 | | |
102+
| 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) |
102103
| do-not-hardcode-connection-verify | Do not hardcode a boolean value to connection_verify | pylint:disable=do-not-hardcode-connection-verify | No LInk. |
103-
104+
| TODO | Address Commented out Pylint Custom Plugin Checkers #3228 | | |

‎tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py‎

Lines changed: 73 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -161,79 +161,93 @@ class ClientHasApprovedMethodNamePrefix(BaseChecker):
161161
" https://azure.github.io/azure-sdk/python_design.html#service-operations",
162162
"unapproved-client-method-name-prefix",
163163
"All clients should use the preferred verbs for method names.",
164-
)
165-
}
166-
options = (
167-
(
168-
"ignore-unapproved-client-method-name-prefix",
169-
{
170-
"default": False,
171-
"type": "yn",
172-
"metavar": "<y_or_n>",
173-
"help": "Allow clients to not use preferred method name prefixes",
174-
},
175164
),
176-
)
165+
}
177166

178-
ignore_clients = [
167+
ignore_clients = [
179168
"PipelineClient",
180169
"AsyncPipelineClient",
181170
"ARMPipelineClient",
182171
"AsyncARMPipelineClient",
183172
]
184173

174+
approved_prefixes = [
175+
"get",
176+
"list",
177+
"create",
178+
"upsert",
179+
"set",
180+
"update",
181+
"replace",
182+
"append",
183+
"add",
184+
"delete",
185+
"remove",
186+
"begin",
187+
"upload",
188+
"download", # standard verbs
189+
"close", # very common verb
190+
"cancel",
191+
"clear",
192+
"subscribe",
193+
"send",
194+
"query", # common verbs
195+
"analyze",
196+
"train",
197+
"detect", # future proofing
198+
"from", # special case
199+
]
200+
201+
ignored_decorators = [
202+
"property",
203+
]
204+
185205
def __init__(self, linter=None):
186206
super(ClientHasApprovedMethodNamePrefix, self).__init__(linter)
207+
self.process_class = None
208+
self.namespace = None
209+
210+
def _check_decorators(self, node):
211+
if not node.decorators:
212+
return False
213+
for decorator in node.decorators.nodes:
214+
if isinstance(decorator, astroid.nodes.Name) and decorator.name in self.ignored_decorators:
215+
return True
216+
return False
217+
218+
def visit_module(self, node):
219+
self.namespace = node.name
187220

188221
def visit_classdef(self, node):
189-
"""Visits every class in file and checks if it is a client. If it is a client, checks
190-
that approved method name prefixes are present.
222+
if all((
223+
node.name.endswith("Client"),
224+
node.name not in self.ignore_clients,
225+
not node.name.startswith("_"),
226+
not '._' in self.namespace,
227+
)):
228+
self.process_class = node
191229

192-
:param node: class node
193-
:type node: ast.ClassDef
194-
:return: None
195-
"""
196-
try:
197-
if node.name.endswith("Client") and node.name not in self.ignore_clients:
198-
client_methods = [
199-
child for child in node.get_children() if child.is_function
200-
]
201-
202-
approved_prefixes = [
203-
"get",
204-
"list",
205-
"create",
206-
"upsert",
207-
"set",
208-
"update",
209-
"replace",
210-
"append",
211-
"add",
212-
"delete",
213-
"remove",
214-
"begin",
215-
]
216-
for idx, method in enumerate(client_methods):
217-
if (
218-
method.name.startswith("__")
219-
or "_exists" in method.name
220-
or method.name.startswith("_")
221-
or method.name.startswith("from")
222-
):
223-
continue
224-
prefix = method.name.split("_")[0]
225-
if prefix.lower() not in approved_prefixes:
226-
self.add_message(
227-
msgid="unapproved-client-method-name-prefix",
228-
node=client_methods[idx],
229-
confidence=None,
230-
)
231-
except AttributeError:
232-
logger.debug(
233-
"Pylint custom checker failed to check if client has approved method name prefix."
230+
def visit_functiondef(self, node):
231+
if any((
232+
self.process_class is None, # not in a client class
233+
node.name.startswith("_"), # private method
234+
node.name.endswith("_exists"), # special case
235+
self._check_decorators(node), # property
236+
node.parent != self.process_class, # nested method
237+
)):
238+
return
239+
240+
# check for approved prefix
241+
parts = node.name.split("_")
242+
if parts[0].lower() not in self.approved_prefixes:
243+
self.add_message(
244+
msgid="unapproved-client-method-name-prefix",
245+
node=node,
246+
confidence=None,
234247
)
235-
pass
236248

249+
def leave_classdef(self, node):
250+
self.process_class = None
237251

238252
class ClientMethodsUseKwargsWithMultipleParameters(BaseChecker):
239253
name = "client-method-multiple-parameters"
@@ -3057,7 +3071,7 @@ def register(linter):
30573071

30583072
# Rules are disabled until false positive rate improved
30593073
# linter.register_checker(CheckForPolicyUse(linter))
3060-
# linter.register_checker(ClientHasApprovedMethodNamePrefix(linter))
3074+
linter.register_checker(ClientHasApprovedMethodNamePrefix(linter))
30613075

30623076
# linter.register_checker(ClientDocstringUsesLiteralIncludeForCodeExample(linter))
30633077
# linter.register_checker(ClientLROMethodsUseCorePolling(linter))

0 commit comments

Comments
 (0)
⚡