Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding strict order statements (closes #353) #358

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/fparser/two/Fortran2003.py
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ def match(reader):
[Use_Stmt, Import_Stmt, Implicit_Part, Declaration_Construct],
None,
reader,
strict_order=True,
)


Expand Down Expand Up @@ -10003,6 +10004,7 @@ def match(reader):
reader,
match_names=True,
strict_order=True,
once_only=True,
)


Expand Down Expand Up @@ -10157,6 +10159,7 @@ def match(reader):
[Specification_Part, Module_Subprogram_Part],
End_Module_Stmt,
reader,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each sub-class can only match once for a valid module? (I initially got worried that this wouldn't work for multiple modules in a file but it's fine.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, a Module can only contain one Module_Stmt, one Specification_Part, one Module_Subprogram_Part and one End_Module_Stmt. Multiple modules are supported in 'higher level' rules, such as Program_Unit.

once_only=True,
)


Expand Down Expand Up @@ -11514,6 +11517,7 @@ def match(reader):
[Specification_Part, Execution_Part, Internal_Subprogram_Part],
End_Function_Stmt,
reader,
once_only=True,
)


Expand Down Expand Up @@ -11790,6 +11794,7 @@ def match(reader):
[Specification_Part, Execution_Part, Internal_Subprogram_Part],
End_Subroutine_Stmt,
reader,
once_only=True,
)


Expand Down
68 changes: 47 additions & 21 deletions src/fparser/two/tests/test_comments.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2017-2020 Science and Technology Facilities Council
# Copyright (c) 2017-2022 Science and Technology Facilities Council
# All rights reserved.
#
# Modifications made as part of the fparser project are distributed
Expand Down Expand Up @@ -34,7 +34,14 @@
""" Module containing tests for aspects of fparser2 related to comments """

import pytest
from fparser.two.Fortran2003 import Program, Comment, Subroutine_Subprogram
from fparser.two.Fortran2003 import (
Program,
Comment,
Subroutine_Subprogram,
Execution_Part,
Specification_Part,
Block_Nonlabel_Do_Construct,
)
from fparser.two.utils import walk
from fparser.api import get_reader

Expand Down Expand Up @@ -242,24 +249,21 @@ def test_prog_comments():
# |--> Comment
# |--> Main_Program
# . |--> Program_Stmt
# . |--> Specification_Part
# . . \--> Implicit_Part
# . . \--> Comment
# . |--> Comment
# |--> Execution_Part
# | |--> Write_Stmt
# | \--> Comment
# | |--> Comment
# . .
# .
# |--> Comment
from fparser.two.Fortran2003 import Main_Program, Write_Stmt, End_Program_Stmt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move these imports to the top (and check for any subsequent repeats).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved all imports to the top.


walk(obj.children, Comment, debug=True)
assert type(obj.content[0]) == Comment
assert str(obj.content[0]) == "! A troublesome comment"
assert type(obj.content[1]) == Main_Program
main_prog = obj.content[1]
assert type(main_prog.content[1].content[0].content[0]) == Comment
assert str(main_prog.content[1].content[0].content[0]) == "! A full comment line"
assert type(main_prog.content[1]) == Comment
assert str(main_prog.content[1]) == "! A full comment line"
exec_part = main_prog.content[2]
assert type(exec_part.content[0]) == Write_Stmt
# Check that we have the in-line comment as a second statement
Expand Down Expand Up @@ -308,17 +312,15 @@ def test_function_comments():
# <class 'fparser.two.Fortran2003.Name'>
# <type 'NoneType'>
# <type 'NoneType'>
# <class 'fparser.two.Fortran2003.Specification_Part'>
# <class 'fparser.two.Fortran2003.Implicit_Part'>
# <class 'fparser.two.Fortran2003.Comment'>
# <class 'fparser.two.Fortran2003.Comment'>
# <type 'str'>, "'! This is a function'"
comment = fn_unit.content[1].content[0].content[0]
comment = fn_unit.content[1]
assert isinstance(comment, Comment)
assert "! This is a function" in str(comment)
comment = fn_unit.content[1].content[2].content[0]
comment = fn_unit.content[2].content[2]
assert isinstance(comment, Comment)
assert "! Comment1" in str(comment)
exec_part = fn_unit.content[2]
exec_part = fn_unit.content[3]
comment = exec_part.content[1]
assert isinstance(comment, Comment)
assert "! Comment2" in str(comment)
Expand All @@ -342,16 +344,15 @@ def test_subroutine_comments():
reader = get_reader(source, isfree=True, ignore_comments=False)
fn_unit = Subroutine_Subprogram(reader)
assert isinstance(fn_unit, Subroutine_Subprogram)
walk(fn_unit.children, Comment, debug=True)
spec_part = fn_unit.content[1]
comment = spec_part.content[0].content[0]
comment = fn_unit.content[1]
assert isinstance(comment, Comment)
assert "! First comment" in str(comment)
comment = spec_part.content[2].content[0]
spec_part = fn_unit.children[2]
comment = spec_part.content[2]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/content/children/ since you use that on the previous line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

assert isinstance(comment, Comment)
assert comment.parent is spec_part.content[2]
assert comment.parent is spec_part
assert "! Body comment" in str(comment)
exec_part = fn_unit.content[2]
exec_part = fn_unit.content[3]
comment = exec_part.content[1]
assert isinstance(comment, Comment)
assert comment.parent is exec_part
Expand Down Expand Up @@ -409,3 +410,28 @@ def test_action_stmts():
assert "a big array" in str(ifstmt)
cmt = get_child(ifstmt, Comment)
assert cmt.parent is ifstmt


def test_do_loop_coments():
"""Check that comments around and within do loops appear in the expected
places in the tree."""
source = """\
program test
integer :: arg1
integer :: iterator
!comment_out 1
arg1 = 10
!comment_out 2
do iterator = 0,arg1
!comment_in
print *, iterator
end do
!comment_out 3
end program test"""
reader = get_reader(source, isfree=True, ignore_comments=False)
obj = Program(reader)
comments = walk(obj, Comment)
assert isinstance(comments[0].parent, Specification_Part)
assert isinstance(comments[1].parent, Execution_Part)
assert isinstance(comments[2].parent, Block_Nonlabel_Do_Construct)
assert isinstance(comments[3].parent, Execution_Part)
25 changes: 25 additions & 0 deletions src/fparser/two/tests/test_fortran2003.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,31 @@ def test_specification_part():
assert isinstance(obj, tcls), repr(obj)
assert "TYPE :: a\nEND TYPE a\nTYPE :: b\nEND TYPE b" in str(obj)

obj = tcls(
get_reader(
"""\
! comment1
use a
! comment2
use b
! comment3
#define x
use c
include 'fred'
#ifdef x
use d
#endif
""",
ignore_comments=False,
)
)
assert isinstance(obj, tcls), repr(obj)
expected = (
"! comment1\nUSE a\n! comment2\nUSE b\n! comment3\n#define x\n"
"USE c\nINCLUDE 'fred'\n#ifdef x\nUSE d\n#endif"
)
assert expected in str(obj)


#
# SECTION 3
Expand Down
24 changes: 13 additions & 11 deletions src/fparser/two/tests/utils/test_blockbase.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2019-2021, Science and Technology Facilities Council
# Copyright (c) 2019-2022, Science and Technology Facilities Council

# All rights reserved.

Expand Down Expand Up @@ -81,20 +81,22 @@ def test_include(f2003_create):
),
ignore_comments=False,
)
result = BlockBase.match(startcls, subclasses, endcls, reader)
result = BlockBase.match(
startcls, subclasses, endcls, reader, strict_order=True, once_only=True
)
assert (
"([Include_Stmt(Include_Filename('1')), Comment('! comment1'), "
"Program_Stmt('PROGRAM', Name('test')), Specification_Part("
"Implicit_Part(Include_Stmt(Include_Filename('2')), "
"Comment('! comment2')), Type_Declaration_Stmt(Intrinsic_Type_Spec("
"'INTEGER', None), None, Entity_Decl_List(',', (Entity_Decl(Name("
"'i'), None, None, None),))), Implicit_Part(Include_Stmt("
"Include_Filename('3')), Comment('! comment3'))), Execution_Part("
"Assignment_Stmt(Name('i'), '=', Int_Literal_Constant('1', None)), "
"Include_Stmt(Include_Filename('4')), Comment('! comment4')), "
"Program_Stmt('PROGRAM', Name('test')), Include_Stmt("
"Include_Filename('2')), Comment('! comment2'), Specification_Part"
"(Type_Declaration_Stmt(Intrinsic_Type_Spec('INTEGER', None), None, "
"Entity_Decl_List(',', (Entity_Decl(Name('i'), None, None, None),))), "
"Include_Stmt(Include_Filename('3')), Comment('! comment3')), "
"Execution_Part(Assignment_Stmt(Name('i'), '=', "
"Int_Literal_Constant('1', None)), Include_Stmt("
"Include_Filename('4')), Comment('! comment4')), "
"Internal_Subprogram_Part(Contains_Stmt('CONTAINS'), Include_Stmt("
"Include_Filename('5')), Comment('! comment5')), End_Program_Stmt("
"'PROGRAM', Name('test'))],)" in str(result).replace("u'", "'")
"'PROGRAM', Name('test'))],)" in str(result)
)
assert "should" not in str(result)

Expand Down
22 changes: 14 additions & 8 deletions src/fparser/two/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ def match(
enable_where_construct_hook=False,
strict_order=False,
strict_match_names=False,
once_only=False,
):
"""
Checks whether the content in reader matches the given
Expand All @@ -574,6 +575,9 @@ def match(
given subclasses.
:param bool strict_match_names: if start name present, end name \
must exist and match.
:param bool once_only: whether to restrict matching to at most \
once for a subclass. This is only active if strict_order is \
also set.

:return: instance of startcls or None if no match is found
:rtype: startcls
Expand Down Expand Up @@ -619,18 +623,14 @@ def match(
if match_names:
start_name = obj.get_start_name()

# Comments and Include statements are always valid sub-classes
classes = subclasses + [di.Comment, di.Include_Stmt]
# Preprocessor directives are always valid sub-classes
cpp_classes = [
getattr(di.C99Preprocessor, cls_name)
for cls_name in di.C99Preprocessor.CPP_CLASS_NAMES
]
classes += cpp_classes
classes = subclasses
if endcls is not None:
classes += [endcls]
endcls_all = tuple([endcls] + endcls.subclasses[endcls.__name__])

# Deal with any preceding comments, includes, and/or directives
DynamicImport.add_comments_includes_directives(content, reader)

try:
# Start trying to match the various subclasses, starting from
# the beginning of the list (where else?)
Expand Down Expand Up @@ -714,9 +714,15 @@ def match(
# We've found the enclosing end statement so break out
found_end = True
break

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not strictly yours but since we're here pylint complains that the elif at L701 could be an if. Probably the same goes for L705.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified.

# Deal with any following comments, includes, and/or directives
DynamicImport.add_comments_includes_directives(content, reader)

if not strict_order:
# Return to start of classes list now that we've matched.
i = 0
elif once_only:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest a comment: we had a match for this sub-class and once_only is set so move on to the next sub-class.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comment.

i += 1
if enable_if_construct_hook:
if isinstance(obj, di.Else_If_Stmt):
# Got an else-if so go back to start of possible
Expand Down