add a lint to warn on implicit string concatenation (pantsbuild#7286)

### Problem

It's occasionally possible to make mistakes when formatting strings using the "implicit" syntax of just placing two string literals next to each other. Although this pattern is used extremely commonly in the codebase right now, it would be nice to be able to point this out.

### Solution

- Add a linter for string concatenations of the form e.g. `'a' 'b'`.
- Add a suppressions file which turns it off for all python files in this repo.

### Result

It's possible this could lead to fewer errors. However, there is currently a lot of noise induced by turning this on and it would be nice to avoid that by eventually fixing code to remove it (e.g. by using triple-quoted `"""` strings instead), but for now, it has been suppressed. Other repos can freely use this lint, however.

build-support/checkstyle/python_suppressions.txt

@@ -0,0 +1 @@
+.*\.py::implicit-string-concatenation

contrib/python/src/python/pants/contrib/python/checks/checker/BUILD

@@ -10,6 +10,7 @@ python_library(
     }
   ),
   dependencies=[
+    '3rdparty/python:asttokens',
     '3rdparty/python:future',
     '3rdparty/python:pycodestyle',
     '3rdparty/python:pyflakes',

contrib/python/src/python/pants/contrib/python/checks/checker/checker.py

@@ -17,6 +17,8 @@
 from pants.contrib.python.checks.checker.except_statements import ExceptStatements
 from pants.contrib.python.checks.checker.file_excluder import FileExcluder
 from pants.contrib.python.checks.checker.future_compatibility import FutureCompatibility
+from pants.contrib.python.checks.checker.implicit_string_concatenation import \
+  ImplicitStringConcatenation
 from pants.contrib.python.checks.checker.import_order import ImportOrder
 from pants.contrib.python.checks.checker.indentation import Indentation
 from pants.contrib.python.checks.checker.missing_contextmanager import MissingContextManager
@@ -132,6 +134,7 @@ def plugins():
     ConstantLogic,
     ExceptStatements,
     FutureCompatibility,
+    ImplicitStringConcatenation,
     ImportOrder,
     Indentation,
     MissingContextManager,

contrib/python/src/python/pants/contrib/python/checks/checker/common.py

@@ -12,6 +12,7 @@
 import textwrap
 import tokenize
 
+import asttokens
 import six
 
 
@@ -105,6 +106,7 @@ def _remove_coding_header(cls, blob):
   def __init__(self, blob, tree, root, filename):
     self._blob = self._remove_coding_header(blob)
     self.tree = tree
+    self.tokenized_file_body = asttokens.ASTTokens(self._blob, tree=self.tree, filename=filename)
     self.lines = OffByOneList(self._blob.decode('utf-8').split('\n'))
     self._root = root
     self.filename = filename

contrib/python/src/python/pants/contrib/python/checks/checker/implicit_string_concatenation.py

@@ -0,0 +1,56 @@
+# coding=utf-8
+# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+import ast
+import token
+import tokenize
+from io import StringIO
+
+from future.utils import PY3
+
+from pants.contrib.python.checks.checker.common import CheckstylePlugin
+
+
+class ImplicitStringConcatenation(CheckstylePlugin):
+  """Detect instances of implicit string concatenation without a plus sign."""
+
+  @classmethod
+  def name(cls):
+    return 'implicit-string-concatenation'
+
+  @classmethod
+  def iter_strings(cls, tree):
+    for ast_node in ast.walk(tree):
+      if isinstance(ast_node, ast.Str):
+        yield ast_node
+
+  @classmethod
+  def string_node_token_names(cls, str_node_text):
+    for tok in tokenize.generate_tokens(StringIO(str_node_text).readline):
+      token_type = tok.type if PY3 else tok[0]
+      yield token.tok_name[token_type]
+
+  @classmethod
+  def has_multiple_strings(cls, token_names):
+    return sum(1 if name == 'STRING' else 0 for name in token_names) > 1
+
+  def uses_implicit_concatenation(self, str_node_text):
+    str_node_token_names = list(self.string_node_token_names(str_node_text))
+    return self.has_multiple_strings(str_node_token_names)
+
+  def nits(self):
+    for str_node in self.iter_strings(self.python_file.tree):
+      str_node_text = self.python_file.tokenized_file_body.get_text(str_node)
+      if self.uses_implicit_concatenation(str_node_text):
+        yield self.warning(
+          'T806',
+          """\
+Implicit string concatenation by separating string literals with a space was detected. Using an
+explicit `+` operator can lead to less error-prone code.""",
+          str_node)
+      # TODO: also consider checking when triple-quoted strings are used -- e.g. '''a''''' becomes
+      # just "a" (from implicit concatenation, which we catch here), but '''''a''' turns into "''a",
+      # without any implicit concatenation.

contrib/python/tests/python/pants_test/contrib/python/checks/checker/test_implicit_string_concatenation.py

@@ -0,0 +1,28 @@
+# coding=utf-8
+# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+from pants_test.contrib.python.checks.checker.plugin_test_base import CheckstylePluginTestBase
+
+from pants.contrib.python.checks.checker.common import Nit
+from pants.contrib.python.checks.checker.implicit_string_concatenation import \
+  ImplicitStringConcatenation
+
+
+class ImplicitStringConcatenationTest(CheckstylePluginTestBase):
+  plugin_type = ImplicitStringConcatenation
+
+  def test_implicit_string_concatenation(self):
+    self.assertNit("'a' 'b'", 'T806', Nit.WARNING)
+    self.assertNit('"a" "b"', 'T806', Nit.WARNING)
+    self.assertNit("'a' \"b\"", 'T806', Nit.WARNING)
+    self.assertNit("('a'\n'b')", 'T806', Nit.WARNING)
+    self.assertNit("('a''b')", 'T806', Nit.WARNING)
+    self.assertNit("'a''b'", 'T806', Nit.WARNING)
+    self.assertNit("'a \\'' 'b'", 'T806', Nit.WARNING)
+    self.assertNoNits("'a' + 'b'")
+    self.assertNoNits("('a' + 'b')")
+    self.assertNoNits("'''hello!'''")
+    self.assertNoNits('"""hello"""')

pants.ini

@@ -242,6 +242,9 @@ configuration: %(pants_supportdir)s/checkstyle/coding_style.xml
 [lint.google-java-format]
 skip: True
 
+[lint.pythonstyle]
+suppress: %(pants_supportdir)s/checkstyle/python_suppressions.txt
+
 [lint.python-eval]
 # After we fix the cycles from the engine refactor we should re-enable this.
 # https://github.com/pantsbuild/pants/issues/4601