Repository: stfc/fparser

Issue:
fparser handles `!f2py` comments as directives
Due to its origins in the f2py project, `FortranFileReader` still contains functionality that treats comments beginning with `!f2py` as directives. Essentially, it seems to strip off the `!f2py` and then attempts to parse the remainder of the line as standard Fortran.

In this issue we will investigate whether there is already functionality to control this behaviour and, if there isn't, add some.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 58aa14c..bd2e1f3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,10 @@ Modifications by (in alphabetical order):
 * P. Vitt, University of Siegen, Germany
 * A. Voysey, UK Met Office
 
+11/03/2021 PR #269 for #268. Removed recognition of f2py directives in
+	   the reader. The original code is still there if needed in
+	   the future.
+
 20/01/2021 PR #289 for #288. Bug fix for matching expressions containing
            real literals with signed exponents. This bug was introduced
            by #285.
diff --git a/doc/developers_guide.rst b/doc/developers_guide.rst
index b34125b..7b35e13 100644
--- a/doc/developers_guide.rst
+++ b/doc/developers_guide.rst
@@ -49,6 +49,15 @@ provided as a string. Both of these classes sub-class `FortranReaderBase`:
 
 Note that the setting for `ignore_comments` provided here can be overridden
 on a per-call basis by methods such as `get_single_line`.
+The 'mode' of the reader is controlled by passing in a suitable instance of
+the `FortranFormat` class:
+
+.. autoclass:: fparser.common.sourceinfo.FortranFormat
+
+Due to its origins in the f2py project, the reader contains support
+for recognising `f2py` directives
+(https://numpy.org/devdocs/f2py/signature-file.html). However, this
+functionality is disabled by default.
 
 A convenience script called read.py is provided in the scripts
 directory which takes a filename as input and returns the file
diff --git a/src/fparser/common/readfortran.py b/src/fparser/common/readfortran.py
index 7960f99..1f88998 100644
--- a/src/fparser/common/readfortran.py
+++ b/src/fparser/common/readfortran.py
@@ -173,19 +173,26 @@ def _is_fix_cont(line):
     return line and len(line) > 5 and line[5] != ' ' and line[:5] == 5 * ' '
 
 
-def _is_fix_comment(line, isstrict):
-    """
+def _is_fix_comment(line, isstrict, f2py_enabled):
+    '''
     Check whether line is a comment line in fixed format Fortran source.
 
-    :param str line: Line of code to check
-    :param bool isstrict: Whether we are strictly enforcing fixed/free fmt
-
     References
     ----------
     :f2008:`3.3.3`
-    """
+
+    :param str line: line of code to check.
+    :param bool isstrict: whether we are strictly enforcing fixed/free fmt.
+    :param bool f2py_enabled: whether support for f2py directives is enabled.
+
+    :returns: whether or not the supplied line is a fixed-format comment.
+    :rtype: bool
+
+    '''
     if line:
         if line[0] in '*cC!':
+            if f2py_enabled and line[1:5].lower() == "f2py":
+                return False
             return True
         if not isstrict:
             i = line.find('!')
@@ -544,14 +551,14 @@ class FortranReaderBase(object):
     code, free format Fortran code, or PYF signatures (with extended
     free format Fortran syntax).
 
-    :param source: A file-like object with .next() method used to
+    :param source: a file-like object with .next() method used to \
                    retrive a line.
-    :type source: either :py:class:`six.StringIO` or a file handle
-    :param mode: A FortranFormat object as returned by
+    :type source: :py:class:`six.StringIO` or a file handle
+    :param mode: a FortranFormat object as returned by \
                  `sourceinfo.get_source_info()`
     :type mode: :py:class:`fparser.common.sourceinfo.Format`
-    :param bool isstrict: Whether we are strictly enforcing fixed format
-    :param bool ignore_comments: Whether or not to discard comments
+    :param bool isstrict: whether we are strictly enforcing fixed format.
+    :param bool ignore_comments: whether or not to discard comments.
 
     The Fortran source is iterated by `get_single_line`,
     `get_next_line`, `put_single_line` methods.
@@ -670,8 +677,8 @@ class FortranReaderBase(object):
         In both situations ``linecount`` will be incremented, that is,
         the line will be consumed.
 
-        :param bool ignore_empty: If True then ignore empty lines.
-        :param bool ignore_comments: If True then ignore comments (overrides
+        :param bool ignore_empty: if True then ignore empty lines.
+        :param bool ignore_comments: if True then ignore comments (overrides \
                                      self._ignore_comments)
 
         See also
@@ -718,11 +725,12 @@ class FortranReaderBase(object):
 
         self.source_lines.append(line)
 
-        if ignore_comments and (self.format.is_fixed or self.format.is_f77):
+        if ignore_comments and (self._format.is_fixed or self._format.is_f77):
             # Check for a fixed-format comment. If the current line *is*
             # a comment and we are ignoring them, then recursively call this
             # routine again to get the next source line.
-            if _is_fix_comment(line, isstrict=self.format.is_strict):
+            if _is_fix_comment(line, isstrict=self._format.is_strict,
+                               f2py_enabled=self._format.f2py_enabled):
                 return self.get_single_line(ignore_empty, ignore_comments)
 
         if ignore_empty and not line:
@@ -1110,22 +1118,24 @@ class FortranReaderBase(object):
         return (line, line.lstrip().startswith('#'))
 
     def handle_cf2py_start(self, line):
-        """ Apply f2py directives to line.
+        '''
+        Process any f2py directives contained in the supplied line. If
+        support for such directives has been disabled then the line is
+        returned unchanged.
 
         F2py directives are specified in the beginning of the line.
 
         f2py directives can be used only in Fortran codes.  They are
         ignored when used inside PYF files.
 
-        Parameters
-        ----------
-        line : str
+        :param str line: the line to check for f2py directives.
 
-        Returns
-        -------
-        line : str
-        """
-        if not line or self._format.is_pyf:
+        :returns: the line with any f2py directives applied (if they are \
+                  enabled in the reader).
+        :rtype: str
+
+        '''
+        if not line or self._format.is_pyf or not self._format.f2py_enabled:
             return line
         if self._format.is_fixed:
             if line[0] in '*cC!#':
@@ -1190,7 +1200,7 @@ class FortranReaderBase(object):
             commentline = ''.join(items[k:])
             break
         if commentline is not None:
-            if commentline.startswith('!f2py'):
+            if self._format.f2py_enabled and commentline.startswith('!f2py'):
                 # go to next iteration:
                 newline = ''.join(noncomment_items) + commentline[5:]
                 self.f2py_comment_lines.append(lineno)
@@ -1278,7 +1288,8 @@ class FortranReaderBase(object):
     # the following contexts: f77, fixed, free, pyf.
 
     def get_source_item(self):
-        """ Return next source item.
+        '''
+        Return the next source item.
 
         A source item is ..
         - a fortran line
@@ -1286,11 +1297,20 @@ class FortranReaderBase(object):
         - a multiline - lines inside triple-quotes, only when in ispyf mode
         - a comment line
         - a preprocessor directive line
-        """
+
+        :returns: the next source item.
+        :rtype: :py:class:`fparser.common.readfortran.Line` or \
+            :py:class:`fparser.common.readfortran.MultiLine` or \
+            :py:class:`fparser.common.readfortran.Comment` or \
+            :py:class:`fparser.common.readfortran.CppDirective` or \
+            :py:class:`fparser.common.readfortran.SyntaxErrorLine` or \
+            :py:class:`fparser.common.readfortran.SyntaxErrorMultiLine`
+
+        '''
         get_single_line = self.get_single_line
         line = get_single_line()
         if line is None:
-            return
+            return None
         startlineno = self.linecount
         line, is_cpp_directive = self.handle_cpp_directive(line)
         if is_cpp_directive:
@@ -1306,7 +1326,8 @@ class FortranReaderBase(object):
                                            endlineno)
 
         line = self.handle_cf2py_start(line)
-        is_f2py_directive = startlineno in self.f2py_comment_lines
+        is_f2py_directive = (self._format.f2py_enabled and
+                             startlineno in self.f2py_comment_lines)
         isstrict = self._format.is_strict
         have_comment = False
         label = None
@@ -1315,11 +1336,11 @@ class FortranReaderBase(object):
         if self._format.is_pyf:
             # handle multilines
             for mlstr in ['"""', "'''"]:
-                r = self.handle_multilines(line, startlineno, mlstr)
-                if r:
-                    return r
+                multiline = self.handle_multilines(line, startlineno, mlstr)
+                if multiline:
+                    return multiline
         if self._format.is_fixed:
-            if _is_fix_comment(line, isstrict):
+            if _is_fix_comment(line, isstrict, self._format.f2py_enabled):
                 # comment line:
                 return self.comment_item(line, startlineno, startlineno)
 
@@ -1415,14 +1436,15 @@ class FortranReaderBase(object):
             lines = [newline]
             next_line = self.get_next_line()
 
-            while _is_fix_cont(next_line) or _is_fix_comment(next_line,
-                                                             isstrict):
+            while (_is_fix_cont(next_line) or
+                   _is_fix_comment(next_line, isstrict,
+                                   self._format.f2py_enabled)):
                 # handle fix format line continuations for F90 or
                 # newer code.  Mixing fix format and free format line
                 # continuations is not allowed nor detected, just
                 # eject warnings.
                 line2 = get_single_line()  # consume next_line as line2
-                if _is_fix_comment(line2, isstrict):
+                if _is_fix_comment(line2, isstrict, self._format.f2py_enabled):
                     # handle fix format comments inside line continuations
                     # after the line construction
                     citem = self.comment_item(line2,
diff --git a/src/fparser/common/sourceinfo.py b/src/fparser/common/sourceinfo.py
index 807ff4c..155b131 100644
--- a/src/fparser/common/sourceinfo.py
+++ b/src/fparser/common/sourceinfo.py
@@ -1,4 +1,4 @@
-# Modified work Copyright (c) 2017-2019 Science and Technology
+# Modified work Copyright (c) 2017-2021 Science and Technology
 # Facilities Council
 # Original work Copyright (c) 1999-2008 Pearu Peterson
 
@@ -85,15 +85,13 @@ class FortranFormat(object):
     "not strict" although it's not entirely clear what that means. It may
     refer to the strictness of adherance to fixed format although what that
     means in the context of free format I don't know.
-    '''
-    def __init__(self, is_free, is_strict):
-        '''
-        Constructs a FortranFormat object from the describing parameters.
 
-        Arguments:
-            is_free   - (Boolean) True for free format, False for fixed.
-            is_strict - (Boolean) Some amount of strictness.
-        '''
+    :param bool is_free: True for free format, False for fixed.
+    :param bool is_strict: some amount of strictness.
+    :param bool enable_f2py: whether f2py directives are enabled or treated \
+                             as comments (the default).
+    '''
+    def __init__(self, is_free, is_strict, enable_f2py=False):
         if is_free is None:
             raise Exception('FortranFormat does not accept a None is_free')
         if is_strict is None:
@@ -101,6 +99,7 @@ class FortranFormat(object):
 
         self._is_free = is_free
         self._is_strict = is_strict
+        self._f2py_enabled = enable_f2py
 
     @classmethod
     def from_mode(cls, mode):
@@ -125,7 +124,8 @@ class FortranFormat(object):
     def __eq__(self, other):
         if isinstance(other, FortranFormat):
             return self.is_free == other.is_free \
-                   and self.is_strict == other.is_strict
+                   and self.is_strict == other.is_strict \
+                   and self.f2py_enabled == other.f2py_enabled
         raise NotImplementedError
 
     def __str__(self):
@@ -183,6 +183,14 @@ class FortranFormat(object):
         '''
         return self._is_free and self._is_strict
 
+    @property
+    def f2py_enabled(self):
+        '''
+        :returns: whether or not f2py directives are enabled.
+        :rtype: bool
+        '''
+        return self._f2py_enabled
+
     @property
     def mode(self):
         '''


Test patch:
diff --git a/src/fparser/common/tests/test_readfortran.py b/src/fparser/common/tests/test_readfortran.py
index 58175e2..ba04c08 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-2020 Science and Technology Facilities Council
+# Copyright (c) 2017-2021 Science and Technology Facilities Council
 #
 # All rights reserved.
 #
@@ -55,6 +55,15 @@ from fparser.common.readfortran import FortranFileReader, \
 from fparser.common.sourceinfo import FortranFormat
 
 
+@pytest.fixture(scope="module",
+                name="f2py_enabled",
+                params=[True, False])
+def f2py_enabled_fixture(request):
+    ''' Fixture that returns whether or not to enable reader support for
+    f2py directives. '''
+    return request.param
+
+
 def test_empty_line_err():
     ''' Check that we raise the expected error if we try and create
     an empty Line '''
@@ -809,7 +818,6 @@ c12346 comment
      'bar
 a    'g
       abc=2
-cf2py call me ! hey
       call you ! hi
       end
      '"""
@@ -819,8 +827,8 @@ cf2py call me ! hey
                 "line #4'call foobar'",
                 'Comment("a    \'g",(6, 6))',
                 "line #7'abc=2'",
-                "line #9'call you ! hi'",
-                "line #10'end'"]
+                "line #8'call you ! hi'",
+                "line #9'end'"]
 
     # Reading from buffer
     reader = FortranStringReader(
@@ -927,7 +935,6 @@ cComment
 !
      !4!line cont. with comment symbol
      &5
-      a = 3!f2py.14 ! pi!
 !   KDMO
       write (obj%print_lun, *) ' KDMO : '
       write (obj%print_lun, *) '  COORD = ',coord, '  BIN_WID = ',             &
@@ -946,17 +953,15 @@ cComment
                 "line #5'b = 345'",
                 "Comment('!',(6, 6))",
                 "Comment('!line cont. with comment symbol',(7, 7))",
-                "line #9'a = 3.14'",
-                "Comment('! pi!',(9, 9))",
-                "Comment('!   KDMO',(10, 10))",
-                'line #11"write (obj%print_lun, *) \' KDMO : \'"',
-                'line #12"write (obj%print_lun, *) \'  COORD = \',coord,'
+                "Comment('!   KDMO',(9, 9))",
+                'line #10"write (obj%print_lun, *) \' KDMO : \'"',
+                'line #11"write (obj%print_lun, *) \'  COORD = \',coord,'
                 + ' \'  BIN_WID = \',             &"',
-                'line #13"obj%bin_wid,\'  VEL_DMO = \', obj%vel_dmo"',
-                "line #14'end subroutine foo'",
-                "line #15'subroutine foo'",
-                "Comment('',(16, 16))",
-                "line #18'end'"]
+                'line #12"obj%bin_wid,\'  VEL_DMO = \', obj%vel_dmo"',
+                "line #13'end subroutine foo'",
+                "line #14'subroutine foo'",
+                "Comment('',(15, 15))",
+                "line #17'end'"]
     reader = FortranStringReader(
         string_fix90, ignore_comments=False)
     assert reader.format.mode == 'fix', repr(reader.format.mode)
@@ -966,6 +971,81 @@ cComment
             re.sub("u", "", expected.pop(0))
 
 
+def test_f2py_directive_fixf90(f2py_enabled):
+    ''' Test the handling of the f2py directive in fixed-format f90. '''
+    string_fix90 = """c -*- fix -*-
+      subroutine foo
+      a = 3!f2py.14 ! pi!
+!f2py a = 0.0
+      end subroutine foo"""
+    reader = FortranStringReader(string_fix90, ignore_comments=False)
+    reader.set_format(FortranFormat(False, False, f2py_enabled))
+    expected = ["Comment('c -*- fix -*-',(1, 1))",
+                "line #2'subroutine foo'"]
+    if f2py_enabled:
+        expected.extend(["line #3'a = 3.14'",
+                         "Comment('! pi!',(3, 3))",
+                         "line #4'a = 0.0'"])
+    else:
+        expected.extend(["line #3'a = 3'",
+                         "Comment('!f2py.14 ! pi!',(3, 3))",
+                         "Comment('!f2py a = 0.0',(4, 4))"])
+    expected.append("line #5'end subroutine foo'")
+    for item in reader:
+        # Remove 'u's to allow for py2/3 unicode differences
+        assert re.sub("u", "", six.text_type(item)) == \
+            re.sub("u", "", expected.pop(0))
+
+
+def test_f2py_freef90(f2py_enabled):
+    ''' Test the handling of f2py directives in free-format f90, in both
+    in-line and full-line comments. '''
+    lines = ["subroutine foo",
+             "   a = 3!f2py.14 ! pi!",
+             "!f2py a = 0.0",
+             "end subroutine foo"]
+    reader = FortranStringReader("\n".join(lines), ignore_comments=False)
+    reader.set_format(FortranFormat(True, False, f2py_enabled))
+    expected = ["line #1'subroutine foo'"]
+    if f2py_enabled:
+        expected.extend(["line #2'a = 3.14'",
+                         "Comment('! pi!',(2, 2))",
+                         "line #3'a = 0.0'"])
+    else:
+        expected.extend(["line #2'a = 3'",
+                         "Comment('!f2py.14 ! pi!',(2, 2))",
+                         "Comment('!f2py a = 0.0',(3, 3))"])
+    expected.append("line #4'end subroutine foo'")
+    for item in reader:
+        # Remove 'u's to allow for py2/3 unicode differences
+        assert re.sub("u", "", six.text_type(item)) == \
+            re.sub("u", "", expected.pop(0))
+
+
+@pytest.mark.xfail(reason="Issue #270: f2py directives not working in F77 "
+                   "code.")
+def test_f2py_directive_f77(f2py_enabled):
+    ''' Test the handling of the f2py directive in fixed-format f77. '''
+    string_f77 = """c -*- f77 -*-
+      subroutine foo
+cf2py call me ! hey
+      end"""
+    expected = ["Comment('c -*- f77 -*-',(1, 1))",
+                "line #2'subroutine foo'"]
+    if f2py_enabled:
+        expected.append("line #3'call me ! hey'")
+    else:
+        expected.append("Comment('cf2py call me ! hey',(3, 3))")
+    expected.append("line #4'end'")
+
+    # Reading from buffer
+    reader = FortranStringReader(
+        string_f77, ignore_comments=False)
+    for item in reader:
+        assert (re.sub("u", "", six.text_type(item)) ==
+                re.sub("u", "", expected.pop(0)))
+
+
 def test_utf_char_in_code(log):
     ''' Check that we cope with Fortran code that contains UTF characters. This
     is not valid Fortran but most compilers cope with it. '''
diff --git a/src/fparser/common/tests/test_sourceinfo.py b/src/fparser/common/tests/test_sourceinfo.py
index 115bab7..a1692db 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-2018 Science and Technology Facilities Council
+# Copyright (c) 2017-2021 Science and Technology Facilities Council
 #
 # All rights reserved.
 #
@@ -94,25 +94,15 @@ def test_fortranformat_constructor(pretty):
     assert unit_under_test.is_fix == (not pretty[0] and not pretty[1])
     assert unit_under_test.is_pyf == (pretty[0] and pretty[1])
     assert unit_under_test.mode == pretty[2]
+    assert not unit_under_test.f2py_enabled
 
 
-##############################################################################
-@pytest.fixture(scope="module",
-                params=[(False, False),
-                        (False, True),
-                        (True, False),
-                        (True, True)])
-def permutations(request):
-    '''
-    Returns all possible permutations of the input arguments.
-    '''
-    return request.param
-
-
-##############################################################################
-
+@pytest.mark.parametrize("permutations", [(False, False),
+                                          (False, True),
+                                          (True, False),
+                                          (True, True)])
 def test_fortranformat_equality(permutations, pretty):
-    #pylint: disable=redefined-outer-name
+    # pylint: disable=redefined-outer-name
     '''
     Tests that the equality operator works as expected.
     '''
@@ -166,10 +156,9 @@ def test_fortranformat_from_mode(mode):
     assert unit_under_test.is_fix == (not mode[1] and not mode[2])
     assert unit_under_test.is_pyf == (mode[1] and mode[2])
     assert str(unit_under_test.mode) == mode[0]
+    assert not unit_under_test.f2py_enabled
 
 
-##############################################################################
-
 def test_format_from_mode_bad():
     '''
     Tests that an exception is thrown for an unrecognised mode string.