(Closes #3038) catch parse error in frontend and test

changelog

@@ -1,3 +1,6 @@
+   61) PR #3040 for #3038. Improves the frontend error reporting of the
+   psyclone command.
+
    60) PR #2967 towards #2551. Adds a barrier minimisation step on the Async
    workflow to improve its performance and adds integration tests.
 

src/psyclone/generator.py

@@ -235,6 +235,8 @@ def generate(filename, api="", kernel_paths=None, script_name=None,
     >>> alg, psy = generate("algspec.f90", distributed_memory=False)
 
     '''
+    logger = logging.getLogger(__name__)
+
     if kernel_paths is None:
         kernel_paths = []
 
@@ -296,7 +298,13 @@ def generate(filename, api="", kernel_paths=None, script_name=None,
             # Check that there is only one module/program per file.
             check_psyir(psyir, filename)
         else:
-            psyir = reader.psyir_from_file(filename)
+            try:
+                psyir = reader.psyir_from_file(filename)
+            except (InternalError, ValueError, IOError) as err:
+                print(f"Failed to create PSyIR from file '{filename}'",
+                      file=sys.stderr)
+                logger.error(err, exc_info=True)
+                sys.exit(1)
 
         # Raise to Algorithm PSyIR
         if api in GOCEAN_API_NAMES:
@@ -359,9 +367,10 @@ def generate(filename, api="", kernel_paths=None, script_name=None,
                 try:
                     # Create language-level PSyIR from the kernel file
                     kernel_psyir = reader.psyir_from_file(filepath)
-                except InternalError as info:
-                    print(f"In kernel file '{filepath}':\n{str(info.value)}",
-                          file=sys.stderr)
+                except (InternalError, ValueError, IOError) as info:
+                    print(f"Failed to create PSyIR from kernel file "
+                          f"'{filepath}'", file=sys.stderr)
+                    logger.error(info, exc_info=True)
                     sys.exit(1)
 
                 # Raise to Kernel PSyIR
@@ -542,7 +551,7 @@ def main(arguments):
     else:
         logging.basicConfig(level=loglevel)
     logger = logging.getLogger(__name__)
-    logger.debug("Logging system initialised.")
+    logger.debug("Logging system initialised. Level is %s.", args.log_level)
 
     # Validate that the given arguments are for the right operation mode
     if not args.psykal_dsl:
@@ -778,6 +787,8 @@ def code_transformation_mode(input_file, recipe_file, output_file,
         of 123 chars, and to "all", to additionally check the input code.
 
     '''
+    logger = logging.getLogger(__name__)
+
     # Load recipe file
     if recipe_file:
         trans_recipe, files_to_skip, resolve_mods = load_script(recipe_file)
@@ -799,10 +810,16 @@ def code_transformation_mode(input_file, recipe_file, output_file,
                     sys.exit(1)
 
         # Parse file
-        psyir = FortranReader(resolve_modules=resolve_mods,
-                              ignore_comments=not keep_comments,
-                              ignore_directives=not keep_directives)\
-            .psyir_from_file(input_file)
+        reader = FortranReader(resolve_modules=resolve_mods,
+                               ignore_comments=not keep_comments,
+                               ignore_directives=not keep_directives)
+        try:
+            psyir = reader.psyir_from_file(input_file)
+        except (InternalError, ValueError, IOError) as err:
+            print(f"Failed to create PSyIR from file '{input_file}' due "
+                  f"to:\n{str(err)}", file=sys.stderr)
+            logger.error(err, exc_info=True)
+            sys.exit(1)
 
         # Modify file
         if trans_recipe:

src/psyclone/parse/kernel.py

@@ -43,7 +43,6 @@
 '''
 
 import os
-import sys
 
 from pyparsing import ParseException
 
@@ -143,26 +142,21 @@ def get_kernel_filepath(module_name, kernel_paths, alg_filename):
     return matches[0]
 
 
-def get_kernel_parse_tree(filepath):
+def get_kernel_parse_tree(filepath: str):
     '''Parse the file in filepath with fparser1 and return a parse tree.
 
-    :param str filepath: path to a file (hopefully) containing \
-    PSyclone kernel code.
+    :param filepath: path to a file (hopefully) containing PSyclone
+                     kernel code.
 
-    :returns: Parse tree of the kernel code contained in the specified \
-    file.
+    :returns: Parse tree of the kernel code contained in the specified
+              file.
     :rtype: :py:class:`fparser.one.block_statements.BeginSource`
 
     :raises ParseError: if fparser fails to parse the file
 
     '''
     parsefortran.FortranParser.cache.clear()
 
-    # If logging is disable during a sphinx doctest run, doctest will just
-    # stop working. So only disable logging if we are not running doctest.
-    if 'sphinx.ext.doctest' not in sys.modules:
-        fparser.logging.disable(fparser.logging.CRITICAL)
-
     try:
         parse_tree = fpapi.parse(filepath)
         # parse_tree includes an extra comment line which contains

src/psyclone/psyir/frontend/fortran.py

@@ -45,10 +45,10 @@
 from fparser.two import Fortran2003, pattern_tools
 from fparser.two.parser import ParserFactory
 from fparser.two.symbol_table import SYMBOL_TABLES
-from fparser.two.utils import NoMatchError
+from fparser.two.utils import FortranSyntaxError, NoMatchError
 from psyclone.configuration import Config
 from psyclone.psyir.frontend.fparser2 import Fparser2Reader
-from psyclone.psyir.nodes import Schedule, Assignment, Routine
+from psyclone.psyir.nodes import Assignment, Node, Routine, Schedule
 from psyclone.psyir.symbols import SymbolTable
 
 
@@ -119,13 +119,14 @@ def validate_name(name: str):
             raise ValueError(
                 f"Invalid Fortran name '{name}' found.")
 
-    def psyir_from_source(self, source_code: str):
-        ''' Generate the PSyIR tree representing the given Fortran source code.
+    def psyir_from_source(self, source_code: str) -> Node:
+        ''' Generate the PSyIR tree for the given Fortran source code.
 
-        :param str source_code: text representation of the code to be parsed.
+        :param source_code: text representation of the code to be parsed.
 
-        :returns: PSyIR representing the provided Fortran source code.
-        :rtype: :py:class:`psyclone.psyir.nodes.Node`
+        :returns: the PSyIR of the provided Fortran source code.
+
+        :raises ValueError: if the supplied Fortran cannot be parsed.
 
         '''
         SYMBOL_TABLES.clear()
@@ -134,7 +135,14 @@ def psyir_from_source(self, source_code: str):
             ignore_comments=self._ignore_comments)
         # Set reader to free format.
         string_reader.set_format(FortranFormat(self._free_form, False))
-        parse_tree = self._parser(string_reader)
+
+        try:
+            parse_tree = self._parser(string_reader)
+        except (FortranSyntaxError, NoMatchError) as err:
+            raise ValueError(
+                f"Failed to parse the provided source code:\n{source_code}\n"
+                f"Error was: {err}\nIs the input valid Fortran (note that CPP "
+                f"directives must be handled by a pre-processor)?") from err
 
         psyir = self._processor.generate_psyir(parse_tree)
         return psyir
@@ -238,6 +246,8 @@ def psyir_from_file(self, file_path):
         :returns: PSyIR representing the provided Fortran file.
         :rtype: :py:class:`psyclone.psyir.nodes.Node`
 
+        :raises ValueError: if the parser fails to parse the contents of
+                            the supplied file.
         '''
         SYMBOL_TABLES.clear()
 
@@ -252,7 +262,13 @@ def psyir_from_file(self, file_path):
                                    include_dirs=Config.get().include_paths,
                                    ignore_comments=self._ignore_comments)
         reader.set_format(FortranFormat(self._free_form, False))
-        parse_tree = self._parser(reader)
+        try:
+            parse_tree = self._parser(reader)
+        except (FortranSyntaxError, NoMatchError) as err:
+            raise ValueError(
+                f"Failed to parse source in file '{file_path}'.\n"
+                f"Error was: {err}\nIs the input valid Fortran (note that CPP "
+                f"directives must be handled by a pre-processor)?") from err
         _, filename = os.path.split(file_path)
 
         psyir = self._processor.generate_psyir(parse_tree, filename)

src/psyclone/tests/domain/lfric/lfric_constants_test.py

@@ -60,7 +60,9 @@ def test_config_loaded_before_constants_created(monkeypatch):
     # If the psyclone command is executed, the flag should be set. The
     # parameters specified here will immediately abort, but still the
     # flag must be set at the end, since the command has to set this flag:
-    with pytest.raises(FileNotFoundError) as err:
+    # (We check for two different exceptions as this behaviour seems to
+    # change between Python 3.9 and more recent versions.)
+    with pytest.raises((FileNotFoundError, SystemExit)) as err:
         generator.main(["some_file.f90"])
     assert Config.has_config_been_initialised() is True
 

src/psyclone/tests/generator_test.py

@@ -63,7 +63,7 @@
 from psyclone.domain.lfric.transformations import LFRicLoopFuseTrans
 from psyclone.errors import GenerationError
 from psyclone.generator import (
-    generate, main, check_psyir, add_builtins_use)
+    generate, main, check_psyir, add_builtins_use, code_transformation_mode)
 from psyclone.parse import ModuleManager
 from psyclone.parse.algorithm import parse
 from psyclone.parse.utils import ParseError
@@ -379,7 +379,7 @@ def test_recurse_correct_kernel_paths():
         kernel_paths=[os.path.join(BASE_PATH, "lfric", "kernels")])
 
 
-def test_kernel_parsing_internalerror(capsys):
+def test_kernel_parsing_internalerror(capsys, caplog):
     '''Checks that the expected output is provided if an internal error is
     caught when parsing a kernel using fparser2.
 
@@ -390,32 +390,41 @@ def test_kernel_parsing_internalerror(capsys):
         main([kern_filename, "-api", "gocean"])
     out, err = capsys.readouterr()
     assert out == ""
-    assert "In kernel file " in str(err)
-    assert (
-        "PSyclone internal error: The argument list ['i', 'j', 'cu', 'p', "
-        "'u'] for routine 'compute_code' does not match the variable "
-        "declarations:\n"
-        "IMPLICIT NONE\n"
-        "INTEGER, INTENT(IN) :: I, J\n"
-        "REAL(KIND = go_wp), INTENT(OUT), DIMENSION(:, :) :: cu\n"
-        "REAL(KIND = go_wp), INTENT(IN), DIMENSION(:, :) :: p\n"
-        "(Note that PSyclone does not support implicit declarations.) Specific"
-        " PSyIR error is \"Could not find 'u' in the Symbol Table.\".\n"
-        in str(err))
+    assert "Failed to create PSyIR from kernel file '" in str(err)
+    # Clear previous logging messages (primarily from fparser)
+    caplog.clear()
+    with caplog.at_level(logging.ERROR, "psyclone.generator"):
+        with pytest.raises(SystemExit):
+            main([kern_filename, "-api", "gocean"])
+        assert caplog.records[0].levelname == "ERROR"
+        assert (
+            "PSyclone internal error: The argument list ['i', 'j', 'cu', 'p', "
+            "'u'] for routine 'compute_code' does not match the variable "
+            "declarations:\n"
+            "IMPLICIT NONE\n"
+            "INTEGER, INTENT(IN) :: I, J\n"
+            "REAL(KIND = go_wp), INTENT(OUT), DIMENSION(:, :) :: cu\n"
+            "REAL(KIND = go_wp), INTENT(IN), DIMENSION(:, :) :: p\n"
+            "(Note that PSyclone does not support implicit declarations.) "
+            "Specific"
+            " PSyIR error is \"Could not find 'u' in the Symbol Table.\".\n"
+            in caplog.text)
 
 
 def test_script_file_too_short():
     '''Checks that generator.py raises an appropriate error when a script
     file name is too short to contain the '.py' extension.
 
     '''
-    with pytest.raises(GenerationError):
+    with pytest.raises(GenerationError) as err:
         _, _ = generate(os.path.join(BASE_PATH, "lfric",
                                      "1_single_invoke.f90"),
                         api="lfric",
                         script_name=os.path.join(
                             BASE_PATH,
                             "lfric", "testkern_xyz_mod.f90"))
+    assert ("expected the script file 'testkern_xyz_mod.f90' to have the "
+            "'.py' extension" in str(err.value))
 
 
 def test_no_script_gocean():
@@ -463,6 +472,29 @@ def test_profile_gocean():
     Profiler._options = []
 
 
+def test_invalid_gocean_alg(monkeypatch, caplog, capsys):
+    '''
+    Test that an error creating PSyIR for a GOcean algorithm layer is
+    handled correctly.
+
+    '''
+    # It's easiest to monkeypatch the psyir_from_file() method so that it
+    # raises an error.
+    def _broken(_1, _2):
+        raise ValueError("This is a test")
+
+    monkeypatch.setattr(FortranReader, "psyir_from_file", _broken)
+    with caplog.at_level(logging.ERROR):
+        with pytest.raises(SystemExit):
+            _ = generate(
+                os.path.join(BASE_PATH, "gocean1p0", "single_invoke.f90"),
+                api="gocean")
+        assert "This is a test" in caplog.text
+        assert "Traceback" in caplog.text
+        _, err = capsys.readouterr()
+        assert "Failed to create PSyIR from file '" in err
+
+
 def test_script_attr_error(script_factory):
     '''Checks that generator.py raises an appropriate error when a script
     file contains a trans() function which raises an attribute error.
@@ -804,7 +836,8 @@ def test_main_api(capsys, caplog):
               "--log-file", "test.out"])
         assert Config.get().api == "lfric"
         assert caplog.records[0].levelname == "DEBUG"
-        assert "Logging system initialised" in caplog.record_tuples[0][2]
+        assert ("Logging system initialised. Level is DEBUG." in
+                caplog.record_tuples[0][2])
 
 
 def test_keep_comments_and_keep_directives(capsys, caplog, tmpdir_factory,
@@ -1128,6 +1161,28 @@ def trans(psyir):
     assert "module newname\n" in new_code
 
 
+def test_code_transformation_parse_failure(tmpdir, caplog, capsys):
+    '''
+    Test the error handling in the code_transformation_mode() method when
+    there is invalid Fortran in the supplied file.
+
+    '''
+    code = '''
+    prog invalid
+      ! This is not valid Fortran
+    end prog invalid
+    '''
+    inputfile = str(tmpdir.join("funny_syntax.f90"))
+    with open(inputfile, "w", encoding='utf-8') as my_file:
+        my_file.write(code)
+    with caplog.at_level(logging.ERROR):
+        with pytest.raises(SystemExit):
+            code_transformation_mode(inputfile, None, None, False, False)
+        out, err = capsys.readouterr()
+        assert "Failed to create PSyIR from file '" in err
+        assert "Is the input valid Fortran" in caplog.text
+
+
 def test_generate_trans_error(tmpdir, capsys, monkeypatch):
     '''Test that a TransformationError exception in the generate function
     is caught and output as expected by the main function.  The