Improves diagnostic errors

Improves code coverage of diagnostics

Author: gnikit

fortls/objects.py

@@ -29,7 +29,7 @@
 from fortls.ftypes import INCLUDE_info, USE_info
 from fortls.helper_functions import get_keywords, get_paren_substring, get_var_stack
 from fortls.jsonrpc import path_to_uri
-from fortls.json_templates import range_json, diagnostic_json
+from fortls.json_templates import range_json, diagnostic_json, location_json
 
 
 def get_use_tree(
@@ -325,10 +325,9 @@ def build(self, file_obj):
         if self.has_related:
             diag["relatedInformation"] = [
                 {
-                    "location": {
-                        "uri": path_to_uri(self.related_path),
-                        **range_json(self.related_line, 0, self.related_line, 0),
-                    },
+                    **location_json(
+                        path_to_uri(self.related_path), self.related_line, 0
+                    ),
                     "message": self.related_message,
                 }
             ]
@@ -1975,6 +1974,26 @@ def add_doc(self, doc_string: str, forward: bool = False):
             if self.last_obj is not None:
                 self.last_obj.add_doc(doc_string)
 
+    def add_error(self, msg: str, sev: int, ln: int, sch: int, ech: int = None):
+        """Add a Diagnostic error, encountered during parsing, for a range
+        in the document.
+
+        Parameters
+        ----------
+        msg : str
+            Error message
+        sev : int
+            Severity, Error, Warning, Notification
+        ln : int
+            Line number
+        sch : int
+            Start character
+        ech : int
+            End character
+        """
+        # Convert from Editor line numbers 1-base index to LSP index which is 0-based
+        self.parse_errors.append(diagnostic_json(ln - 1, sch, ln - 1, ech, msg, sev))
+
     def start_ppif(self, line_number: int):
         self.pp_if.append([line_number - 1, -1])
 

fortls/parse_fortran.py

@@ -22,6 +22,7 @@
     SELECT_TYPE_ID,
     SUBMODULE_TYPE_ID,
     FRegex,
+    Severity,
     log,
 )
 from fortls.ftypes import (
@@ -69,7 +70,6 @@
     fortran_var,
     fortran_where,
 )
-from fortls.json_templates import diagnostic_json
 
 
 def get_line_context(line: str) -> tuple[str, None] | tuple[str, str]:
@@ -1156,8 +1156,8 @@ def preprocess(
     def check_file(self, obj_tree, max_line_length=-1, max_comment_line_length=-1):
         diagnostics = []
         if (max_line_length > 0) or (max_comment_line_length > 0):
-            line_message = f'Line length exceeds "max_line_length" ({max_line_length})'
-            comment_message = (
+            msg_line = f'Line length exceeds "max_line_length" ({max_line_length})'
+            msg_comment = (
                 'Comment line length exceeds "max_comment_line_length"'
                 f" ({max_comment_line_length})"
             )
@@ -1169,22 +1169,17 @@ def check_file(self, obj_tree, max_line_length=-1, max_comment_line_length=-1):
             for (i, line) in enumerate(self.contents_split):
                 if COMMENT_LINE_MATCH.match(line) is None:
                     if 0 < max_line_length < len(line):
-                        diagnostics.append(
-                            diagnostic_json(
-                                i, max_line_length, i, len(line), line_message, 2
-                            )
+                        self.ast.add_error(
+                            msg_line, Severity.warn, i + 1, max_line_length, len(line)
                         )
                 else:
                     if 0 < max_comment_line_length < len(line):
-                        diagnostics.append(
-                            diagnostic_json(
-                                i,
-                                max_comment_line_length,
-                                i,
-                                len(line),
-                                comment_message,
-                                2,
-                            )
+                        self.ast.add_error(
+                            msg_comment,
+                            Severity.warn,
+                            i + 1,
+                            max_comment_line_length,
+                            len(line),
                         )
         errors, diags_ast = self.ast.check_file(obj_tree)
         diagnostics += diags_ast
@@ -1646,16 +1641,8 @@ def parse(
 
             elif obj_type == "vis":
                 if file_ast.current_scope is None:
-                    file_ast.parse_errors.append(
-                        diagnostic_json(
-                            line_number,
-                            0,
-                            line_number,
-                            0,
-                            "Visibility statement without enclosing scope",
-                            1,
-                        )
-                    )
+                    msg = "Visibility statement without enclosing scope"
+                    file_ast.add_error(msg, Severity.error, line_number, 0)
                 else:
                     if (len(obj_info.obj_names) == 0) and (obj_info.type == 1):
                         file_ast.current_scope.set_default_vis(-1)
@@ -1738,37 +1725,32 @@ def _parse_implicit(self, line: str, ln: int, file_ast: fortran_ast):
         match = FRegex.IMPLICIT.match(line)
         if match is None:
             return False
-        err_message = None
         if file_ast.current_scope is None:
-            err_message = "IMPLICIT statement without enclosing scope"
+            msg = "IMPLICIT statement without enclosing scope"
+            file_ast.add_error(msg, Severity.error, ln, match.start(1), match.end(1))
         else:
             if match.group(1).lower() == "none":
                 file_ast.current_scope.set_implicit(False, ln)
             else:
                 file_ast.current_scope.set_implicit(True, ln)
-        if err_message:
-            file_ast.parse_errors.append(
-                diagnostic_json(ln, match.start(1), ln, match.end(1), err_message, 1)
-            )
+
         self.parser_debug("IMPLICIT", self.line, ln)
         return True
 
     def _parse_contains(self, line: str, ln: int, file_ast: fortran_ast):
         match = FRegex.CONTAINS.match(line)
         if match is None:
             return False
-        err_message: str = None
+        msg: str = None
         try:
             if file_ast.current_scope is None:
-                err_message = "CONTAINS statement without enclosing scope"
+                msg = "CONTAINS statement without enclosing scope"
             else:
                 file_ast.current_scope.mark_contains(ln)
         except ValueError:
-            err_message = "Multiple CONTAINS statements in scope"
-        if err_message:
-            file_ast.parse_errors.append(
-                diagnostic_json(ln, match.start(1), ln, match.end(1), err_message, 1)
-            )
+            msg = "Multiple CONTAINS statements in scope"
+        if msg:
+            file_ast.add_error(msg, Severity.error, ln, match.start(1), match.end(1))
         self.parser_debug("CONTAINS", self.line, ln)
         return True
 

test/test_server.py

@@ -733,182 +733,3 @@ def hover_request(file_path, line, char):
     check_return(results[8], ())
     check_return(results[9], ())
     check_return(results[10], ((3, " !! Doc 9"), (4, " !! Doc 10")))
-
-
-def test_diagnostics():
-    """
-    Tests some aspects of diagnostics
-    """
-
-    def check_return(results, ref_results):
-        for i, r in enumerate(results):
-            print(r["diagnostics"], ref_results[i])
-            assert r["diagnostics"] == ref_results[i]
-
-    string = write_rpc_request(1, "initialize", {"rootPath": str(test_dir)})
-    # Test subroutines and functions with interfaces as arguments
-    file_path = str(test_dir / "test_diagnostic_int.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    # Test that  use, non_intrinsic does not raise a diagnostic error
-    file_path = str(test_dir / "test_nonintrinsic.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    # Test that submodules with spacings in their parent's names are parsed
-    file_path = str(test_dir / "test_submodule.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    # Tests that variables named end do not close the scope prematurely
-    file_path = str(test_dir / "diag" / "test_scope_end_name_var.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    # Test that externals can be split between multiple lines
-    # and that diagnostics for multiple definitions of externals can account
-    # for that
-    file_path = str(test_dir / "diag" / "test_external.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    # Checks that forall with end forall inside a case select does not cause
-    # unexpected end of scope.
-    file_path = str(test_dir / "diag" / "test_forall.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    # Test USE directive ordering errors
-    file_path = str(test_dir / "diag" / "test_use_ordering.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    # Test where blocks
-    file_path = str(test_dir / "diag" / "test_where.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    # Test where semicolon (multi-line)
-    file_path = str(test_dir / "diag" / "test_semicolon.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    # Test ENUM block
-    file_path = str(test_dir / "diag" / "test_enum.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    # Test module procedure in submodules importing scopes
-    file_path = str(test_dir / "subdir" / "test_submod.F90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    errcode, results = run_request(string)
-    assert errcode == 0
-
-    # Load a different config file
-    # Test long lines
-    root = str(test_dir / "diag")
-    string = write_rpc_request(1, "initialize", {"rootPath": root})
-    file_path = str(test_dir / "diag" / "test_lines.f90")
-    string += write_rpc_notification(
-        "textDocument/didOpen", {"textDocument": {"uri": file_path}}
-    )
-    file_path = str(test_dir / "diag" / "conf_long_lines.json")
-    errcode, res = run_request(string, [f"--config {file_path}"])
-    assert errcode == 0
-    results.extend(res[1:])
-
-    root = path_to_uri(str((test_dir / "diag" / "test_external.f90").resolve()))
-    ref_results = [
-        [],
-        [],
-        [],
-        [],
-        [
-            {
-                "range": {
-                    "start": {"line": 7, "character": 17},
-                    "end": {"line": 7, "character": 22},
-                },
-                "message": 'Variable "VAR_B" declared twice in scope',
-                "severity": 1,
-                "relatedInformation": [
-                    {
-                        "location": {
-                            "uri": str(root),
-                            "range": {
-                                "start": {"line": 5, "character": 0},
-                                "end": {"line": 5, "character": 0},
-                            },
-                        },
-                        "message": "First declaration",
-                    }
-                ],
-            },
-            {
-                "range": {
-                    "start": {"line": 8, "character": 17},
-                    "end": {"line": 8, "character": 22},
-                },
-                "message": 'Variable "VAR_A" declared twice in scope',
-                "severity": 1,
-                "relatedInformation": [
-                    {
-                        "location": {
-                            "uri": str(root),
-                            "range": {
-                                "start": {"line": 3, "character": 0},
-                                "end": {"line": 3, "character": 0},
-                            },
-                        },
-                        "message": "First declaration",
-                    }
-                ],
-            },
-        ],
-        [],
-        [],
-        [],
-        [],
-        [],
-        [],
-        [
-            {
-                "range": {
-                    "start": {"line": 2, "character": 100},
-                    "end": {"line": 2, "character": 155},
-                },
-                "message": 'Line length exceeds "max_line_length" (100)',
-                "severity": 2,
-            },
-            {
-                "range": {
-                    "start": {"line": 3, "character": 100},
-                    "end": {"line": 3, "character": 127},
-                },
-                "message": (
-                    'Comment line length exceeds "max_comment_line_length" (100)'
-                ),
-                "severity": 2,
-            },
-        ],
-    ]
-    check_return(results[1:], ref_results)
-
-
-if __name__ == "__main__":
-    test_init()
-    test_logger()
-    test_open()
-    test_change()
-    test_symbols()
-    test_workspace_symbols()
-    test_comp()
-    test_sig()
-    test_def()
-    test_refs()
-    test_hover()
-    test_docs()
-    test_diagnostics()