From 98b285dafb90b7136dadc55c534888e7155205b1 Mon Sep 17 00:00:00 2001
From: Chih-Hung Hsieh <chh@google.com>
Date: Wed, 28 Apr 2021 14:49:32 -0700
Subject: [PATCH] Fix/suppress most pylint and gpylint warnings

* Add missing function doc strings.
  Suppress this warning on trivial functions in *_warn_patterns.py.
* Remove unused g-importing-memeber, g-complex-comprehension.
* Suppress pylint warning on unrecognized g-* options.
* Suppress too-few-public-methods warnings on simple classes.
* Suppress too-many-arguments and missing-function-docstring in
  html_writer.py, which will be refactored later.
* Fix bad naming, long lines and line breaks, and bad quotes.

Test: compare output for build.log
Change-Id: Icdb34f014a10ec1e642c2cfe8003fc3ae245b507
---
 tools/warn.py                      |  1 +
 tools/warn/android_project_list.py |  1 +
 tools/warn/chrome_project_list.py  |  1 +
 tools/warn/cpp_warn_patterns.py    |  9 ++++--
 tools/warn/html_writer.py          | 25 ++++++++--------
 tools/warn/java_warn_patterns.py   |  7 +++--
 tools/warn/make_warn_patterns.py   |  4 +--
 tools/warn/other_warn_patterns.py  |  7 +++--
 tools/warn/severity.py             |  5 ++--
 tools/warn/tidy_warn_patterns.py   |  8 +++--
 tools/warn/warn.py                 |  5 +++-
 tools/warn/warn_common.py          | 47 ++++++++++++++++++------------
 12 files changed, 75 insertions(+), 45 deletions(-)

diff --git a/tools/warn.py b/tools/warn.py
index 5f796f528..97f54f9b4 100755
--- a/tools/warn.py
+++ b/tools/warn.py
@@ -27,6 +27,7 @@ import sys
 
 
 def main():
+  """Old main() calls warn.warn."""
   os.environ['PYTHONPATH'] = os.path.dirname(os.path.abspath(__file__))
   subprocess.check_call(['/usr/bin/python3', '-m', 'warn.warn'] + sys.argv[1:])
 
diff --git a/tools/warn/android_project_list.py b/tools/warn/android_project_list.py
index 82c0fbd63..8383dc0c3 100644
--- a/tools/warn/android_project_list.py
+++ b/tools/warn/android_project_list.py
@@ -17,6 +17,7 @@
 
 
 def create_pattern(name, pattern=None):
+  """Return a tuple of name and warn patten."""
   if pattern is not None:
     return [name, '(^|.*/)' + pattern + '/.*: warning:']
   return [name, '(^|.*/)' + name + '/.*: warning:']
diff --git a/tools/warn/chrome_project_list.py b/tools/warn/chrome_project_list.py
index 609652267..d8b2179c8 100644
--- a/tools/warn/chrome_project_list.py
+++ b/tools/warn/chrome_project_list.py
@@ -8,6 +8,7 @@ unification of the Chrome and Android warn.py.
 
 
 def create_pattern(pattern):
+  """Return a tuple of name and warn patten."""
   return [pattern, '(^|.*/)' + pattern + '/.*: warning:']
 
 
diff --git a/tools/warn/cpp_warn_patterns.py b/tools/warn/cpp_warn_patterns.py
index e8783bc47..2fa9916ec 100644
--- a/tools/warn/cpp_warn_patterns.py
+++ b/tools/warn/cpp_warn_patterns.py
@@ -15,10 +15,12 @@
 
 """Warning patterns for C/C++ compiler, but not clang-tidy."""
 
+# No need of doc strings for trivial small functions.
+# pylint:disable=missing-function-docstring
+
 import re
 
 # pylint:disable=relative-beyond-top-level
-# pylint:disable=g-importing-member
 from .severity import Severity
 
 
@@ -56,7 +58,8 @@ def harmless(description, pattern_list):
 
 
 warn_patterns = [
-    # pylint:disable=line-too-long,g-inconsistent-quotes
+    # pylint does not recognize g-inconsistent-quotes
+    # pylint:disable=line-too-long,bad-option-value,g-inconsistent-quotes
     medium('Implicit function declaration',
            [r".*: warning: implicit declaration of function .+",
             r".*: warning: implicitly declaring library function"]),
@@ -300,7 +303,7 @@ warn_patterns = [
     medium('Missing noreturn',
            [r".*: warning: function '.*' could be declared with attribute 'noreturn'"]),
     medium('User warning',
-           [r".*: warning: #warning "".+"""]),
+           [r".*: warning: #warning \".+\""]),
     medium('Vexing parsing problem',
            [r".*: warning: empty parentheses interpreted as a function declaration"]),
     medium('Dereferencing void*',
diff --git a/tools/warn/html_writer.py b/tools/warn/html_writer.py
index be71b5506..bed25ed6f 100644
--- a/tools/warn/html_writer.py
+++ b/tools/warn/html_writer.py
@@ -15,6 +15,9 @@
 
 """Emit warning messages to html or csv files."""
 
+# Many functions in this module have too many arguments to be refactored.
+# pylint:disable=too-many-arguments,missing-function-docstring
+
 # To emit html page of warning messages:
 #   flags: --byproject, --url, --separator
 # Old stuff for static html components:
@@ -57,11 +60,10 @@ import html
 import sys
 
 # pylint:disable=relative-beyond-top-level
-# pylint:disable=g-importing-member
 from .severity import Severity
 
 
-html_head_scripts = """\
+HTML_HEAD_SCRIPTS = """\
   <script type="text/javascript">
   function expand(id) {
     var e = document.getElementById(id);
@@ -113,7 +115,7 @@ def html_big(param):
 def dump_html_prologue(title, writer, warn_patterns, project_names):
   writer('<html>\n<head>')
   writer('<title>' + title + '</title>')
-  writer(html_head_scripts)
+  writer(HTML_HEAD_SCRIPTS)
   emit_stats_by_project(writer, warn_patterns, project_names)
   writer('</head>\n<body>')
   writer(html_big(title))
@@ -142,7 +144,7 @@ def create_warnings(warn_patterns, project_names):
     2D warnings array where warnings[p][s] is # of warnings in project name p of
     severity level s
   """
-  # pylint:disable=g-complex-comprehension
+  # pylint:disable=invalid-name
   warnings = {p: {s.value: 0 for s in Severity.levels} for p in project_names}
   for i in warn_patterns:
     s = i['severity'].value
@@ -153,7 +155,6 @@ def create_warnings(warn_patterns, project_names):
 
 def get_total_by_project(warnings, project_names):
   """Returns dict, project as key and # warnings for that project as value."""
-  # pylint:disable=g-complex-comprehension
   return {
       p: sum(warnings[p][s.value] for s in Severity.levels)
       for p in project_names
@@ -162,7 +163,6 @@ def get_total_by_project(warnings, project_names):
 
 def get_total_by_severity(warnings, project_names):
   """Returns dict, severity as key and # warnings of that severity as value."""
-  # pylint:disable=g-complex-comprehension
   return {
       s.value: sum(warnings[p][s.value] for p in project_names)
       for s in Severity.levels
@@ -235,7 +235,7 @@ def emit_row_counts_per_severity(total_by_severity, stats_header, stats_rows,
   writer('<script>')
   emit_const_string_array('StatsHeader', stats_header, writer)
   emit_const_object_array('StatsRows', stats_rows, writer)
-  writer(draw_table_javascript)
+  writer(DRAW_TABLE_JAVASCRIPT)
   writer('</script>')
 
 
@@ -246,8 +246,8 @@ def emit_stats_by_project(writer, warn_patterns, project_names):
   total_by_project = get_total_by_project(warnings, project_names)
   total_by_severity = get_total_by_severity(warnings, project_names)
   stats_header = emit_table_header(total_by_severity)
-  total_all_projects, stats_rows = \
-    emit_row_counts_per_project(warnings, total_by_project, total_by_severity, project_names)
+  total_all_projects, stats_rows = emit_row_counts_per_project(
+      warnings, total_by_project, total_by_severity, project_names)
   emit_row_counts_per_severity(total_by_severity, stats_header, stats_rows,
                                total_all_projects, writer)
 
@@ -287,6 +287,7 @@ def dump_stats(writer, warn_patterns):
 #     id for each warning pattern
 #     sort by project, severity, warn_id, warning_message
 def emit_buttons(writer):
+  """Write the button elements in HTML."""
   writer('<button class="button" onclick="expandCollapse(1);">'
          'Expand all warnings</button>\n'
          '<button class="button" onclick="expandCollapse(0);">'
@@ -412,7 +413,7 @@ def emit_warning_arrays(writer, warn_patterns):
   writer('];')
 
 
-scripts_for_warning_groups = """
+SCRIPTS_FOR_WARNING_GROUPS = """
   function compareMessages(x1, x2) { // of the same warning type
     return (WarningMessages[x1[2]] <= WarningMessages[x2[2]]) ? -1 : 1;
   }
@@ -620,7 +621,7 @@ def emit_js_data(writer, flags, warning_messages, warning_links,
     emit_const_html_string_array('WarningLinks', warning_links, writer)
 
 
-draw_table_javascript = """
+DRAW_TABLE_JAVASCRIPT = """
 google.charts.load('current', {'packages':['table']});
 google.charts.setOnLoadCallback(drawTable);
 function drawTable() {
@@ -653,7 +654,7 @@ def dump_html(flags, output_stream, warning_messages, warning_links,
   writer('\n<script>')
   emit_js_data(writer, flags, warning_messages, warning_links, warning_records,
                warn_patterns, project_names)
-  writer(scripts_for_warning_groups)
+  writer(SCRIPTS_FOR_WARNING_GROUPS)
   writer('</script>')
   emit_buttons(writer)
   # Warning messages are grouped by severities or project names.
diff --git a/tools/warn/java_warn_patterns.py b/tools/warn/java_warn_patterns.py
index ac1ed5d9b..534f48d2c 100644
--- a/tools/warn/java_warn_patterns.py
+++ b/tools/warn/java_warn_patterns.py
@@ -15,8 +15,10 @@
 
 """Warning patterns for Java compiler tools."""
 
+# No need of doc strings for trivial small functions.
+# pylint:disable=missing-function-docstring
+
 # pylint:disable=relative-beyond-top-level
-# pylint:disable=g-importing-member
 from .cpp_warn_patterns import compile_patterns
 from .severity import Severity
 
@@ -59,7 +61,8 @@ def low(name, description=None):
 
 
 warn_patterns = [
-    # pylint:disable=line-too-long,g-inconsistent-quotes
+    # pylint does not recognize g-inconsistent-quotes
+    # pylint:disable=line-too-long,bad-option-value,g-inconsistent-quotes
     # Warnings from Javac
     java_medium('Use of deprecated',
                 [r'.*: warning: \[deprecation\] .+',
diff --git a/tools/warn/make_warn_patterns.py b/tools/warn/make_warn_patterns.py
index 4b20493ba..a54c502f3 100644
--- a/tools/warn/make_warn_patterns.py
+++ b/tools/warn/make_warn_patterns.py
@@ -16,12 +16,12 @@
 """Warning patterns for build make tools."""
 
 # pylint:disable=relative-beyond-top-level
-# pylint:disable=g-importing-member
 from .cpp_warn_patterns import compile_patterns
 from .severity import Severity
 
 warn_patterns = [
-    # pylint:disable=line-too-long,g-inconsistent-quotes
+    # pylint does not recognize g-inconsistent-quotes
+    # pylint:disable=line-too-long,bad-option-value,g-inconsistent-quotes
     {'category': 'make', 'severity': Severity.MEDIUM,
      'description': 'make: overriding commands/ignoring old commands',
      'patterns': [r".*: warning: overriding commands for target .+",
diff --git a/tools/warn/other_warn_patterns.py b/tools/warn/other_warn_patterns.py
index 8df5b87d7..d05c8e90f 100644
--- a/tools/warn/other_warn_patterns.py
+++ b/tools/warn/other_warn_patterns.py
@@ -15,8 +15,10 @@
 
 """Warning patterns from other tools."""
 
+# No need of doc strings for trivial small functions.
+# pylint:disable=missing-function-docstring
+
 # pylint:disable=relative-beyond-top-level
-# pylint:disable=g-importing-member
 from .cpp_warn_patterns import compile_patterns
 from .severity import Severity
 
@@ -57,7 +59,8 @@ def rust(severity, description, pattern):
 
 
 warn_patterns = [
-    # pylint:disable=line-too-long,g-inconsistent-quotes
+    # pylint does not recognize g-inconsistent-quotes
+    # pylint:disable=line-too-long,bad-option-value,g-inconsistent-quotes
     # aapt warnings
     aapt('No comment for public symbol',
          [r".*: warning: No comment for public symbol .+"]),
diff --git a/tools/warn/severity.py b/tools/warn/severity.py
index b4c03c9ef..20064c379 100644
--- a/tools/warn/severity.py
+++ b/tools/warn/severity.py
@@ -19,8 +19,9 @@ This file stores definition for class Severity that is used in warn_patterns.
 """
 
 
-# pylint:disable=old-style-class
+# pylint:disable=too-few-public-methods
 class SeverityInfo:
+  """Class of Severity Info, part of a Severity object."""
 
   def __init__(self, value, color, column_header, header):
     self.value = value
@@ -29,7 +30,7 @@ class SeverityInfo:
     self.header = header
 
 
-# pylint:disable=old-style-class
+# pylint:disable=too-few-public-methods
 class Severity:
   """Class of Severity levels where each level is a SeverityInfo."""
 
diff --git a/tools/warn/tidy_warn_patterns.py b/tools/warn/tidy_warn_patterns.py
index 5416cb23d..7018d1089 100644
--- a/tools/warn/tidy_warn_patterns.py
+++ b/tools/warn/tidy_warn_patterns.py
@@ -15,8 +15,10 @@
 
 """Warning patterns for clang-tidy."""
 
+# No need of doc strings for trivial small functions.
+# pylint:disable=missing-function-docstring
+
 # pylint:disable=relative-beyond-top-level
-# pylint:disable=g-importing-member
 from .cpp_warn_patterns import compile_patterns
 from .severity import Severity
 
@@ -39,7 +41,6 @@ def group_tidy_warn_pattern(description):
 
 
 def analyzer_high(description, patterns):
-  # Important clang analyzer warnings to be fixed ASAP.
   return {
       'category': 'C/C++',
       'severity': Severity.HIGH,
@@ -74,7 +75,8 @@ def analyzer_group_check(check):
 
 
 warn_patterns = [
-    # pylint:disable=line-too-long,g-inconsistent-quotes
+    # pylint does not recognize g-inconsistent-quotes
+    # pylint:disable=line-too-long,bad-option-value,g-inconsistent-quotes
     group_tidy_warn_pattern('android'),
     simple_tidy_warn_pattern('abseil-string-find-startswith'),
     simple_tidy_warn_pattern('bugprone-argument-comment'),
diff --git a/tools/warn/warn.py b/tools/warn/warn.py
index cb5daec71..acfbb5592 100755
--- a/tools/warn/warn.py
+++ b/tools/warn/warn.py
@@ -20,7 +20,8 @@ import multiprocessing
 import signal
 import sys
 
-# pylint:disable=relative-beyond-top-level
+# pylint:disable=relative-beyond-top-level,no-name-in-module
+# suppress false positive of no-name-in-module warnings
 from . import warn_common as common
 
 
@@ -50,6 +51,7 @@ def classify_warnings(args):
 
 def create_and_launch_subprocesses(num_cpu, classify_warnings_fn, arg_groups,
                                    group_results):
+  """Fork num_cpu processes to classify warnings."""
   pool = multiprocessing.Pool(num_cpu)
   for cpu in range(num_cpu):
     proc_result = pool.map(classify_warnings_fn, arg_groups[cpu])
@@ -59,6 +61,7 @@ def create_and_launch_subprocesses(num_cpu, classify_warnings_fn, arg_groups,
 
 
 def main():
+  """Old main() calls new common_main."""
   use_google3 = False
   common.common_main(use_google3, create_and_launch_subprocesses,
                      classify_warnings)
diff --git a/tools/warn/warn_common.py b/tools/warn/warn_common.py
index b2dd8abff..d69050f54 100755
--- a/tools/warn/warn_common.py
+++ b/tools/warn/warn_common.py
@@ -52,8 +52,8 @@ import os
 import re
 import sys
 
-# pylint:disable=relative-beyond-top-level
-# pylint:disable=g-importing-member
+# pylint:disable=relative-beyond-top-level,no-name-in-module
+# suppress false positive of no-name-in-module warnings
 from . import android_project_list
 from . import chrome_project_list
 from . import cpp_warn_patterns as cpp_patterns
@@ -115,6 +115,8 @@ def get_project_names(project_list):
 
 
 def find_project_index(line, project_patterns):
+  """Return the index to the project pattern array."""
+  # pylint:disable=invalid-name
   for i, p in enumerate(project_patterns):
     if p.match(line):
       return i
@@ -124,30 +126,30 @@ def find_project_index(line, project_patterns):
 def classify_one_warning(warning, link, results, project_patterns,
                          warn_patterns):
   """Classify one warning line."""
+  # pylint:disable=invalid-name
   for i, w in enumerate(warn_patterns):
     for cpat in w['compiled_patterns']:
       if cpat.match(warning):
         p = find_project_index(warning, project_patterns)
         results.append([warning, link, i, p])
         return
-      else:
-        # If we end up here, there was a problem parsing the log
-        # probably caused by 'make -j' mixing the output from
-        # 2 or more concurrent compiles
-        pass
+  # If we end up here, there was a problem parsing the log
+  # probably caused by 'make -j' mixing the output from
+  # 2 or more concurrent compiles
 
 
-def remove_prefix(s, sub):
-  """Remove everything before last occurrence of substring sub in string s."""
-  if sub in s:
-    inc_sub = s.rfind(sub)
-    return s[inc_sub:]
-  return s
+def remove_prefix(src, sub):
+  """Remove everything before last occurrence of substring sub in string src."""
+  if sub in src:
+    inc_sub = src.rfind(sub)
+    return src[inc_sub:]
+  return src
 
 
 # TODO(emmavukelj): Don't have any generate_*_cs_link functions call
 # normalize_path a second time (the first time being in parse_input_file)
 def generate_cs_link(warning_line, flags, android_root=None):
+  """Try to add code search HTTP URL prefix."""
   if flags.platform == 'chrome':
     return generate_chrome_cs_link(warning_line, flags)
   if flags.platform == 'android':
@@ -279,8 +281,7 @@ def normalize_path(path, flags, android_root=None):
   if idx >= 0:
     # remove chrome_root/, we want path relative to that
     return path[idx + len('chrome_root/'):]
-  else:
-    return path
+  return path
 
 
 def normalize_warning_line(line, flags, android_root=None):
@@ -309,6 +310,7 @@ def parse_input_file_chrome(infile, flags):
   # Remove the duplicated warnings save ~8% of time when parsing
   # one typical build log than before
   unique_warnings = dict()
+  # pylint:disable=invalid-name
   for line in infile:
     if warning_pattern.match(line):
       normalized_line = normalize_warning_line(line, flags)
@@ -344,6 +346,7 @@ def add_normalized_line_to_warnings(line, flags, android_root, unique_warnings):
 
 def parse_input_file_android(infile, flags):
   """Parse Android input file, collect parameters and warning lines."""
+  # pylint:disable=too-many-locals,too-many-branches
   platform_version = 'unknown'
   target_product = 'unknown'
   target_variant = 'unknown'
@@ -393,6 +396,7 @@ def parse_input_file_android(infile, flags):
             line, flags, android_root, unique_warnings)
       continue
 
+    # pylint:disable=invalid-name
     if line_counter < 100:
       # save a little bit of time by only doing this for the first few lines
       line_counter += 1
@@ -424,6 +428,7 @@ def parse_input_file_android(infile, flags):
 
 
 def parse_input_file(infile, flags):
+  """Parse one input file for chrome or android."""
   if flags.platform == 'chrome':
     return parse_input_file_chrome(infile, flags)
   if flags.platform == 'android':
@@ -448,9 +453,12 @@ def get_warn_patterns(platform):
   if platform == 'chrome':
     warn_patterns = cpp_patterns.warn_patterns
   elif platform == 'android':
-    warn_patterns = make_patterns.warn_patterns + cpp_patterns.warn_patterns + java_patterns.warn_patterns + tidy_patterns.warn_patterns + other_patterns.warn_patterns
+    warn_patterns = (make_patterns.warn_patterns + cpp_patterns.warn_patterns +
+                     java_patterns.warn_patterns + tidy_patterns.warn_patterns +
+                     other_patterns.warn_patterns)
   else:
     raise Exception('platform name %s is not valid' % platform)
+  # pylint:disable=invalid-name
   for w in warn_patterns:
     w['members'] = []
     # Each warning pattern has a 'projects' dictionary, that
@@ -473,6 +481,7 @@ def parallel_classify_warnings(warning_data, args, project_names,
                                use_google3, create_launch_subprocs_fn,
                                classify_warnings_fn):
   """Classify all warning lines with num_cpu parallel processes."""
+  # pylint:disable=too-many-arguments,too-many-locals
   num_cpu = args.processes
   group_results = []
 
@@ -531,8 +540,10 @@ def parallel_classify_warnings(warning_data, args, project_names,
 def process_log(logfile, flags, project_names, project_patterns, warn_patterns,
                 html_path, use_google3, create_launch_subprocs_fn,
                 classify_warnings_fn, logfile_object):
-  # pylint: disable=g-doc-args
-  # pylint: disable=g-doc-return-or-yield
+  # pylint does not recognize g-doc-*
+  # pylint: disable=bad-option-value,g-doc-args
+  # pylint: disable=bad-option-value,g-doc-return-or-yield
+  # pylint: disable=too-many-arguments,too-many-locals
   """Function that handles processing of a log.
 
   This is isolated into its own function (rather than just taking place in main)