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
Add methods to skip lint check and fix typehint regex #585
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
# Copyright (c) Microsoft Corporation. All rights reserved. | ||
# Licensed under the MIT License. | ||
|
||
VERSION = "0.1.1" | ||
VERSION = "0.1.2" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,9 +12,20 @@ | |
KW_ARG_NAME = "**kwargs" | ||
VALIDATION_REQUIRED_DUNDER = ["__init__",] | ||
KWARG_NOT_REQUIRED_METHODS = ["close",] | ||
TYPEHINT_NOT_REQUIRED_METHODS = ["close",] | ||
REGEX_ITEM_PAGED = "~[\w.]*\.([\w]*)\s?[\[\(][\w]*[\]\)]" | ||
TYPEHINT_NOT_REQUIRED_METHODS = ["close", "__init__"] | ||
REGEX_ITEM_PAGED = "~[\w.]*\.([\w]*)\s?[\[\(][^\n]*[\]\)]" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making certain I grok this, you're moving to explicitly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. REGEX for item paged was finding only part of return type if type is like ~azure..ItemPaged[someType[SomeSubType]]. Previous REGEX stops scan as soon as it find closing bracket. |
||
PAGED_TYPES = ["ItemPaged", "AsyncItemPaged",] | ||
# Methods that are implementation of known interface should be excluded from lint check | ||
# for e.g. get, update, keys | ||
LINT_EXCLUSION_METHODS = [ | ||
"get", | ||
"has_key", | ||
"items", | ||
"keys", | ||
"update", | ||
"values", | ||
"close", | ||
] | ||
|
||
|
||
def is_kwarg_mandatory(func_name): | ||
|
@@ -75,7 +86,7 @@ def _inspect(self): | |
error_message = "Error in parsing decorators for function {}".format( | ||
self.name | ||
) | ||
self.errors.append(error_message) | ||
self.add_error(error_message) | ||
logging.error(error_message) | ||
|
||
self.is_class_method = "@classmethod" in self.annotations | ||
|
@@ -125,7 +136,7 @@ def _parse_function(self): | |
self._copy_kw_args() | ||
|
||
if not self.return_type and is_typehint_mandatory(self.name): | ||
self.errors.append("Return type is missing in both typehint and docstring") | ||
self.add_error("Return type is missing in both typehint and docstring") | ||
# Validate return type | ||
self._validate_pageable_api() | ||
|
||
|
@@ -194,7 +205,8 @@ def _parse_docstring(self): | |
def _parse_typehint(self): | ||
|
||
# Skip parsing typehint if typehint is not expected for e.g dunder or async methods | ||
if not is_typehint_mandatory(self.name) or self.is_async: | ||
# and if return type is already found | ||
if self.return_type and not is_typehint_mandatory(self.name) or self.is_async: | ||
return | ||
|
||
# Parse type hint to get return type and types for positional args | ||
|
@@ -203,7 +215,7 @@ def _parse_typehint(self): | |
type_hint_ret_type = typehint_parser.find_return_type() | ||
# Type hint must be present for all APIs. Flag it as an error if typehint is missing | ||
if not type_hint_ret_type: | ||
self.errors.append("Typehint is missing for method {}".format(self.name)) | ||
self.add_error("Typehint is missing for method {}".format(self.name)) | ||
return | ||
|
||
if not self.return_type: | ||
|
@@ -214,7 +226,7 @@ def _parse_typehint(self): | |
long_ret_type = self.return_type | ||
if long_ret_type != type_hint_ret_type and short_return_type != type_hint_ret_type: | ||
error_message = "Return type in type hint is not matching return type in docstring" | ||
self.errors.append(error_message) | ||
self.add_error(error_message) | ||
|
||
|
||
def _generate_signature_token(self, apiview): | ||
|
@@ -287,6 +299,10 @@ def generate_tokens(self, apiview): | |
|
||
|
||
def add_error(self, error_msg): | ||
# Ignore errors for lint check excluded methods | ||
if self.name in LINT_EXCLUSION_METHODS: | ||
return | ||
|
||
# Hide all diagnostics for now for dunder methods | ||
# These are well known protocol implementation | ||
if not self.name.startswith("_") or self.name in VALIDATION_REQUIRED_DUNDER: | ||
|
@@ -303,7 +319,7 @@ def _validate_pageable_api(self): | |
if ret_short_type in PAGED_TYPES: | ||
logging.debug("list API returns valid paged return type") | ||
return | ||
error_msg = "list API {0} should return ItemPaged or AsyncItemPaged".format(self.name) | ||
error_msg = "list API {0} should return ItemPaged or AsyncItemPaged instead of {1}".format(self.name, self.return_type) | ||
logging.error(error_msg) | ||
self.add_error(error_msg) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give me an example of this running for something we DONT own? Just for my own reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question - I'm also curious :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not fully generalized. I have tried for few external packages and it didn't work. This fix is to mainly avoid other folders in package root other than "Azure". I will debug what's wrong in generating stub for external package.