diff --git a/shopify_python/git_utils.py b/shopify_python/git_utils.py index 268039f..e7e24ed 100644 --- a/shopify_python/git_utils.py +++ b/shopify_python/git_utils.py @@ -22,7 +22,7 @@ def _remote_origin_master(git_repo): def _modified_in_branch(git_repo, other_ref): - # type: (repo.Repo, head.Head) -> typing.Sequence[str] + # type: (repo.Repo, head.Head) -> typing.List[str] common_commit = git_repo.merge_base(git_repo.active_branch, other_ref)[0] diffs = common_commit.diff(git_repo.active_branch.commit) changed_files = [] diff --git a/shopify_python/shopify_styleguide.py b/shopify_python/shopify_styleguide.py index f3937e9..40b2952 100644 --- a/shopify_python/shopify_styleguide.py +++ b/shopify_python/shopify_styleguide.py @@ -27,27 +27,57 @@ class ShopifyStyleGuideChecker(checkers.BaseTokenChecker): 'disable-name-only', "Disable pylint rules via message name (e.g. unused-import) and not message code (e.g. W0611) to " "help code reviewers understand why a linter rule was disabled for a line of code."), + 'C6102': ('Forbidden use of typing.Sequence[str], use typing.List[str] or some specific collection instead', + 'sequence-of-string', + 'Since str itself also satisfies typing.Sequence[str], the latter should be replaced by ' + 'a more specific iterable type, such as typing.List[str]') } RE_PYLINT_DISABLE = re.compile(r'^#[ \t]*pylint:[ \t]*(disable|enable)[ \t]*=(?P[a-zA-Z0-9\-_, \t]+)$') RE_PYLINT_MESSAGE_CODE = re.compile(r'^[A-Z]{1,2}[0-9]{4}$') + RE_COMMENT_TYPE_ANNOTATION = re.compile(r'^# type.*:.*$') + RE_SEQUENCE_STRING = re.compile(r'^.*Sequence\[str\].*$') + def process_tokens(self, tokens): # type: (typing.Sequence[typing.Tuple]) -> None - for _type, string, start, _, _ in tokens: - start_row, _ = start - if _type == tokenize.COMMENT: - - def get_name(code): - try: - return self.linter.msgs_store.get_msg_display_string(code) - except pylint.utils.UnknownMessageError: - return 'unknown' - - matches = self.RE_PYLINT_DISABLE.match(string) - if matches: - for msg in matches.group('messages').split(','): - msg = msg.strip() - if self.RE_PYLINT_MESSAGE_CODE.match(msg): - self.add_message('disable-name-only', line=start_row, - args={'code': msg, 'name': get_name(msg)}) + for _type, string, start, _, line in tokens: + if _type == tokenize.NAME: + self.__validate_name(string, start, line) + elif _type == tokenize.COMMENT: + self.__validate_comment(string, start) + + def __validate_comment(self, string, start): + # type: (str, typing.Tuple[int, int]) -> None + self.__disable_name_only(string, start) + if self.RE_COMMENT_TYPE_ANNOTATION.match(string): + self.__sequence_str(string, start) + + def __validate_name(self, string, start, line): + # type: (str, typing.Tuple[int, int], str) -> None + if string == 'Sequence' and 'Sequence[str]' in line: + self.__sequence_str(line, start) + + def __disable_name_only(self, string, start): + # type: (str, typing.Tuple[int, int]) -> None + start_row, _ = start + + def get_name(code): + try: + return self.linter.msgs_store.get_msg_display_string(code) + except pylint.utils.UnknownMessageError: + return 'unknown' + + matches = self.RE_PYLINT_DISABLE.match(string) + if matches: + for msg in matches.group('messages').split(','): + msg = msg.strip() + if self.RE_PYLINT_MESSAGE_CODE.match(msg): + self.add_message('disable-name-only', line=start_row, + args={'code': msg, 'name': get_name(msg)}) + + def __sequence_str(self, string, start): + # type: (str, typing.Tuple[int, int]) -> None + start_row, _ = start + if self.RE_SEQUENCE_STRING.match(string): + self.add_message('sequence-of-string', line=start_row) diff --git a/tests/functional/sequence_of_string_2.py b/tests/functional/sequence_of_string_2.py new file mode 100644 index 0000000..2fec379 --- /dev/null +++ b/tests/functional/sequence_of_string_2.py @@ -0,0 +1,18 @@ +# pylint:disable=missing-docstring,unused-import +import typing + + +def one_param(seq): + # type: (typing.Sequence[str]) -> str # [sequence-of-string] + copy = seq # type:typing.Sequence[str] # [sequence-of-string] + return copy[0] + + +def multiple_params(message, seq, index): + # type: (string, typing.Sequence[str], int) -> str # [sequence-of-string] + return message + seq[index] + + +def return_type(message_1, message_2): + # type: (str, str) -> typing.Sequence[str] # [sequence-of-string] + return [message_1, message_2] diff --git a/tests/functional/sequence_of_string_2.txt b/tests/functional/sequence_of_string_2.txt new file mode 100644 index 0000000..59455b9 --- /dev/null +++ b/tests/functional/sequence_of_string_2.txt @@ -0,0 +1,4 @@ +sequence-of-string:6::Forbidden use of typing.Sequence[str], use typing.List[str] or some specific collection instead +sequence-of-string:7::Forbidden use of typing.Sequence[str], use typing.List[str] or some specific collection instead +sequence-of-string:12::Forbidden use of typing.Sequence[str], use typing.List[str] or some specific collection instead +sequence-of-string:17::Forbidden use of typing.Sequence[str], use typing.List[str] or some specific collection instead \ No newline at end of file diff --git a/tests/functional/sequence_of_string_3.py b/tests/functional/sequence_of_string_3.py new file mode 100644 index 0000000..b4772c1 --- /dev/null +++ b/tests/functional/sequence_of_string_3.py @@ -0,0 +1,18 @@ +# pylint:disable=missing-docstring,unused-import +import typing + + +def one_param(seq: typing.Sequence[str]) -> str: # [sequence-of-string] + copy: typing.Sequence[str] = seq # [sequence-of-string] + return copy[0] + + +def multiple_params( + message: str, + seq: typing.Sequence[str], # [sequence-of-string] + index: int) -> str: + return message + seq[index] + + +def return_type(message_1: str, message_2: str) -> typing.Sequence[str]: # [sequence-of-string] + return [message_1, message_2] diff --git a/tests/functional/sequence_of_string_3.rc b/tests/functional/sequence_of_string_3.rc new file mode 100644 index 0000000..0ba2b63 --- /dev/null +++ b/tests/functional/sequence_of_string_3.rc @@ -0,0 +1,2 @@ +[testoptions] +min_pyver=3.6 diff --git a/tests/functional/sequence_of_string_3.txt b/tests/functional/sequence_of_string_3.txt new file mode 100644 index 0000000..d11504a --- /dev/null +++ b/tests/functional/sequence_of_string_3.txt @@ -0,0 +1,4 @@ +sequence-of-string:5::Forbidden use of typing.Sequence[str], use typing.List[str] or some specific collection instead +sequence-of-string:6::Forbidden use of typing.Sequence[str], use typing.List[str] or some specific collection instead +sequence-of-string:12::Forbidden use of typing.Sequence[str], use typing.List[str] or some specific collection instead +sequence-of-string:17::Forbidden use of typing.Sequence[str], use typing.List[str] or some specific collection instead \ No newline at end of file