Add pylint to CI and fix all Python

* Ended up bigger than I'd hoped, but I think these are nearly all good changes...

* Add pylint in CI
* Split other linters into own jobs, was getting a bit all-or-nothing in that job
* Add a sane config tweaked from default, but less onerous
* Define the least-delta max-line-length that PEP-008 allows, 99 chars, and reflow a few lines. Looks nice anyway IMO
* Fix all the various errors
* Fix a few typos etc too
* Rename one module (script) that wasn't  python-friendly (snake_case) naming
* ...and references to it
* Initialise some object attributes in the constructor, not randomly elsewhere (soft warning in IDE)

This actually found a few bugs, and gotchas (woohoo) so it'd be good if someone could sanity check those changes here:
  * In get_release_contributors `realname` was never defined so that code path would throw exceptions
 * Some of the regex char classes were actually control characters (missing the raw string prefix), so fixed.

This closes #2546

Actually remove all escaping of literal braces

Simple testing proves this isn't needed here:

$ cat /usr/include/qt/QtCore/qlocale.h | python -c 'import re; import sys; s = sys.stdin.read(); print(re.search(r"enum Country[^\n]+{([^}]+)}", s).group(1))' | md5sum
46625fcf574732033f70b2f10ce25bc4  -

vs
$ cat /usr/include/qt/QtCore/qlocale.h | python -c 'import re; import sys; s = sys.stdin.read(); print(re.search(r"enum Country[^\n]+\{([^}]+)\}", s).group(1))' | md5sum
46625fcf574732033f70b2f10ce25bc4  -

.editorconfig

@@ -27,3 +27,7 @@ function_next_line = false
 
 [libs/**]
 ignore = true
+
+[*.py]
+# This is the widest that is still PEP-8 compliant
+max_line_length = 99

.github/autobuild/get_build_vars.py

@@ -1,9 +1,11 @@
 #!/usr/bin/env python3
-# This script is trigged from the Github Autobuild workflow.
-# It analyzes Jamulus.pro and git push details (tag vs. branch, etc.) to decide
-#   - whether a release should be created,
-#   - whether it is a pre-release, and
-#   - what its title should be.
+"""
+This script is triggered from the GitHub Autobuild workflow.
+It analyzes Jamulus.pro and git push details (tag vs. branch, etc.) to decide
+  - whether a release should be created,
+  - whether it is a pre-release, and
+  - what its title should be.
+"""
 
 import os
 import re
@@ -15,10 +17,10 @@
 def get_version_from_jamulus_pro():
     with open(REPO_PATH + '/Jamulus.pro', 'r') as f:
         pro_content = f.read()
-    m = re.search(r'^VERSION\s*=\s*(\S+)$', pro_content, re.MULTILINE)
-    if not m:
-        raise Exception("Unable to determine Jamulus.pro VERSION")
-    return m.group(1)
+    matches = re.search(r'^VERSION\s*=\s*(\S+)$', pro_content, re.MULTILINE)
+    if not matches:
+        raise ValueError("Unable to determine Jamulus.pro VERSION")
+    return matches.group(1)
 
 
 def get_git_hash():
@@ -34,26 +36,26 @@ def get_git_hash():
 
 def get_build_version(jamulus_pro_version):
     if "dev" in jamulus_pro_version:
-        version = "{}-{}".format(jamulus_pro_version, get_git_hash())
+        version = f"{jamulus_pro_version}-{get_git_hash()}"
         return 'intermediate', version
 
     version = jamulus_pro_version
     return 'release', version
 
 
 def set_github_variable(varname, varval):
-    print("{}='{}'".format(varname, varval))  # console output
-    outputfile = os.getenv('GITHUB_OUTPUT')
-    with open(outputfile, "a") as ghout:
+    print(f"{varname}='{varval}'")  # console output
+    output_file = os.getenv('GITHUB_OUTPUT')
+    with open(output_file, "a") as ghout:
         ghout.write(f"{varname}={varval}\n")
 
 jamulus_pro_version = get_version_from_jamulus_pro()
 set_github_variable("JAMULUS_PRO_VERSION", jamulus_pro_version)
 build_type, build_version = get_build_version(jamulus_pro_version)
 print(f'building a version of type "{build_type}": {build_version}')
 
-fullref = os.environ['GITHUB_REF']
-publish_to_release = bool(re.match(r'^refs/tags/r\d+_\d+_\d+\S*$', fullref))
+full_ref = os.environ['GITHUB_REF']
+publish_to_release = bool(re.match(r'^refs/tags/r\d+_\d+_\d+\S*$', full_ref))
 
 # BUILD_VERSION is required for all builds including branch pushes
 # and PRs:
@@ -65,12 +67,12 @@ def set_github_variable(varname, varval):
 set_github_variable("PUBLISH_TO_RELEASE", str(publish_to_release).lower())
 
 if publish_to_release:
-    reflist = fullref.split("/", 2)
-    release_tag = reflist[2]
+    ref_list = full_ref.split("/", 2)
+    release_tag = ref_list[2]
     release_title = f"Release {build_version}  ({release_tag})"
     is_prerelease = not re.match(r'^r\d+_\d+_\d+$', release_tag)
     if not is_prerelease and build_version != release_tag[1:].replace('_', '.'):
-        raise Exception(f"non-pre-release tag {release_tag} doesn't match Jamulus.pro VERSION = {build_version}")
+        raise ValueError(f"non-pre-release tag {release_tag} doesn't match Jamulus.pro VERSION = {build_version}")
 
     # Those variables are only used when a release is created at all:
     set_github_variable("IS_PRERELEASE", str(is_prerelease).lower())

.github/workflows/coding-style-check.yml

@@ -9,20 +9,22 @@ on:
       - '**.h'
       - '**.mm'
       - '**.sh'
+      - '**.py'
   pull_request:
     branches: [ main ]
     paths:
       - '**.cpp'
       - '**.h'
       - '**.mm'
       - '**.sh'
+      - '**.py'
 
 permissions:
   contents: read
 
 jobs:
-  check-coding-style:
-    name: Verify coding style
+  check-c-like-style:
+    name: Verify C-like coding style
     # As this action runs with minimal permissions and does not influence the build output,
     # we perform no version pinning for the runner or the installed tools here.
     # The clangFormatVersion is based on Ubuntu current LTS (jammy at time of writing).
@@ -36,10 +38,26 @@ jobs:
         # When updating the extension list, remember to update
         # Jamulus.pro's CLANG_SOURCES as well and the paths: in 'on:' list above.
         extensions: 'cpp,h,mm'
+
+  check-bash-style:
+    name: Verify shell script (Bash) coding style
+    runs-on: ubuntu-latest
     # shellcheck is already pre-installed on ubuntu-latest
+    steps:
+    - uses: actions/checkout@v3
     - name: Check .sh with shellcheck
       run: find -name '*.sh' -not -path './libs/*' -exec shellcheck --shell=bash {} +
     - name: Install shfmt
       run: sudo snap install shfmt
     - name: Check .sh with shfmt
       run: shfmt -d .
+
+  check-python-style:
+    name: Verify Python coding style
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v3
+    - name: Install pylint
+      run: pip install --user "pylint < 3.0"
+    - name: Check Python files with pylint
+      run: find ./tools -name '*.py' -print -exec pylint {} +

.pylintrc

@@ -0,0 +1,39 @@
+[MAIN]
+
+# Specify a score threshold under which the program will exit with error.
+fail-under=10
+
+[BASIC]
+
+# Minimum line length for functions/classes that require docstrings, shorter
+# ones are exempt.
+docstring-min-length=100
+
+
+# Good variable names which should always be accepted, separated by a comma.
+good-names=s, f, i, j, k, e, _
+
+# Include a hint for the correct naming format with invalid-name.
+include-naming-hint=yes
+
+[FORMAT]
+
+# Maximum number of characters on a single line.
+# See .editorconfig
+max-line-length=99
+
+disable=
+        # Disable enforcing constants as ALL_CAPS -
+        # pylint produces too many false positives here
+        C0103,
+        # Allow redefining names from outer scope
+        W0621,
+        # Assume utf-8 everywhere
+        W1514,
+        # Allow classes with any number of public methods
+        R0903,
+        # Allow TODOs in code
+        W0511
+
+output-format=colorized
+

CONTRIBUTING.md

@@ -20,6 +20,7 @@ If a feature or function can be accomplished in another way by another system or
 
 ### Source code consistency
 
+#### C-like languages
 Please install and run `clang-format` on your PC **before committing** to maintain a consistent coding style. You should use the version we use to validate the style in our CI [(see our coding-style-check file and look for the `clangFormatVersion`)](https://github.com/jamulussoftware/jamulus/blob/main/.github/workflows/coding-style-check.yml#L20). Our CI will fail and tell you about styling violations.
 
 There are several ways to run clang-format:
@@ -30,7 +31,7 @@ There are several ways to run clang-format:
 
 - By hand: run `clang-format -i <path/to/changed/files>`
 
-#### Style definition
+##### Style definition
 
 Please see the [.clang_format file](https://github.com/jamulussoftware/jamulus/blob/main/.clang-format) in the root folder. In summary:
 
@@ -40,6 +41,12 @@ Please see the [.clang_format file](https://github.com/jamulussoftware/jamulus/b
 - Enclose all bodies of `if`, `else`, `while`, `for`, etc. in braces `{` and `}` on separate lines.
 - Do not use concatinations in strings with parameters. Instead use substitutions. **Do:** `QString ( tr ( "Hello, %1. Have a nice day!" ) ).arg( getName() )` **Don't:** `tr ( "Hello " ) + getName() + tr ( ". Have a nice day!" )` ...to make translation easier.
 
+#### Python
+Please install and use [pylint](https://pylint.org/) to scan any Python code.
+There is a configuration file that defines some overrides,
+and note the [Editorconfig file](.editorconfig) in the project too.
+
+
 ### Supported platforms
 
 We support the following platforms and versions:

Jamulus.pro

@@ -1067,7 +1067,7 @@ DISTFILES += ChangeLog \
     tools/create-translation-issues.sh \
     tools/generate_json_rpc_docs.py \
     tools/get_release_contributors.py \
-    tools/qt5-to-qt6-country-code-table.py \
+    tools/qt5_to_qt6_country_code_table.py \
     tools/update-copyright-notices.sh \
     windows/deploy_windows.ps1 \
     windows/installer.nsi

src/util.h

@@ -834,7 +834,7 @@ class CLocale
     static bool                    IsCountryCodeSupported ( unsigned short iCountryCode );
     static QLocale::Country        GetCountryCodeByTwoLetterCode ( QString sTwoLetterCode );
 #if QT_VERSION >= QT_VERSION_CHECK( 6, 0, 0 )
-    // ./tools/qt5-to-qt6-country-code-table.py generates these lists:
+    // ./tools/qt5_to_qt6_country_code_table.py generates these lists:
     constexpr int const static wireFormatToQt6Table[] = {
         0,   1,   3,   4,   5,   6,   7,   8,   9,   10,  11,  12,  13,  15,  16,  17,  18,  19,  20,  21,  22,  23,  24,  25,  26,  27,  28,
         29,  30,  31,  32,  33,  35,  36,  37,  38,  39,  40,  41,  43,  45,  46,  48,  49,  50,  51,  53,  54,  55,  57,  56,  58,  59,  118,

tools/generate_json_rpc_docs.py

@@ -36,13 +36,14 @@ class DocumentationItem:
     def __init__(self, name, type_):
         """
         @param name: str
-        @param type: str
+        @param type_: str
         """
         self.name = name
         self.type = type_
         self.brief = DocumentationText()
         self.params = []
         self.results = []
+        self.current_tag = None
 
     def handle_tag(self, tag_name):
         """
@@ -57,7 +58,7 @@ def handle_tag(self, tag_name):
             self.current_tag = DocumentationText()
             self.results.append(self.current_tag)
         else:
-            raise Exception("Unknown tag: " + tag_name)
+            raise ValueError("Unknown tag: " + tag_name)
 
     def handle_text(self, text):
         """
@@ -75,11 +76,7 @@ def to_markdown(self):
         """
         @return: Markdown-formatted str with name, brief, params and results
         """
-        output = []
-        output.append("### " + self.name)
-        output.append("")
-        output.append(str(self.brief))
-        output.append("")
+        output = ["### " + self.name, "", str(self.brief), ""]
 
         if len(self.params) > 0:
             output.append("Parameters:")
@@ -133,10 +130,8 @@ def to_markdown(self):
         """
         @return: str containing the Markdown table
         """
-        output = []
-        output.append("| Name | Type | Description |")
-        output.append("| --- | --- | --- |")
-        tag_re = re.compile(r"^\{(\w+)\}\s+([\S]+)\s+-\s+(.*)$", re.DOTALL)
+        output = ["| Name | Type | Description |", "| --- | --- | --- |"]
+        tag_re = re.compile(r"^{(\w+)}\s+(\S+)\s+-\s+(.*)$", re.DOTALL)
         for tag in self.tags:
             text = str(tag)
             # Parse tag in form of "{type} name - description"
@@ -185,7 +180,7 @@ def to_markdown(self):
 
 items.sort(key=lambda item: item.name)
 
-preamble = """
+PREAMBLE = """
 # Jamulus JSON-RPC Documentation
 
 <!--
@@ -251,7 +246,7 @@ def to_markdown(self):
 
 docs_path = os.path.join(repo_root, 'docs', 'JSON-RPC.md')
 with open(docs_path, "w") as f:
-    f.write(preamble)
+    f.write(PREAMBLE)
 
     f.write("## Method reference\n")
     for item in filter(lambda item: item.type == "method", items):