Repository: stfc/fparser

Issue:
Support extension to permit in-line '!' comments in fixed-format Fortran
The OpenMPI Fortran include files are fixed format (they have the `-*- fortran -*-` header and non-comment lines begin with 6 spaces). However, they also have some 'inline' style comments, e.g.:

    ! MPI F08 conformance
    !
          logical MPI_SUBARRAYS_SUPPORTED
          logical MPI_ASYNC_PROTECTS_NONBLOCKING
          ! Hard-coded for .false. for now
          parameter (MPI_SUBARRAYS_SUPPORTED= .false.)

and fparser does not accept such comments giving rise to a syntax error. Given that this is in OpenMPI code this must mean that the vast majority of compilers have no problem, despite it being non-standards compliant F77. I'm therefore inclined to say that fparser should just silently permit such comments too.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 613d94f..f6c59c9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,9 @@ Modifications by (in alphabetical order):
 * A. Voysey, UK Met Office
 
 
+01/02/2023 PR #387 for #386. Support extension to permit in-line '!' 
+           comments in fixed-format Fortran.
+
 30/11/2022 PR #382 for #264. Ignore quotation marks in in-line comments.
 
 18/10/2022 PR #380 towards #379. Improve support for operators and symbol
diff --git a/doc/fparser.rst b/doc/fparser.rst
index ff091cd..f805166 100644
--- a/doc/fparser.rst
+++ b/doc/fparser.rst
@@ -1,7 +1,7 @@
 .. -*- rest -*-
 
 ..
-    Copyright (c) 2017-2018 Science and Technology Facilities Council.
+    Copyright (c) 2017-2023 Science and Technology Facilities Council.
 
     All rights reserved.
 
@@ -100,7 +100,7 @@ As indicated by the above output, the `fparser.api.parse()` function
 returns a `Statement` tree representation of the parsed source code.
 This `parse()` function is actually a convenience method that wraps
 the creation of a reader for Fortran source code (either
-FortranStringReader or FortranFileReader) followed by a call to use
+`FortranStringReader` or `FortranFileReader`) followed by a call to use
 that reader to create the tree, e.g.:
 
 ::
@@ -232,6 +232,14 @@ For example,
   >>> reader.next()
   Line('integer i,incx,incy,ix,iy,m,mp1,n',(9, 9),'')
 
+Owing to its origins in the f2py project, the reader contains functionality
+to identify the format of the provided source by examining Python-style
+encoding information (c.f. `PEP 263`). Since this is not a part of the Fortran
+standard, this functionality is disabled by default. It can be enabled by
+providing the argument `ignore_encoding=False` to the reader.
+
+.. _PEP 263: https://peps.python.org/pep-0263/
+
 Note that the `FortranReaderBase.next()` method may return `Line`,
 `SyntaxErrorLine`, `Comment`, `MultiLine`, or `SyntaxErrorMultiLine`
 instances.
diff --git a/src/fparser/common/readfortran.py b/src/fparser/common/readfortran.py
index 4bb7e97..8efc4c8 100644
--- a/src/fparser/common/readfortran.py
+++ b/src/fparser/common/readfortran.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
-# Modified work Copyright (c) 2017-2022 Science and Technology
+# Modified work Copyright (c) 2017-2023 Science and Technology
 # Facilities Council.
 # Original work Copyright (c) 1999-2008 Pearu Peterson
 
@@ -791,22 +791,13 @@ class FortranReaderBase:
             if self.reader is not None:
                 # inside INCLUDE statement
                 try:
-                    # Manually check to see if something has not
-                    # matched and has been placed in the fifo. We
-                    # can't use _next() as this method is associated
-                    # with the include reader (self.reader._next()),
-                    # not this reader (self._next()).
-                    return self.fifo_item.pop(0)
-                except IndexError:
-                    # There is nothing in the fifo buffer.
-                    try:
-                        # Return a line from the include.
-                        return self.reader.next(ignore_comments)
-                    except StopIteration:
-                        # There is nothing left in the include
-                        # file. Setting reader to None indicates that
-                        # we should now read from the main reader.
-                        self.reader = None
+                    # Return a line from the include.
+                    return self.reader.next(ignore_comments)
+                except StopIteration:
+                    # There is nothing left in the include
+                    # file. Setting reader to None indicates that
+                    # we should now read from the main reader.
+                    self.reader = None
             item = self._next(ignore_comments)
             if isinstance(item, Line) and _IS_INCLUDE_LINE(item.line):
                 # catch INCLUDE statement and create a new FortranReader
@@ -1555,9 +1546,12 @@ class FortranFileReader(FortranReaderBase):
 
     :param file_candidate: A filename or file-like object.
     :param list include_dirs: Directories in which to look for inclusions.
-    :param list source_only: Fortran source files to search for modules
+    :param list source_only: Fortran source files to search for modules \
                              required by "use" statements.
     :param bool ignore_comments: Whether or not to ignore comments
+    :param Optional[bool] ignore_encoding: whether or not to ignore Python-style \
+        encoding information (e.g. "-*- fortran -*-") when attempting to determine \
+        the format of the file. Default is True.
 
     For example:
 
@@ -1568,7 +1562,12 @@ class FortranFileReader(FortranReaderBase):
     """
 
     def __init__(
-        self, file_candidate, include_dirs=None, source_only=None, ignore_comments=True
+        self,
+        file_candidate,
+        include_dirs=None,
+        source_only=None,
+        ignore_comments=True,
+        ignore_encoding=True,
     ):
         # The filename is used as a unique ID. This is then used to cache the
         # contents of the file. Obviously if the file changes content but not
@@ -1592,9 +1591,11 @@ class FortranFileReader(FortranReaderBase):
             message = "FortranFileReader is used with a filename"
             message += " or file-like object."
             raise ValueError(message)
-        mode = fparser.common.sourceinfo.get_source_info(file_candidate)
+        mode = fparser.common.sourceinfo.get_source_info(
+            file_candidate, ignore_encoding
+        )
 
-        FortranReaderBase.__init__(self, self.file, mode, ignore_comments)
+        super().__init__(self.file, mode, ignore_comments)
 
         if include_dirs is None:
             self.include_dirs.insert(0, os.path.dirname(self.id))
@@ -1620,6 +1621,9 @@ class FortranStringReader(FortranReaderBase):
     :param list source_only: Fortran source files to search for modules
                              required by "use" statements.
     :param bool ignore_comments: Whether or not to ignore comments
+    :param Optional[bool] ignore_encoding: whether or not to ignore Python-style \
+        encoding information (e.g. "-*- fortran -*-") when attempting to determine \
+        the format of the source. Default is True.
 
     For example:
 
@@ -1635,7 +1639,12 @@ class FortranStringReader(FortranReaderBase):
     """
 
     def __init__(
-        self, string, include_dirs=None, source_only=None, ignore_comments=True
+        self,
+        string,
+        include_dirs=None,
+        source_only=None,
+        ignore_comments=True,
+        ignore_encoding=True,
     ):
         # The Python ID of the string was used to uniquely identify it for
         # caching purposes. Unfortunately this ID is only unique for the
@@ -1648,8 +1657,10 @@ class FortranStringReader(FortranReaderBase):
         #
         self.id = "string-" + str(hash(string))
         source = StringIO(string)
-        mode = fparser.common.sourceinfo.get_source_info_str(string)
-        FortranReaderBase.__init__(self, source, mode, ignore_comments)
+        mode = fparser.common.sourceinfo.get_source_info_str(
+            string, ignore_encoding=ignore_encoding
+        )
+        super().__init__(source, mode, ignore_comments)
         if include_dirs is not None:
             self.include_dirs = include_dirs[:]
         if source_only is not None:
diff --git a/src/fparser/common/sourceinfo.py b/src/fparser/common/sourceinfo.py
index e8ce5c4..de01d03 100644
--- a/src/fparser/common/sourceinfo.py
+++ b/src/fparser/common/sourceinfo.py
@@ -1,5 +1,5 @@
-# Modified work Copyright (c) 2017-2022 Science and Technology
-# Facilities Council
+# Modified work Copyright (c) 2017-2023 Science and Technology
+# Facilities Council.
 # Original work Copyright (c) 1999-2008 Pearu Peterson
 
 # All rights reserved.
@@ -227,25 +227,35 @@ _HAS_PYF_HEADER = re.compile(r"-[*]-\s*pyf\s*-[*]-", re.I).search
 _FREE_FORMAT_START = re.compile(r"[^c*!]\s*[^\s\d\t]", re.I).match
 
 
-def get_source_info_str(source):
+def get_source_info_str(source, ignore_encoding=True):
     """
     Determines the format of Fortran source held in a string.
 
-    Returns a FortranFormat object.
+    :param bool ignore_encoding: whether or not to ignore any Python-style \
+                                 encoding information in the first line of the file.
+
+    :returns: a FortranFormat object.
+    :rtype: :py:class:`fparser.common.sourceinfo.FortranFormat`
+
     """
     lines = source.splitlines()
     if not lines:
         return FortranFormat(False, False)
 
-    firstline = lines[0].lstrip()
-    if _HAS_F_HEADER(firstline):
-        return FortranFormat(False, True)
-    if _HAS_FIX_HEADER(firstline):
-        return FortranFormat(False, False)
-    if _HAS_FREE_HEADER(firstline):
-        return FortranFormat(True, False)
-    if _HAS_PYF_HEADER(firstline):
-        return FortranFormat(True, True)
+    if not ignore_encoding:
+        # We check to see whether the file contains a comment describing its
+        # encoding. This has nothing to do with the Fortran standard (see e.g.
+        # https://peps.python.org/pep-0263/) and hence is not done by default.
+        firstline = lines[0].lstrip()
+        if _HAS_F_HEADER(firstline):
+            # -*- fortran -*- implies Fortran77 so fixed format.
+            return FortranFormat(False, True)
+        if _HAS_FIX_HEADER(firstline):
+            return FortranFormat(False, False)
+        if _HAS_FREE_HEADER(firstline):
+            return FortranFormat(True, False)
+        if _HAS_PYF_HEADER(firstline):
+            return FortranFormat(True, True)
 
     line_tally = 10000  # Check up to this number of non-comment lines
     is_free = False
@@ -263,7 +273,7 @@ def get_source_info_str(source):
 ##############################################################################
 
 
-def get_source_info(file_candidate):
+def get_source_info(file_candidate, ignore_encoding=True):
     """
     Determines the format of Fortran source held in a file.
 
@@ -303,7 +313,9 @@ def get_source_info(file_candidate):
         #
         pointer = file_candidate.tell()
         file_candidate.seek(0)
-        source_info = get_source_info_str(file_candidate.read())
+        source_info = get_source_info_str(
+            file_candidate.read(), ignore_encoding=ignore_encoding
+        )
         file_candidate.seek(pointer)
         return source_info
 
@@ -322,7 +334,9 @@ def get_source_info(file_candidate):
     with open(
         file_candidate, "r", encoding="utf-8", errors="fparser-logging"
     ) as file_object:
-        string = get_source_info_str(file_object.read())
+        string = get_source_info_str(
+            file_object.read(), ignore_encoding=ignore_encoding
+        )
     return string
 
 


Test patch:
diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml
index bbafd85..229fd1d 100644
--- a/.github/workflows/unit-tests.yml
+++ b/.github/workflows/unit-tests.yml
@@ -1,7 +1,7 @@
 # -----------------------------------------------------------------------------
 # BSD 3-Clause License
 #
-# Copyright (c) 2020-2022, Science and Technology Facilities Council.
+# Copyright (c) 2020-2023, Science and Technology Facilities Council.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -65,7 +65,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: ['3.6', '3.8', '3.10']
+        python-version: ['3.7', '3.8', '3.11']
     steps:
     - uses: actions/checkout@v2
       with:
diff --git a/src/fparser/common/tests/test_readfortran.py b/src/fparser/common/tests/test_readfortran.py
index 36f64d3..a3b7f5a 100644
--- a/src/fparser/common/tests/test_readfortran.py
+++ b/src/fparser/common/tests/test_readfortran.py
@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 ##############################################################################
-# Copyright (c) 2017-2022 Science and Technology Facilities Council
+# Copyright (c) 2017-2023 Science and Technology Facilities Council.
 #
 # All rights reserved.
 #
@@ -104,11 +104,10 @@ def test_line_map():
     assert line.get_line(apply_map=True) == "write(*,*) 'var = ', var"
 
 
-def test_111fortranreaderbase(log, monkeypatch):
+def test_fortranreaderbase_logging(log, monkeypatch):
     """
-    Tests the FortranReaderBase class.
+    Tests the logging functionality of the FortranReaderBase class.
 
-    Currently only tests logging functionality.
     """
 
     class FailFile:
@@ -534,7 +533,7 @@ def test_include6(tmpdir, ignore_comments):
     fortran_code = (
         "program test\n"
         "  ! prog comment 1\n"
-        "  include '{0}'\n"
+        "  include '{0}' ! this is an include\n"
         "  ! prog comment 2\n"
         "end program".format(include_filename)
     )
@@ -549,6 +548,7 @@ def test_include6(tmpdir, ignore_comments):
             "! include comment 1\n"
             "print *, 'Hello'\n"
             "! include comment 2\n"
+            "! this is an include\n"
             "! prog comment 2\n"
             "end program"
         )
@@ -858,7 +858,31 @@ def test_string_reader():
         assert unit_under_test.get_single_line(ignore_empty=True) == expected
 
 
-##############################################################################
+@pytest.mark.parametrize("reader_cls", [FortranStringReader, FortranFileReader])
+def test_reader_ignore_encoding(reader_cls, tmp_path):
+    """
+    Tests that the Fortran{String,File}Reader can be configured to take notice of
+    Python-style encoding information.
+    """
+    source = "! -*- f77 -*-\n" + FULL_FREE_SOURCE
+    if reader_cls is FortranFileReader:
+        sfile = tmp_path / "my_test.f90"
+        sfile.write_text(source)
+        # File location with full path.
+        rinput = str(sfile.absolute())
+    else:
+        rinput = source
+    reader = reader_cls(rinput)
+    # By default the encoding information is ignored so the format should be
+    # free format, not strict.
+    assert reader.format == FortranFormat(True, False)
+    # Check that explicitly setting ignore_encoding=True gives the same behaviour.
+    reader1 = reader_cls(rinput, ignore_encoding=True)
+    assert reader1.format == FortranFormat(True, False)
+    # Now test when the reader takes notice of the encoding information.
+    reader2 = reader_cls(rinput, ignore_encoding=False)
+    # Should be fixed format, strict.
+    assert reader2.format == FortranFormat(False, True)
 
 
 def test_inherited_f77():
@@ -887,7 +911,9 @@ a    'g
     ]
 
     # Reading from buffer
-    reader = FortranStringReader(string_f77, ignore_comments=False)
+    reader = FortranStringReader(
+        string_f77, ignore_comments=False, ignore_encoding=False
+    )
     assert reader.format.mode == "f77", repr(reader.format.mode)
     stack = expected[:]
     for item in reader:
@@ -899,7 +925,7 @@ a    'g
     with open(filename, "w") as fortran_file:
         print(string_f77, file=fortran_file)
 
-    reader = FortranFileReader(filename, ignore_comments=False)
+    reader = FortranFileReader(filename, ignore_comments=False, ignore_encoding=False)
     stack = expected[:]
     for item in reader:
         assert str(item) == stack.pop(0)
@@ -971,7 +997,9 @@ end python module foo
         "Comment('! end of file',(26, 26))",
     ]
 
-    reader = FortranStringReader(string_pyf, ignore_comments=False)
+    reader = FortranStringReader(
+        string_pyf, ignore_comments=False, ignore_encoding=False
+    )
     assert reader.format.mode == "pyf", repr(reader.format.mode)
     for item in reader:
         assert str(item) == expected.pop(0)
@@ -1018,7 +1046,9 @@ cComment
         "Comment('',(15, 15))",
         "line #17'end'",
     ]
-    reader = FortranStringReader(string_fix90, ignore_comments=False)
+    reader = FortranStringReader(
+        string_fix90, ignore_comments=False, ignore_encoding=False
+    )
     assert reader.format.mode == "fix", repr(reader.format.mode)
     for item in reader:
         assert str(item) == expected.pop(0)
diff --git a/src/fparser/common/tests/test_sourceinfo.py b/src/fparser/common/tests/test_sourceinfo.py
index aa3df87..098de10 100644
--- a/src/fparser/common/tests/test_sourceinfo.py
+++ b/src/fparser/common/tests/test_sourceinfo.py
@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 ##############################################################################
-# Copyright (c) 2017-2022 Science and Technology Facilities Council.
+# Copyright (c) 2017-2023 Science and Technology Facilities Council.
 #
 # All rights reserved.
 #
@@ -206,8 +206,9 @@ def test_format_from_mode_bad():
         ("! -*- fix -*-", FortranFormat(False, False)),
         ("! -*- pyf -*-", FortranFormat(True, True)),
     ],
+    name="header",
 )
-def header(request):
+def header_fixture(request):
     """
     Returns parameters for header tests.
     """
@@ -283,7 +284,7 @@ def test_get_source_info_str(header, content):
     if content[0] is not None:
         full_source += content[0]
 
-    source_info = get_source_info_str(full_source)
+    source_info = get_source_info_str(full_source, ignore_encoding=False)
     if header[0]:
         assert source_info == header[1]
     else:  # No header
@@ -331,7 +332,7 @@ def test_get_source_info_filename(extension, header, content):
         print(full_source, file=source_file)
 
     try:
-        source_info = get_source_info(filename)
+        source_info = get_source_info(filename, ignore_encoding=False)
         if extension[1] is not None:
             assert source_info == extension[1]
         elif header[0] is not None:
@@ -361,7 +362,7 @@ def test_get_source_info_file(extension, header, content):
         print(full_source, file=source_file)
         source_file.seek(0)
 
-        source_info = get_source_info(source_file)
+        source_info = get_source_info(source_file, ignore_encoding=False)
         if header[0] is not None:
             assert source_info == header[1]
         else:  # No header