Enable Pylint in CI and fix its errors (#311)

* Remove wildcard imports

Use explicit imports rather than wildcards. This is more maintainable.

* Enable Pylint in CI and fix its errors

The main fixes were:

* Specify encoding for all file opens. By default it depends on environment variables which is bad.
* Use `with` to open files. Otherwise they don't necessarily get closed.

There were also a few minor things like using `enumerate`, not using objects as default arguments, etc. In some cases I slightly refactored the code.

.pre-commit-config.yaml

@@ -21,11 +21,10 @@ repos:
     hooks:
       - id: black
 
-  # TODO: Enable this when lints are fixed.
-  # - repo: https://github.com/PyCQA/pylint
-  #   rev: v3.3.1
-  #   hooks:
-  #     - id: pylint
+  - repo: https://github.com/PyCQA/pylint
+    rev: v3.3.1
+    hooks:
+      - id: pylint
 
   - repo: https://github.com/RobertCraigie/pyright-python
     rev: v1.1.383

.pylintrc

@@ -0,0 +1,31 @@
+[MAIN]
+py-version = 3.9.0
+disable=
+    # Allow 'TODO:' in code.
+    fixme,
+    # Overly zealous duplicate code detection.
+    duplicate-code,
+    # These debatable style lints are quite annoying, and often push
+    # you into mixing up small changes (adding one statement to a function)
+    # with large refactors (splitting the function up into shorter functions).
+    too-few-public-methods,
+    too-many-arguments,
+    too-many-positional-arguments,
+    too-many-branches,
+    too-many-instance-attributes,
+    too-many-locals,
+    too-many-return-statements,
+    too-many-statements,
+    # Handled by Black.
+    line-too-long,
+    # This is technically correct but not that important.
+    logging-fstring-interpolation,
+    # TODO: These should be enabled but writing documentation for
+    # all of the code is not feasible in one go.
+    missing-module-docstring,
+    missing-function-docstring,
+    missing-class-docstring,
+
+# These names are fine when used sensibly. Without listing them here
+# Pylint will complain they are too short.
+good-names=c,i,j,k,id,pc

c_utils.py

@@ -2,7 +2,8 @@
 import os
 import pprint
 
-from shared_utils import *
+from constants import causes, csrs, csrs32
+from shared_utils import InstrDict, arg_lut
 
 pp = pprint.PrettyPrinter(indent=2)
 logging.basicConfig(level=logging.INFO, format="%(levelname)s:: %(message)s")
@@ -42,7 +43,7 @@ def make_c(instr_dict: InstrDict):
         mask = ((1 << (end - begin + 1)) - 1) << begin
         arg_str += f"#define INSN_FIELD_{sanitized_name.upper()} {hex(mask)}\n"
 
-    with open(f"{os.path.dirname(__file__)}/encoding.h", "r") as file:
+    with open(f"{os.path.dirname(__file__)}/encoding.h", "r", encoding="utf-8") as file:
         enc_header = file.read()
 
     commit = os.popen('git log -1 --format="format:%h"').read()
@@ -74,5 +75,5 @@ def make_c(instr_dict: InstrDict):
 """
 
     # Write the modified output to the file
-    with open("encoding.out.h", "w") as enc_file:
+    with open("encoding.out.h", "w", encoding="utf-8") as enc_file:
         enc_file.write(output_str)

chisel_utils.py

@@ -1,10 +1,8 @@
 import logging
 import pprint
 
-from constants import *
-
-# from shared_utils import overlaps, overlap_allowed, extension_overlap_allowed, instruction_overlap_allowed, process_enc_line, same_base_isa, add_segmented_vls_insn, expand_nf_field
-from shared_utils import *
+from constants import causes, csrs, csrs32
+from shared_utils import InstrDict, instr_dict_2_extensions
 
 pp = pprint.PrettyPrinter(indent=2)
 logging.basicConfig(level=logging.INFO, format="%(levelname)s:: %(message)s")
@@ -23,7 +21,6 @@ def make_chisel(instr_dict: InstrDict, spinal_hdl: bool = False):
     if not spinal_hdl:
         extensions = instr_dict_2_extensions(instr_dict)
         for e in extensions:
-            e_instrs = filter(lambda i: instr_dict[i]["extension"][0] == e, instr_dict)
             if "rv64_" in e:
                 e_format = e.replace("rv64_", "").upper() + "64"
             elif "rv32_" in e:
@@ -33,10 +30,11 @@ def make_chisel(instr_dict: InstrDict, spinal_hdl: bool = False):
             else:
                 e_format = e.upper()
             chisel_names += f'  val {e_format+"Type"} = Map(\n'
-            for instr in e_instrs:
-                tmp_instr_name = '"' + instr.upper().replace(".", "_") + '"'
-                chisel_names += f'   {tmp_instr_name:<18s} -> BitPat("b{instr_dict[instr]["encoding"].replace("-","?")}"),\n'
-            chisel_names += f"  )\n"
+            for instr_name, instr in instr_dict.items():
+                if instr["extension"][0] == e:
+                    tmp_instr_name = '"' + instr_name.upper().replace(".", "_") + '"'
+                    chisel_names += f'   {tmp_instr_name:<18s} -> BitPat("b{instr["encoding"].replace("-","?")}"),\n'
+            chisel_names += "  )\n"
 
     for num, name in causes:
         cause_names_str += f'  val {name.lower().replace(" ","_")} = {hex(num)}\n'
@@ -65,12 +63,11 @@ def make_chisel(instr_dict: InstrDict, spinal_hdl: bool = False):
     csr_names_str += """    res.toArray
   }"""
 
-    if spinal_hdl:
-        chisel_file = open("inst.spinalhdl", "w")
-    else:
-        chisel_file = open("inst.chisel", "w")
-    chisel_file.write(
-        f"""
+    with open(
+        "inst.spinalhdl" if spinal_hdl else "inst.chisel", "w", encoding="utf-8"
+    ) as chisel_file:
+        chisel_file.write(
+            f"""
 /* Automatically generated by parse_opcodes */
 object Instructions {{
 {chisel_names}
@@ -82,5 +79,4 @@ def make_chisel(instr_dict: InstrDict, spinal_hdl: bool = False):
 {csr_names_str}
 }}
 """
-    )
-    chisel_file.close()
+        )

constants.py

@@ -44,7 +44,7 @@
 )
 
 
-def read_csv(filename: str):
+def read_int_map_csv(filename: str) -> "list[tuple[int, str]]":
     """
     Reads a CSV file and returns a list of tuples.
     Each tuple contains an integer value (from the first column) and a string (from the second column).
@@ -55,20 +55,26 @@ def read_csv(filename: str):
     Returns:
         list of tuple: A list of (int, str) tuples extracted from the CSV file.
     """
-    with open(filename) as f:
+    with open(filename, encoding="utf-8") as f:
         csv_reader = csv.reader(f, skipinitialspace=True)
         return [(int(row[0], 0), row[1]) for row in csv_reader]
 
 
-causes = read_csv("causes.csv")
-csrs = read_csv("csrs.csv")
-csrs32 = read_csv("csrs32.csv")
+causes = read_int_map_csv("causes.csv")
+csrs = read_int_map_csv("csrs.csv")
+csrs32 = read_int_map_csv("csrs32.csv")
 
-# Load the argument lookup table (arg_lut) from a CSV file, mapping argument names to their bit positions
-arg_lut = {
-    row[0]: (int(row[1]), int(row[2]))
-    for row in csv.reader(open("arg_lut.csv"), skipinitialspace=True)
-}
+
+def read_arg_lut_csv(filename: str) -> "dict[str, tuple[int, int]]":
+    """
+    Load the argument lookup table (arg_lut) from a CSV file, mapping argument names to their bit positions.
+    """
+    with open(filename, encoding="utf-8") as f:
+        csv_reader = csv.reader(f, skipinitialspace=True)
+        return {row[0]: (int(row[1]), int(row[2])) for row in csv_reader}
+
+
+arg_lut = read_arg_lut_csv("arg_lut.csv")
 
 # for mop
 arg_lut["mop_r_t_30"] = (30, 30)

go_utils.py

@@ -1,8 +1,9 @@
 import logging
 import pprint
+import subprocess
 import sys
 
-from shared_utils import *
+from shared_utils import InstrDict, signed
 
 pp = pprint.PrettyPrinter(indent=2)
 logging.basicConfig(level=logging.INFO, format="%(levelname)s:: %(message)s")
@@ -49,14 +50,12 @@ def make_go(instr_dict: InstrDict):
     return &inst{{ {hex(opcode)}, {hex(funct3)}, {hex(rs1)}, {hex(rs2)}, {signed(csr,12)}, {hex(funct7)} }}
 """
 
-    with open("inst.go", "w") as file:
+    with open("inst.go", "w", encoding="utf-8") as file:
         file.write(prelude)
         file.write(instr_str)
         file.write(endoffile)
 
     try:
-        import subprocess
-
-        subprocess.run(["go", "fmt", "inst.go"])
-    except:
+        subprocess.run(["go", "fmt", "inst.go"], check=True)
+    except:  # pylint: disable=bare-except
         pass