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

Add methods to skip lint check and fix typehint regex #585

Merged
merged 1 commit into from May 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -31,7 +31,7 @@

INIT_PY_FILE = "__init__.py"

logging.getLogger().setLevel(logging.INFO)
logging.getLogger().setLevel(logging.ERROR)


class StubGenerator:
Expand Down Expand Up @@ -87,7 +87,7 @@ def generate_tokens(self):
pkg_root_path = self.pkg_path
pkg_name, version, namespace = parse_setup_py(pkg_root_path)

logging.debug("package name: {0}, version:{1}".format(pkg_name, version))
logging.debug("package name: {0}, version:{1}, namespace:{2}".format(pkg_name, version, namespace))

logging.debug("Installing package from {}".format(self.pkg_path))
self._install_package(pkg_name)
Expand Down Expand Up @@ -156,8 +156,18 @@ def _generate_tokens(self, pkg_root_path, package_name, version, namespace):
apiview = ApiView(nodeindex, package_name, 0, version, namespace)
modules = self._find_modules(pkg_root_path)
logging.debug("Modules to generate tokens: {}".format(modules))

# find root module name
root_module = ""
if namespace:
root_module = namespace.split(".")[0]

# load all modules and parse them recursively
for m in modules:
if not m.startswith(root_module):
Copy link
Member

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.

Copy link
Member

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 :)

Copy link
Member Author

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.

logging.debug("Skipping module {0}. Module should start with {1}".format(m, root_module))
continue

logging.debug("Importing module {}".format(m))
try:
module_obj = importlib.import_module(m)
Expand Down
@@ -1,4 +1,4 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

VERSION = "0.1.1"
VERSION = "0.1.2"
Expand Up @@ -17,7 +17,7 @@
find_docstring_return_type = "(?<!:):rtype\s?:\s+([^:\n]+)(?!:)"

# Regex to parse type hints
find_type_hint_ret_type = "(?<!#)#\stype:[\s\S]*->\s?([^\n]*)"
find_type_hint_ret_type = "(?<!#)#\s+type:[^\n]*->\s+([^\n]*)"

docstring_types = ["param", "type", "paramtype", "keyword", "rtype"]

Expand Down
Expand Up @@ -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]*[\]\)]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making certain I grok this, you're moving to explicitly include everything up to the newline because otherwise you were just eating comments right?

Copy link
Member Author

Choose a reason for hiding this comment

The 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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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):
Expand Down Expand Up @@ -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:
Expand All @@ -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)

Expand Down