refactor: adds custom exceptions

This is a first attempt at the problem. Slowly I will have to remove
the levels of nested handling of certain errors and allow them
to bubble up.

Author: gnikit

fortls/debug.py

@@ -8,7 +8,7 @@
 from .helper_functions import only_dirs, resolve_globs
 from .jsonrpc import JSONRPC2Connection, ReadWriter, path_from_uri
 from .langserver import LangServer
-from .parsers.internal.parser import FortranFile
+from .parsers.internal.parser import FortranFile, ParserError
 
 
 class DebugError(Exception):
@@ -460,14 +460,14 @@ def read_config(root: str | None):
     print("\nTesting parser")
     separator()
 
-    ensure_file_accessible(args.debug_filepath)
     pp_suffixes, pp_defs, include_dirs = read_config(args.debug_rootpath)
-
-    print(f'  File = "{args.debug_filepath}"')
     file_obj = FortranFile(args.debug_filepath, pp_suffixes)
-    err_str, _ = file_obj.load_from_disk()
-    if err_str:
-        raise DebugError(f"Reading file failed: {err_str}")
+    try:
+        file_obj.load_from_disk()
+    except ParserError as exc:
+        msg = f"Reading file {args.debug_filepath} failed: {str(exc)}"
+        raise DebugError(msg) from exc
+    print(f'  File = "{args.debug_filepath}"')
     print(f"  Detected format: {'fixed' if file_obj.fixed else 'free'}")
     print("\n" + "=" * 80 + "\nParser Output\n" + "=" * 80 + "\n")
     file_ast = file_obj.parse(debug=True, pp_defs=pp_defs, include_dirs=include_dirs)

fortls/langserver.py

@@ -51,7 +51,7 @@
     load_intrinsics,
     set_lowercase_intrinsics,
 )
-from fortls.parsers.internal.parser import FortranFile, get_line_context
+from fortls.parsers.internal.parser import FortranFile, ParserError, get_line_context
 from fortls.parsers.internal.scope import Scope
 from fortls.parsers.internal.use import Use
 from fortls.parsers.internal.utilities import (
@@ -1313,9 +1313,10 @@ def serve_onChange(self, request: dict):
                     return
         # Parse newly updated file
         if reparse_req:
-            _, err_str = self.update_workspace_file(path, update_links=True)
-            if err_str is not None:
-                self.post_message(f"Change request failed for file '{path}': {err_str}")
+            try:
+                self.update_workspace_file(path, update_links=True)
+            except LSPError as e:
+                self.post_message(f"Change request failed for file '{path}': {str(e)}")
                 return
             # Update include statements linking to this file
             for _, tmp_file in self.workspace.items():
@@ -1350,11 +1351,12 @@ def serve_onSave(
                     for key in ast_old.global_dict:
                         self.obj_tree.pop(key, None)
             return
-        did_change, err_str = self.update_workspace_file(
-            filepath, read_file=True, allow_empty=did_open
-        )
-        if err_str is not None:
-            self.post_message(f"Save request failed for file '{filepath}': {err_str}")
+        try:
+            did_change = self.update_workspace_file(
+                filepath, read_file=True, allow_empty=did_open
+            )
+        except LSPError as e:
+            self.post_message(f"Save request failed for file '{filepath}': {str(e)}")
             return
         if did_change:
             # Update include statements linking to this file
@@ -1390,12 +1392,14 @@ def update_workspace_file(
                             return False, None
                         else:
                             return False, "File does not exist"  # Error during load
-                err_string, file_changed = file_obj.load_from_disk()
-                if err_string:
-                    log.error("%s : %s", err_string, filepath)
-                    return False, err_string  # Error during file read
-                if not file_changed:
-                    return False, None
+                try:
+                    file_changed = file_obj.load_from_disk()
+                    if not file_changed:
+                        return False, None
+                except ParserError as exc:
+                    log.error("%s : %s", str(exc), filepath)
+                    raise LSPError from exc
+
             ast_new = file_obj.parse(
                 pp_defs=self.pp_defs, include_dirs=self.include_dirs
             )
@@ -1452,9 +1456,11 @@ def file_init(
             A Fortran file object or a string containing the error message
         """
         file_obj = FortranFile(filepath, pp_suffixes)
-        err_str, _ = file_obj.load_from_disk()
-        if err_str:
-            return err_str
+        # TODO: allow to bubble up the error message
+        try:
+            file_obj.load_from_disk()
+        except ParserError as e:
+            return str(e)
         try:
             # On Windows multiprocess does not propagate global variables in a shell.
             # Windows uses 'spawn' while Unix uses 'fork' which propagates globals.
@@ -1844,6 +1850,10 @@ def update_recursion_limit(limit: int) -> None:
         sys.setrecursionlimit(limit)
 
 
+class LSPError(Exception):
+    """Base class for Language Server Protocol errors"""
+
+
 class JSONRPC2Error(Exception):
     def __init__(self, code, message, data=None):
         self.code = code

fortls/parsers/internal/parser.py

@@ -871,41 +871,44 @@ def copy(self) -> FortranFile:
         copy_obj.set_contents(self.contents_split)
         return copy_obj
 
-    def load_from_disk(self) -> tuple[str | None, bool | None]:
+    def load_from_disk(self) -> bool:
         """Read file from disk or update file contents only if they have changed
         A MD5 hash is used to determine that
 
         Returns
         -------
-        tuple[str|None, bool|None]
-            ``str`` : string containing IO error message else None
-            ``bool``: boolean indicating if the file has changed
+        bool
+            boolean indicating if the file has changed
+
+        Raises
+        ------
+        FileReadDecodeError
+            If the file could not be read or decoded
         """
         contents: str
         try:
             with open(self.path, encoding="utf-8", errors="replace") as f:
                 contents = re.sub(r"\t", r" ", f.read())
-        except OSError:
-            return "Could not read/decode file", None
-        else:
-            # Check if files are the same
-            try:
-                hash = hashlib.md5(
-                    contents.encode("utf-8"), usedforsecurity=False
-                ).hexdigest()
-            # Python <=3.8 does not have the `usedforsecurity` option
-            except TypeError:
-                hash = hashlib.md5(contents.encode("utf-8")).hexdigest()
-
-            if hash == self.hash:
-                return None, False
-
-            self.hash = hash
-            self.contents_split = contents.splitlines()
-            self.fixed = detect_fixed_format(self.contents_split)
-            self.contents_pp = self.contents_split
-            self.nLines = len(self.contents_split)
-            return None, True
+        except OSError as exc:
+            raise FileReadDecodeError("Could not read/decode file") from exc
+        # Check if files are the same
+        try:
+            hash = hashlib.md5(
+                contents.encode("utf-8"), usedforsecurity=False
+            ).hexdigest()
+        # Python <=3.8 does not have the `usedforsecurity` option
+        except TypeError:
+            hash = hashlib.md5(contents.encode("utf-8")).hexdigest()
+
+        if hash == self.hash:
+            return False
+
+        self.hash = hash
+        self.contents_split = contents.splitlines()
+        self.fixed = detect_fixed_format(self.contents_split)
+        self.contents_pp = self.contents_split
+        self.nLines = len(self.contents_split)
+        return True
 
     def apply_change(self, change: dict) -> bool:
         """Apply a change to the file."""
@@ -2261,24 +2264,18 @@ def append_multiline_macro(def_value: str | tuple, line: str):
             if include_path is not None:
                 try:
                     include_file = FortranFile(include_path)
-                    err_string, _ = include_file.load_from_disk()
-                    if err_string is None:
-                        log.debug("\n!!! Parsing include file '%s'", include_path)
-                        _, _, _, defs_tmp = preprocess_file(
-                            include_file.contents_split,
-                            file_path=include_path,
-                            pp_defs=defs_tmp,
-                            include_dirs=include_dirs,
-                            debug=debug,
-                        )
-                        log.debug("!!! Completed parsing include file\n")
-
-                    else:
-                        log.debug("!!! Failed to parse include file: %s", err_string)
-
-                except:
-                    log.debug("!!! Failed to parse include file: exception")
-
+                    include_file.load_from_disk()
+                    log.debug("\n!!! Parsing include file '%s'", include_path)
+                    _, _, _, defs_tmp = preprocess_file(
+                        include_file.contents_split,
+                        file_path=include_path,
+                        pp_defs=defs_tmp,
+                        include_dirs=include_dirs,
+                        debug=debug,
+                    )
+                    log.debug("!!! Completed parsing include file")
+                except ParserError as e:
+                    log.debug("!!! Failed to parse include file: %s", str(e))
             else:
                 log.debug(
                     "%s !!! Could not locate include file (%d)", line.strip(), i + 1
@@ -2313,3 +2310,11 @@ def append_multiline_macro(def_value: str | tuple, line: str):
                 line = line_new
         output_file.append(line)
     return output_file, pp_skips, pp_defines, defs_tmp
+
+
+class ParserError(Exception):
+    """Parser base class exception"""
+
+
+class FileReadDecodeError(ParserError):
+    """File could not be read/decoded"""

test/test_parser.py

@@ -1,13 +1,13 @@
+import pytest
 from setup_tests import test_dir
 
-from fortls.parsers.internal.parser import FortranFile
+from fortls.parsers.internal.parser import FileReadDecodeError, FortranFile
 
 
 def test_line_continuations():
     file_path = test_dir / "parse" / "line_continuations.f90"
     file = FortranFile(str(file_path))
-    err_str, _ = file.load_from_disk()
-    assert err_str is None
+    file.load_from_disk()
     try:
         file.parse()
         assert True
@@ -19,8 +19,7 @@ def test_line_continuations():
 def test_submodule():
     file_path = test_dir / "parse" / "submodule.f90"
     file = FortranFile(str(file_path))
-    err_str, _ = file.load_from_disk()
-    assert err_str is None
+    file.load_from_disk()
     try:
         ast = file.parse()
         assert True
@@ -31,3 +30,9 @@ def test_submodule():
     except Exception as e:
         print(e)
         assert False
+
+
+def test_load_from_disk_exception():
+    file = FortranFile("/path/to/nonexistent/file.f90")
+    with pytest.raises(FileReadDecodeError):
+        file.load_from_disk()