From 00b96c9bd40991d5c50757ca7b34df3067d8d46a Mon Sep 17 00:00:00 2001
From: Sumner Evans <me@sumnerevans.com>
Date: Fri, 8 May 2020 12:38:16 -0600
Subject: [PATCH] Fix linter errors; use custom check for print statements
 because flake8-print is not working anymore

---
 CONTRIBUTING.rst                     | 23 ++++++++++++++---------
 cicd/custom_style_check.py           | 21 +++++++++++++++++----
 sublime/adapters/subsonic/adapter.py |  3 +--
 sublime/app.py                       |  2 +-
 sublime/cache_manager.py             |  4 +---
 5 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index c336e5f..7cd39c9 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -80,10 +80,11 @@ Code Style
 
 This project follows `PEP-8`_ **strictly**. The *only* exception is maximum line
 length, which is 88 for this project (in accordance with ``black``'s defaults).
-Additionally, lines that contain a single string literal are allowed to extend
-past that.
-Additionally, this project uses ``black`` to enforce consistent, deterministic
-code style.
+Lines that contain a single string literal are allowed to extend past the
+maximum line length limit.
+
+This project uses flake8, mypy, and black to do static analysis of the code and
+to enforce a consistent (and as deterministic as possible) code style.
 
 Although you can technically do all of the formatting yourself, it is
 recommended that you use the following tools (they are automatically installed
@@ -100,19 +101,22 @@ before knowing if your code is the correct style.
   * ``flake8-importorder`` (with the ``edited`` import style): enforce ordering
     of import statements.
   * ``flake8-pep3101``: no ``%`` string formatting.
-  * ``flake8-print`` no print statements. Use the more powerful and useful
-    ``logging`` library instead. In the rare case that you actually want to
-    print to the terminal (the ``--version`` flag for example), then just
-    disable this check with a ``# noqa: T001`` comment.
 
 * `mypy`_ is used for type checking. All type errors must be resolved.
+
 * `black`_ is used for auto-formatting. The CI process runs ``black --check`` to
   make sure that you've run ``black`` on all files (or are just good at manually
   formatting).
+
 * ``TODO`` statements must include an associated issue number (in other words,
   if you want to check in a change with outstanding TODOs, there must be an
   issue associated with it to fix it).
 
+* ``print`` statements are not allowed. Use the more powerful and useful
+  ``logging`` library instead. In the rare case that you actually want to print
+  to the terminal (the ``--version`` flag for example), then just disable this
+  check with a ``# noqa`` or a ``# noqa: T001`` comment.
+
 .. _black: https://github.com/psf/black
 .. _`PEP-8`: https://www.python.org/dev/peps/pep-0008/
 .. _mypy: http://mypy-lang.org/
@@ -122,7 +126,8 @@ checks for uses of ``print``. You can run the same checks that the lint job runs
 yourself with the following commands::
 
     $ flake8
-    $ mypy sublime
+    $ mypy sublime tests/**/*.py
+    $ black --check .
     $ ./cicd/custom_style_check.py
 
 Testing
diff --git a/cicd/custom_style_check.py b/cicd/custom_style_check.py
index 2f5ed37..bbd032f 100755
--- a/cicd/custom_style_check.py
+++ b/cicd/custom_style_check.py
@@ -6,8 +6,17 @@ from pathlib import Path
 
 from termcolor import cprint
 
-todo_re = re.compile(r"#\s*TODO:?\s*")
-accounted_for_todo = re.compile(r"#\s*TODO:?\s*\((#\d+)\)")
+todo_re = re.compile(r"\s*#\s*TODO:?\s*")
+accounted_for_todo = re.compile(r"\s*#\s*TODO:?\s*\((#\d+)\)")
+print_re = re.compile(r"\s+print\(.*\)")
+
+
+def noqa_re(error_id: str = ""):
+    return re.compile(rf"#\s*noqa(:\s*{error_id})?\s*\n$")
+
+
+def eprint(*strings):
+    cprint(" ".join(strings), "red", end="", attrs=["bold"])
 
 
 def check_file(path: Path) -> bool:
@@ -16,8 +25,12 @@ def check_file(path: Path) -> bool:
     valid = True
 
     for i, line in enumerate(file, start=1):
-        if todo_re.search(line) and not accounted_for_todo.search(line):
-            cprint(f"{i}: {line}", "red", end="", attrs=["bold"])
+        if todo_re.match(line) and not accounted_for_todo.match(line):
+            eprint(f"{i}: {line}")
+            valid = False
+
+        if print_re.search(line) and not noqa_re("T001").search(line):
+            eprint(f"{i}: {line}")
             valid = False
 
     file.close()
diff --git a/sublime/adapters/subsonic/adapter.py b/sublime/adapters/subsonic/adapter.py
index 86ddbbe..9431809 100644
--- a/sublime/adapters/subsonic/adapter.py
+++ b/sublime/adapters/subsonic/adapter.py
@@ -1,7 +1,6 @@
 import json
 import logging
 import os
-import types
 from datetime import datetime
 from pathlib import Path
 from time import sleep
@@ -9,7 +8,7 @@ from typing import Any, Dict, List, Optional, Sequence, Tuple, Union
 
 import requests
 
-from .api_objects import Response, Song
+from .api_objects import Response
 from .. import Adapter, api_objects as API, ConfigParamDescriptor
 
 
diff --git a/sublime/app.py b/sublime/app.py
index 67ec072..19e2898 100644
--- a/sublime/app.py
+++ b/sublime/app.py
@@ -15,7 +15,7 @@ except Exception:
     tap_imported = False
 
 try:
-    import keyring
+    # import keyring
 
     has_keyring = True
 except ImportError:
diff --git a/sublime/cache_manager.py b/sublime/cache_manager.py
index a128ea0..a2ea262 100644
--- a/sublime/cache_manager.py
+++ b/sublime/cache_manager.py
@@ -132,9 +132,7 @@ class SearchResult:
         if getattr(self, member) is None:
             setattr(self, member, set())
 
-        setattr(
-            self, member, getattr(getattr(self, member, set()), "union")(set(results)),
-        )
+        setattr(self, member, getattr(self, member, set()).union(set(results)))
 
     def _to_result(self, it: Iterable[S], transform: Callable[[S], str],) -> List[S]:
         all_results = sorted(