Linter rule to forbid Sequence[str]

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 = []

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<messages>[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)

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]

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

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]

tests/functional/sequence_of_string_3.rc

@@ -0,0 +1,2 @@
+[testoptions]
+min_pyver=3.6

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