Add style checks to CI

Signed-off-by: Ayan Sinha Mahapatra <[email protected]>

Author: AyanSinhaMahapatra

.github/workflows/docs-ci.yml

@@ -1,4 +1,4 @@
-name: CI Documentation
+name: CI Documentation and Code style
 
 on: [push, pull_request]
 
@@ -21,7 +21,7 @@ jobs:
           python-version: ${{ matrix.python-version }}
 
       - name: Install Dependencies
-        run:  pip install -e .[docs]
+        run:  pip install -e .[docs,testing]
 
       - name: Check Sphinx Documentation build minimally
         working-directory: ./docs
@@ -31,4 +31,5 @@ jobs:
         working-directory: ./docs
         run: ./scripts/doc8_style_check.sh
 
-
+      - name: Check for Code style errors
+        run: make check-ci

Makefile

@@ -33,11 +33,19 @@ valid: isort black
 
 check:
 	@echo "-> Run pycodestyle (PEP8) validation"
-	@${ACTIVATE} pycodestyle --max-line-length=100 --exclude=.eggs,venv,lib,thirdparty,docs,migrations,settings.py,.cache .
+	@${ACTIVATE} pycodestyle --max-line-length=100 --exclude=.eggs,venv,lib,thirdparty,docs,scripts,tests,migrations,settings.py,.cache .
 	@echo "-> Run isort imports ordering validation"
-	@${ACTIVATE} isort --sl --check-only -l 100 setup.py src tests .
+	@${ACTIVATE} isort --sl -l 100 src tests setup.py --check-only
 	@echo "-> Run black validation"
-	@${ACTIVATE} black --check --check -l 100 src tests setup.py
+	@${ACTIVATE} black --check -l 100 src tests setup.py
+
+check-ci:
+	@echo "-> Run pycodestyle (PEP8) validation"
+	pycodestyle --max-line-length=100 --exclude=.eggs,venv,lib,thirdparty,docs,scripts,tests,migrations,settings.py,.cache .
+	@echo "-> Run isort imports ordering validation"
+	isort --sl -l 100 src tests setup.py --check-only
+	@echo "-> Run black validation"
+	black --check -l 100 src tests setup.py
 
 clean:
 	@echo "-> Clean the Python env"

etc/scripts/utils_thirdparty.py

@@ -854,7 +854,7 @@ def fetch_license_files(self, dest_dir=THIRDPARTY_DIR, use_cached_index=False):
                 if TRACE:
                     print(f"Fetched license from remote: {lic_url}")
 
-            except:
+            except Exception:
                 try:
                     # try licensedb second
                     lic_url = f"{LICENSEDB_API_URL}/{filename}"
@@ -867,7 +867,7 @@ def fetch_license_files(self, dest_dir=THIRDPARTY_DIR, use_cached_index=False):
                     if TRACE:
                         print(f"Fetched license from licensedb: {lic_url}")
 
-                except:
+                except Exception:
                     msg = f'No text for license {filename} in expression "{self.license_expression}" from {self}'
                     print(msg)
                     errors.append(msg)
@@ -1302,7 +1302,7 @@ def is_pure(self):
 def is_pure_wheel(filename):
     try:
         return Wheel.from_filename(filename).is_pure()
-    except:
+    except Exception:
         return False
 
 

src/commoncode/cliutils.py

@@ -10,9 +10,7 @@
 import sys
 
 import click
-
-# FIXME: this is NOT API
-from click._termui_impl import ProgressBar
+from click._termui_impl import ProgressBar  # FIXME: this is NOT API
 from click.termui import style
 from click.types import BoolParamType
 from click.utils import echo

src/commoncode/command.py

@@ -203,7 +203,7 @@ def close_pipe(p):
         # Ensure process death otherwise proc.wait may hang in some cases
         # NB: this will run only on POSIX OSes supporting signals
         os.kill(proc.pid, signal.SIGKILL)  # NOQA
-    except:
+    except Exception:
         pass
 
     # This may slow things down a tad on non-POSIX Oses but is safe:

src/commoncode/fileset.py

@@ -161,7 +161,7 @@ def load(location):
     assert os.path.exists(fn) and os.path.isfile(fn), msg
     mode = "r"
     with open(fn, mode) as f:
-        return [l.strip() for l in f if l and l.strip()]
+        return [line.strip() for line in f if line and line.strip()]
 
 
 def includes_excludes(patterns, message):

src/commoncode/fileutils.py

@@ -557,7 +557,7 @@ def _rm_handler(function, path, excinfo):  # NOQA
     elif function == os.remove:
         try:
             delete(path, _err_handler=None)
-        except:
+        except Exception:
             pass
 
         if os.path.exists(path):

src/commoncode/hash.py

@@ -284,9 +284,8 @@ def multi_checksums(location, checksum_names=("md5", "sha1", "sha256", "sha512",
     mapping is guaranted to contains all the requested names as keys. If the location is not a file,
     or if the file is empty, the values are None.
 
-    The purpose of this function is to return a set of checksums for a supported set of checksum
-    algorithms for a given location. This is an API function used in ScanCode --info plugin to get
-    checksum values. 
+    The purpose of this function is to avoid read the same file multiple times
+    to compute different checksums.
     """
     if not filetype.is_file(location):
         return {name: None for name in checksum_names}

src/commoncode/resource.py

@@ -511,7 +511,8 @@ def _create_resources_from_paths(self, root, paths):
                     )
                     if not newpar:
                         raise Exception(
-                            f"ERROR: Codebase._create_resources_from_paths: cannot create parent for: {parent_path!r}"
+                            "ERROR: Codebase._create_resources_from_paths:"
+                            f" cannot create parent for: {parent_path!r}"
                         )
                     parent = newpar
 
@@ -1686,7 +1687,7 @@ def _get_scan_data_helper(self, location):
         """
         try:
             return json.loads(location)
-        except:
+        except Exception:
             location = abspath(normpath(expanduser(location)))
             with open(location) as f:
                 scan_data = json.load(f)
@@ -1842,7 +1843,7 @@ def _populate(self, scan_data):
         ##########################################################
         for attr_name in self.codebase_attributes:
             value = scan_data.get(attr_name)
-            if value == None:
+            if not value:
                 continue
             setattr(self.attributes, attr_name, value)
 

src/commoncode/system.py

@@ -113,12 +113,13 @@ def has_case_sensitive_fs():
     case sensitive by default, newer macOS use APFS which is no longer case
     sensitive by default.
 
-    From https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/APFS_Guide/FAQ/FAQ.html
-        How does Apple File System handle filenames?
-        APFS accepts only valid UTF-8 encoded filenames for creation, and preserves
-        both case and normalization of the filename on disk in all variants. APFS,
-        like HFS+, is case-sensitive on iOS and is available in case-sensitive and
-        case-insensitive variants on macOS, with case-insensitive being the default.
+    From
+    https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/APFS_Guide/FAQ/FAQ.html
+    How does Apple File System handle filenames?
+    APFS accepts only valid UTF-8 encoded filenames for creation, and preserves
+    both case and normalization of the filename on disk in all variants. APFS,
+    like HFS+, is case-sensitive on iOS and is available in case-sensitive and
+    case-insensitive variants on macOS, with case-insensitive being the default.
     """
     return not os.path.exists(__file__.upper())