Add presubmit for Java against *ForTesting in production

For (Objective-)C++, there is
_CheckNoProductionCodeUsingTestOnlyFunctions in //PRESUBMIT.py which
can detect when a method with a ForTesting suffix is referenced in
production code.

This bug tracks adding such similar presubmit check for production
.java files, which are approximated as .java files with no "test"
or "junit" in the path and filename.

The check is a modified copy of the one for C++. It does not attempt
to share code with the C++ check, because some details are simpler for
Java (e.g., no need to consider *_for_testing as well). The check is
just a presubmit prompt, not error, because there are false positives
to be expected.

Bug: 821981
Change-Id: I3bb137197070ac696fbe0fd35d50abbfb823a8d8
Reviewed-on: https://chromium-review.googlesource.com/977581Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546063}

PRESUBMIT.py

@@ -658,6 +658,49 @@ def _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api):
     return []
 
 
+def _CheckNoProductionCodeUsingTestOnlyFunctionsJava(input_api, output_api):
+  """This is a simplified version of
+  _CheckNoProductionCodeUsingTestOnlyFunctions for Java files.
+  """
+  javadoc_start_re = input_api.re.compile(r'^\s*/\*\*')
+  javadoc_end_re = input_api.re.compile(r'^\s*\*/')
+  name_pattern = r'ForTest(s|ing)?'
+  # Describes an occurrence of "ForTest*" inside a // comment.
+  comment_re = input_api.re.compile(r'//.*%s' % name_pattern)
+  # Catch calls.
+  inclusion_re = input_api.re.compile(r'(%s)\s*\(' % name_pattern)
+  # Ignore definitions. (Comments are ignored separately.)
+  exclusion_re = input_api.re.compile(r'(%s)[^;]+\{' % name_pattern)
+
+  problems = []
+  sources = lambda x: input_api.FilterSourceFile(
+    x,
+    black_list=(('(?i).*test', r'.*\/junit\/')
+                + input_api.DEFAULT_BLACK_LIST),
+    white_list=(r'.*\.java$',)
+  )
+  for f in input_api.AffectedFiles(include_deletes=False, file_filter=sources):
+    local_path = f.LocalPath()
+    is_inside_javadoc = False
+    for line_number, line in f.ChangedContents():
+      if is_inside_javadoc and javadoc_end_re.search(line):
+        is_inside_javadoc = False
+      if not is_inside_javadoc and javadoc_start_re.search(line):
+        is_inside_javadoc = True
+      if is_inside_javadoc:
+        continue
+      if (inclusion_re.search(line) and
+          not comment_re.search(line) and
+          not exclusion_re.search(line)):
+        problems.append(
+          '%s:%d\n    %s' % (local_path, line_number, line.strip()))
+
+  if problems:
+    return [output_api.PresubmitPromptOrNotify(_TEST_ONLY_WARNING, problems)]
+  else:
+    return []
+
+
 def _CheckNoIOStreamInHeaders(input_api, output_api):
   """Checks to make sure no .h files include <iostream>."""
   files = []
@@ -2667,6 +2710,8 @@ def _CommonChecks(input_api, output_api):
 
   results.extend(
       _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
+  results.extend(
+      _CheckNoProductionCodeUsingTestOnlyFunctionsJava(input_api, output_api))
   results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
   results.extend(_CheckNoUNIT_TESTInSourceFiles(input_api, output_api))
   results.extend(_CheckDCHECK_IS_ONHasBraces(input_api, output_api))

PRESUBMIT_test.py

@@ -1591,5 +1591,52 @@ class NoProductionCodeUsingTestOnlyFunctions(unittest.TestCase):
     self.assertEqual(0, len(results))
 
 
+class NoProductionJavaCodeUsingTestOnlyFunctions(unittest.TestCase):
+  def testTruePositives(self):
+    mock_input_api = MockInputApi()
+    mock_input_api.files = [
+      MockFile('dir/java/src/foo.java', ['FooForTesting();']),
+      MockFile('dir/java/src/bar.java', ['FooForTests(x);']),
+      MockFile('dir/java/src/baz.java', ['FooForTest(','y',');']),
+      MockFile('dir/java/src/mult.java', [
+        'int x = SomethingLongHere()',
+        '    * SomethingLongHereForTesting();'
+      ]),
+    ]
+
+    results = PRESUBMIT._CheckNoProductionCodeUsingTestOnlyFunctionsJava(
+        mock_input_api, MockOutputApi())
+    self.assertEqual(1, len(results))
+    self.assertEqual(4, len(results[0].items))
+    self.assertTrue('foo.java' in results[0].items[0])
+    self.assertTrue('bar.java' in results[0].items[1])
+    self.assertTrue('baz.java' in results[0].items[2])
+    self.assertTrue('mult.java' in results[0].items[3])
+
+  def testFalsePositives(self):
+    mock_input_api = MockInputApi()
+    mock_input_api.files = [
+      MockFile('dir/java/src/foo.xml', ['FooForTesting();']),
+      MockFile('dir/java/src/foo.java', ['FooForTests() {']),
+      MockFile('dir/java/src/bar.java', ['// FooForTest();']),
+      MockFile('dir/java/src/bar2.java', ['x = 1; // FooForTest();']),
+      MockFile('dir/javatests/src/baz.java', ['FooForTest(','y',');']),
+      MockFile('dir/junit/src/baz.java', ['FooForTest(','y',');']),
+      MockFile('dir/junit/src/javadoc.java', [
+        '/** Use FooForTest(); to obtain foo in tests.'
+        ' */'
+      ]),
+      MockFile('dir/junit/src/javadoc2.java', [
+        '/** ',
+        ' * Use FooForTest(); to obtain foo in tests.'
+        ' */'
+      ]),
+    ]
+
+    results = PRESUBMIT._CheckNoProductionCodeUsingTestOnlyFunctionsJava(
+        mock_input_api, MockOutputApi())
+    self.assertEqual(0, len(results))
+
+
 if __name__ == '__main__':
   unittest.main()