[Android log] Fix presubmit check reports in base package

PRESUBMIT.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Top-level presubmit script for Chromium.
 
 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
 for more details about the presubmit API built into depot_tools.
 """
 
@@ skipping 1293 matching lines @@
       black_list=_EXCLUDED_PATHS + input_api.DEFAULT_BLACK_LIST)
 
 
 def _CheckAndroidCrLogUsage(input_api, output_api):
   """Checks that new logs using org.chromium.base.Log:
     - Are using 'TAG' as variable name for the tags (warn)
     - Are using the suggested name format for the tags: "cr.<PackageTag>" (warn)
     - Are using a tag that is shorter than 23 characters (error)
   """
   cr_log_import_pattern = input_api.re.compile(
-      r'^import org\.chromium\.base\.Log;$', input_api.re.MULTILINE);
+      r'^import org\.chromium\.base\.Log;$', input_api.re.MULTILINE)
+  class_in_base_pattern = input_api.re.compile(
+      r'^package org\.chromium\.base;$', input_api.re.MULTILINE)
+  has_some_log_import_pattern = input_api.re.compile(
+      r'^import .*\.Log;$', input_api.re.MULTILINE)
   # Extract the tag from lines like `Log.d(TAG, "*");` or `Log.d("TAG", "*");`
-  cr_log_pattern = input_api.re.compile(r'^\s*Log\.\w\((?P<tag>\"?\w+\"?)\,')
+  log_call_pattern = input_api.re.compile(r'^\s*Log\.\w\((?P<tag>\"?\w+\"?)\,')
   log_decl_pattern = input_api.re.compile(
       r'^\s*private static final String TAG = "(?P<name>(.*)")',
       input_api.re.MULTILINE)
   log_name_pattern = input_api.re.compile(r'^cr[.\w]*')
 
   REF_MSG = ('See base/android/java/src/org/chromium/base/README_logging.md '
             'or contact dgn@chromium.org for more info.')
   sources = lambda x: input_api.FilterSourceFile(x, white_list=(r'.*\.java$',))
-  tag_errors = []
+
   tag_decl_errors = []
   tag_length_errors = []
+  tag_errors = []
+  util_log_errors = []
 
   for f in input_api.AffectedSourceFiles(sources):
     file_content = input_api.ReadFile(f)
     has_modified_logs = False
 
     # Per line checks
-    if cr_log_import_pattern.search(file_content):
+    if (cr_log_import_pattern.search(file_content) or
+        (class_in_base_pattern.search(file_content) and

[dgn, 2015/06/11 15:33:58]

How can I make it in one check?

I think the closest I got was with something like

class_in_base_without_log_import_pattern = input_api.re.compile(
    r'package org\.chromium\.base;'  # Must be in the base package
     '(?:\n.*)*'                     # There can be some lines of code
     'import(?!.*\.Log;)')           # Eventual imports must not be about Log

But that only works properly with one import statement.

+            not has_some_log_import_pattern.search(file_content))):
+      # Checks to run for files using cr log
       for line_num, line in f.ChangedContents():
 
         # Check if the new line is doing some logging
-        match = cr_log_pattern.search(line)
+        match = log_call_pattern.search(line)
         if match:
           has_modified_logs = True
 
           # Make sure it uses "TAG"
           if not match.group('tag') == 'TAG':
             tag_errors.append("%s:%d" % (f.LocalPath(), line_num))
+    else:
+      # Report non cr Log function calls in changed lines
+      for line_num, line in f.ChangedContents():
+        if log_call_pattern.search(line):
+          util_log_errors.append("%s:%d" % (f.LocalPath(), line_num))
 
     # Per file checks
     if has_modified_logs:
       # Make sure the tag is using the "cr" prefix and is not too long
       match = log_decl_pattern.search(file_content)
       tag_name = match.group('name') if match else ''
       if not log_name_pattern.search(tag_name ):
         tag_decl_errors.append(f.LocalPath())
       if len(tag_name) > 23:
         tag_length_errors.append(f.LocalPath())
 
   results = []
   if tag_decl_errors:
     results.append(output_api.PresubmitPromptWarning(
         'Please define your tags using the suggested format: .\n'
         '"private static final String TAG = "cr.<package tag>".\n' + REF_MSG,
         tag_decl_errors))
 
   if tag_length_errors:
     results.append(output_api.PresubmitError(
         'The tag length is restricted by the system to be at most '
         '23 characters.\n' + REF_MSG,
         tag_length_errors))
 
   if tag_errors:
     results.append(output_api.PresubmitPromptWarning(
         'Please use a variable named "TAG" for your log tags.\n' + REF_MSG,
         tag_errors))
 
+  if util_log_errors:
+    results.append(output_api.PresubmitPromptWarning(
+        'Please use org.chromium.base.Log for new logs.\n' + REF_MSG,
+        util_log_errors))
+
   return results
 
 
-# TODO(dgn): refactor with _CheckAndroidCrLogUsage
-def _CheckNoNewUtilLogUsage(input_api, output_api):
-  """Checks that new logs are using org.chromium.base.Log."""
-
-  chromium_log_import_pattern = input_api.re.compile(
-      r'^import org\.chromium\.base\.Log;$', input_api.re.MULTILINE);
-  log_pattern = input_api.re.compile(r'^\s*(android\.util\.)?Log\.\w')
-  sources = lambda x: input_api.FilterSourceFile(x, white_list=(r'.*\.java$',))
-
-  errors = []
-
-  for f in input_api.AffectedSourceFiles(sources):
-    if chromium_log_import_pattern.search(input_api.ReadFile(f)) is not None:
-      # Uses org.chromium.base.Log already
-      continue
-
-    for line_num, line in f.ChangedContents():
-      if log_pattern.search(line):
-        errors.append("%s:%d" % (f.LocalPath(), line_num))
-
-  results = []
-  if len(errors):
-    results.append(output_api.PresubmitPromptWarning(
-        'Please use org.chromium.base.Log for new logs.\n' +
-        'See base/android/java/src/org/chromium/base/README_logging.md ' +
-        'or contact dgn@chromium.org for more info.',
-        errors))
-  return results
-
-
 def _CheckForCopyrightedCode(input_api, output_api):
   """Verifies that newly added code doesn't contain copyrighted material
   and is properly licensed under the standard Chromium license.
 
   As there can be false positives, we maintain a whitelist file. This check
   also verifies that the whitelist file is up to date.
   """
   import sys
   original_sys_path = sys.path
   try:
@@ skipping 108 matching lines @@
         if deprecated in line:
           results.append(output_api.PresubmitError(
               "%s:%d: Use of deprecated JS %s, use %s instead" %
               (fpath.LocalPath(), lnum, deprecated, replacement)))
   return results
 
 
 def _AndroidSpecificOnUploadChecks(input_api, output_api):
   """Groups checks that target android code."""
   results = []
-  results.extend(_CheckNoNewUtilLogUsage(input_api, output_api))
   results.extend(_CheckAndroidCrLogUsage(input_api, output_api))
   return results
 
 
 def _CommonChecks(input_api, output_api):
   """Checks common to both upload and commit."""
   results = []
   results.extend(input_api.canned_checks.PanProjectChecks(
       input_api, output_api,
       excluded_paths=_EXCLUDED_PATHS + _TESTRUNNER_PATHS))
@@ skipping 343 matching lines @@
       # (e.g. OWNERS checks before finished code review), and we're
       # running local presubmit anyway.
       if 'presubmit' in builder:
         masters[master].pop(builder)
       else:
         # Convert testfilter format to the one expected by git-cl-try.
         testfilter = masters[master][builder].get('testfilter', 'defaulttests')
         masters[master][builder] = [testfilter]
 
   return masters