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 all 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 @@ -460,6 +460,7 @@ def match(reader):
[Use_Stmt, Import_Stmt, Implicit_Part, Declaration_Construct],
None,
reader,
strict_order=True,
)


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


Expand Down Expand Up @@ -11070,6 +11072,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 @@ -12577,6 +12580,7 @@ def match(reader):
[Specification_Part, Execution_Part, Internal_Subprogram_Part],
End_Function_Stmt,
reader,
once_only=True,
)


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


Expand Down
83 changes: 55 additions & 28 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,11 +34,24 @@
""" Module containing tests for aspects of fparser2 related to comments """

import pytest
from fparser.two.Fortran2003 import Program, Comment, Subroutine_Subprogram
from fparser.two.utils import walk
from fparser.api import get_reader

from fparser.api import get_reader
from fparser.two.Fortran2003 import (
Program,
Comment,
Subroutine_Subprogram,
Execution_Part,
Specification_Part,
Block_Nonlabel_Do_Construct,
Main_Program,
Write_Stmt,
End_Program_Stmt,
If_Construct,
Allocate_Stmt,
Derived_Type_Def,
Function_Subprogram)
from fparser.two.parser import ParserFactory
from fparser.two.utils import walk, get_child

# this is required to setup the fortran2003 classes
_ = ParserFactory().create(std="f2003")
Expand Down Expand Up @@ -242,24 +255,20 @@ 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

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 @@ -299,7 +308,6 @@ def test_function_comments():
! That was a function
end function my_mod
"""
from fparser.two.Fortran2003 import Function_Subprogram

reader = get_reader(source, isfree=True, ignore_comments=False)
fn_unit = Function_Subprogram(reader)
Expand All @@ -308,17 +316,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 +348,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.children[2]
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 All @@ -368,7 +373,6 @@ def test_derived_type():
! Ending comment
end type my_type
"""
from fparser.two.Fortran2003 import Derived_Type_Def

reader = get_reader(source, isfree=True, ignore_comments=False)
dtype = Derived_Type_Def(reader)
Expand Down Expand Up @@ -399,8 +403,6 @@ def test_action_stmts():
my_array2(size))
end if
"""
from fparser.two.Fortran2003 import If_Construct, Allocate_Stmt
from fparser.two.utils import get_child

reader = get_reader(source, isfree=True, ignore_comments=False)
ifstmt = If_Construct(reader)
Expand All @@ -409,3 +411,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)
44 changes: 43 additions & 1 deletion src/fparser/two/tests/test_fortran2003.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ def assert_raises(exc, fcls, string):
# SECTION 2
#


def test_specification_part():
"""Tests for parsing specification-part (R204)."""
reader = get_reader(
Expand Down Expand Up @@ -141,6 +140,49 @@ 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)

# Test a correct order of subclasses matches and then an incorrect
# order of subclasses fails to match (due to the strict_order flag
# being set). In this case Use_Stmt should precede
# Declaration_Construct
code = (
"USE my_mod\n"
"INTEGER :: a")
reader = get_reader(code)
tcls = Specification_Part
obj = tcls(reader)
assert str(obj) == code

reader = get_reader(
"integer :: a\n"
"use my_mod\n")
tcls = Specification_Part
obj = tcls(reader)
assert obj is None

#
# 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
29 changes: 19 additions & 10 deletions src/fparser/two/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,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 @@ -642,6 +643,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 @@ -690,18 +694,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 @@ -769,11 +769,11 @@ def match(
reader,
f"Name '{end_name}' has no corresponding starting name",
)
elif strict_match_names and start_name and not end_name:
if strict_match_names and start_name and not end_name:
raise FortranSyntaxError(
reader, f"Expecting name '{start_name}' but none given"
)
elif (
if (
start_name
and end_name
and (start_name.lower() != end_name.lower())
Expand All @@ -785,9 +785,18 @@ 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.

# There was a match for this sub-class and
# once_only is set so move on to the next
# sub-class
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
Loading