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

FIX: Respect exclude option in config file #111

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
77 changes: 50 additions & 27 deletions fprettify/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1929,10 +1929,10 @@ def str2bool(str):
else:
return None

def get_config_file_list(filename):
def get_config_file_list(start_dir):
"""helper function to create list of config files found in parent directories"""
config_file_list = []
dir = os.path.dirname(filename)
dir = start_dir
while True:
config_file = os.path.join(dir, '.fprettify.rc')
if os.path.isfile(config_file):
Expand All @@ -1943,13 +1943,15 @@ def get_config_file_list(filename):
dir = parent
return config_file_list

arguments = {'prog': argv[0],
def get_argparse_arguments():
arguments = {'prog': argv[0],
'description': 'Auto-format modern Fortran source files.',
'formatter_class': argparse.ArgumentDefaultsHelpFormatter}

if argparse.__name__ == "configargparse":
arguments['args_for_setting_config_path'] = ['-c', '--config-file']
arguments['description'] = arguments['description'] + " Config files ('.fprettify.rc') in the home (~) directory and any such files located in parent directories of the input file will be used. When the standard input is used, the search is started from the current directory."
if argparse.__name__ == "configargparse":
arguments['args_for_setting_config_path'] = ['-c', '--config-file']
arguments['description'] = arguments['description'] + " Config files ('.fprettify.rc') in the home (~) directory and any such files located in parent directories of the input file will be used. When the standard input is used, the search is started from the current directory."
return arguments

def get_arg_parser(args):
"""helper function to create the parser object"""
Expand Down Expand Up @@ -2027,9 +2029,17 @@ def get_arg_parser(args):
version='%(prog)s 0.3.7')
return parser

parser = get_arg_parser(arguments)

args = parser.parse_args(argv[1:])
def pars_args_with_config_file(directory):
"""
Parse arguments together with the config file.
Requires configargparse package.
"""
filearguments = get_argparse_arguments()
filearguments['default_config_files'] = ['~/.fprettify.rc'] \
+ get_config_file_list(directory if directory != '-' else os.getcwd())
file_argparser = get_arg_parser(filearguments)
file_args = file_argparser.parse_args(argv[1:])
return file_args

def build_ws_dict(args):
"""helper function to build whitespace dictionary"""
Expand All @@ -2046,6 +2056,19 @@ def build_ws_dict(args):
ws_dict['intrinsics'] = args.whitespace_intrinsics
return ws_dict

def build_case_dict(args):
Copy link
Author

@danielhollas danielhollas May 27, 2021

Choose a reason for hiding this comment

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

Just a bit of unrelated refactor to make this consistent with build_ws_dict().

"""helper function to build case dictionary"""
return {
'keywords' : file_args.case[0],
'procedures' : file_args.case[1],
'operators' : file_args.case[2],
'constants' : file_args.case[3]
}

parser = get_arg_parser(get_argparse_arguments())

args = parser.parse_args(argv[1:])

# support legacy input:
if 'stdin' in args.path and not os.path.isfile('stdin'):
args.path = ['-' if _ == 'stdin' else _ for _ in args.path]
Expand All @@ -2057,13 +2080,14 @@ def build_ws_dict(args):
sys.exit(1)
else:
if not os.path.exists(directory):
sys.stderr.write("directory " + directory +
sys.stderr.write("file " + directory +
" does not exist!\n")
sys.exit(1)
if not os.path.isfile(directory) and directory != '-' and not args.recursive:
sys.stderr.write("file " + directory + " does not exist!\n")
if os.path.isdir(directory) and not args.recursive:
Copy link
Author

Choose a reason for hiding this comment

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

I think this is a bit clearer. Also the directory != '-' condition was redundant here.

sys.stderr.write("%s is a directory. Use --recursive option\n" % directory)
sys.exit(1)


if not args.recursive:
filenames = [directory]
else:
Expand All @@ -2077,35 +2101,34 @@ def build_ws_dict(args):

for dirpath, dirnames, files in os.walk(directory,topdown=True):

file_args = args
Copy link
Author

Choose a reason for hiding this comment

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

This is the actual fix.

if argparse.__name__ == "configargparse":
file_args = pars_args_with_config_file(directory)

# Prune excluded patterns from list of child directories
# https://stackoverflow.com/a/19859907
Copy link
Author

Choose a reason for hiding this comment

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

I was fairly confused by this code so added a link.

dirnames[:] = [dirname for dirname in dirnames if not any(
[fnmatch(dirname,exclude_pattern) or fnmatch(os.path.join(dirpath,dirname),exclude_pattern)
for exclude_pattern in args.exclude]
fnmatch(dirname,exclude_pattern) or fnmatch(os.path.join(dirpath,dirname),exclude_pattern)
for exclude_pattern in file_args.exclude
)]

for ffile in [os.path.join(dirpath, f) for f in files
if any(f.endswith(_) for _ in ext)
and not any([
and not any(
fnmatch(f,exclude_pattern)
for exclude_pattern in args.exclude])]:
for exclude_pattern in file_args.exclude)]:
filenames.append(ffile)

for filename in filenames:

# reparse arguments using the file's list of config files
filearguments = arguments
file_args = args
if argparse.__name__ == "configargparse":
filearguments['default_config_files'] = ['~/.fprettify.rc'] + get_config_file_list(os.path.abspath(filename) if filename != '-' else os.getcwd())
file_argparser = get_arg_parser(filearguments)
file_args = file_argparser.parse_args(argv[1:])
ws_dict = build_ws_dict(file_args)
dirname = os.path.dirname(os.path.abspath(filename))
file_args = pars_args_with_config_file(dirname)

case_dict = {
'keywords' : file_args.case[0],
'procedures' : file_args.case[1],
'operators' : file_args.case[2],
'constants' : file_args.case[3]
}
ws_dict = build_ws_dict(file_args)
case_dict = build_case_dict(file_args)

stdout = file_args.stdout or directory == '-'
diffonly=file_args.diff
Expand Down
209 changes: 201 additions & 8 deletions fprettify/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,14 @@ def joinpath(path1, path2):
RESULT_FILE = joinpath(RESULT_DIR, r'expected_results')
FAILED_FILE = joinpath(RESULT_DIR, r'failed_results')

TEMP_DIR = joinpath(MYPATH, r'tmp_test_dir/')

RUNSCRIPT = joinpath(MYPATH, r"../../fprettify.py")

fprettify.set_fprettify_logger(logging.ERROR)


class AlienInvasion(Exception):
class FileException(Exception):
"""Should not happen"""
pass

Expand Down Expand Up @@ -85,6 +87,27 @@ def setUp(self):
"""
self.maxDiff = None

def createTmpDir(self):
"""
Create a temporary directory for IO tests
"""
if os.path.lexists(TEMP_DIR):
raise FileException(
"remove directory %s" % TEMP_DIR) # pragma: no cover
os.mkdir(TEMP_DIR)

def removeTmpDir(self):
"""
Remove the temporary test directory and all its content.
"""
if not os.path.isdir(TEMP_DIR):
return

for dirpath, _, files in os.walk(TEMP_DIR, topdown=False):
for f in files:
os.remove(joinpath(dirpath, f))
os.rmdir(dirpath)

@classmethod
def setUpClass(cls):
"""
Expand Down Expand Up @@ -258,10 +281,8 @@ def test_io(self):
instring = "CALL alien_invasion( 👽 )"
outstring_exp = "CALL alien_invasion(👽)"

alien_file = "alien_invasion.f90"
if os.path.isfile(alien_file):
raise AlienInvasion(
"remove file alien_invasion.f90") # pragma: no cover
self.createTmpDir()
alien_file = joinpath(TEMP_DIR, "alien_invasion.f90")

try:
with io.open(alien_file, 'w', encoding='utf-8') as infile:
Expand Down Expand Up @@ -289,11 +310,183 @@ def test_io(self):
for outstr in outstring:
self.assertEqual(outstring_exp, outstr.strip())
except: # pragma: no cover
if os.path.isfile(alien_file):
os.remove(alien_file)
self.removeTmpDir()
raise
else:
self.removeTmpDir()

def test_recursive_mode(self):
"""test recursive mode which finds all fortran files in the tree"""

instring = "CALL alien_invasion( x)"
formatted_string = "CALL alien_invasion(x)"

self.createTmpDir()

# We will create the following paths inside TEMP_DIR
# - alien_file.f90
# - excluded_alien_file.f90
# - subdir/alien_invasion.f90
# - subdir/excluded_alien_invasion.f90
# - excluded_subdir/alien_invasion.f90
alien_file = "alien_invasion.f90"
excluded_file = "excluded_alien_invasion.f90"
subdir = joinpath(TEMP_DIR, "subdir/")
excluded_subdir = joinpath(TEMP_DIR, "excluded_subdir/")
os.mkdir(subdir)
os.mkdir(excluded_subdir)

def create_file(fname):
with io.open(fname, 'w', encoding='utf-8') as infile:
infile.write(instring)

def check_output_file(fname, str_exp):
with io.open(fname, 'r', encoding='utf-8') as infile:
self.assertEqual(str_exp, infile.read().strip())

try:
create_file(joinpath(TEMP_DIR, alien_file))
create_file(joinpath(TEMP_DIR, excluded_file))
create_file(joinpath(subdir, alien_file))
create_file(joinpath(subdir, excluded_file))
create_file(joinpath(excluded_subdir, alien_file))

p1 = subprocess.Popen([
RUNSCRIPT,
'--recursive',
'-e', 'excluded_*',
TEMP_DIR],
stdout=subprocess.PIPE)
p1.wait()

# Check files that should be formatted.
check_output_file(joinpath(TEMP_DIR, alien_file), formatted_string)
check_output_file(joinpath(subdir, alien_file), formatted_string)

# Check excluded files.
check_output_file(joinpath(TEMP_DIR, excluded_file), instring)
check_output_file(joinpath(subdir, excluded_file), instring)

# Check excluded directory.
check_output_file(joinpath(excluded_subdir, alien_file), instring)

except: # pragma: no cover
self.removeTmpDir()
raise
else:
self.removeTmpDir()

def test_config_stdin(self):
Copy link
Author

Choose a reason for hiding this comment

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

This is a test for the fix in #98.

outstring = []
instring = "CALL alien_invasion( x)"
outstring_with_config = "call alien_invasion(x)"
self.createTmpDir()

alien_file = joinpath(TEMP_DIR, "alien_invasion.f90")
config_file = joinpath(os.getcwd(), ".fprettify.rc")
conf_string = "case=[1,1,1,2]\nexclude=[excluded*]"

if os.path.exists(config_file):
raise FileException(
"remove file %s" % conf_file_cwd) # pragma: no cover

def create_file(fname, string):
with io.open(fname, 'w', encoding='utf-8') as infile:
infile.write(string)

try:
create_file(alien_file, instring)
create_file(config_file, conf_string)

# testing stdin --> stdout, with configuration file read from CWD
p1 = subprocess.Popen(RUNSCRIPT,
stdout=subprocess.PIPE, stdin=subprocess.PIPE)
outstr = p1.communicate(instring.encode('UTF-8'))[0].decode('UTF-8')
self.assertEqual(outstring_with_config, outstr.strip())

except: # pragma: no cover
self.removeTmpDir()
if os.path.isfile(config_file):
os.remove(config_file)
raise
else:
self.removeTmpDir()
os.remove(config_file)

def test_config_file(self):
"""simple test for configuration file reading"""

outstring = []
instring = "CALL alien_invasion( x)"
outstring_with_config = "call alien_invasion(x)"
outstring_without_config = "CALL alien_invasion(x)"

self.createTmpDir()
dirname = TEMP_DIR

alien_file = joinpath(dirname, "alien_invasion.f90")
excluded_file = joinpath(dirname, "excluded.f90")
config_file = joinpath(dirname, ".fprettify.rc")
conf_string = "case=[1,1,1,2]\nexclude=[excluded*]"

excluded_subdir = joinpath(TEMP_DIR, 'excluded_subdir/')
subdir = joinpath(TEMP_DIR, 'subdir/')

def create_file(fname, string):
with io.open(fname, 'w', encoding='utf-8') as infile:
infile.write(string)

def check_output_file(fname, str_exp):
with io.open(fname, 'r', encoding='utf-8') as infile:
self.assertEqual(str_exp, infile.read().strip())

try:
create_file(alien_file, instring)
create_file(excluded_file, instring)
create_file(config_file, conf_string)

# testing stdin --> stdout
# In this case, the config file will not be read,
# because it is not located in CWD.
p1 = subprocess.Popen(RUNSCRIPT,
stdout=subprocess.PIPE, stdin=subprocess.PIPE)
outstr = p1.communicate(instring.encode('UTF-8'))[0].decode('UTF-8')
self.assertEqual(outstring_without_config, outstr.strip())

# testing file --> stdout
p1 = subprocess.Popen([RUNSCRIPT, alien_file, '--stdout'],
stdout=subprocess.PIPE)
outstr = p1.communicate(instring.encode('UTF-8')[0])[0].decode('UTF-8')
self.assertEqual(outstring_with_config, outstr.strip())

# testing recursive mode
os.mkdir(subdir)
file_in_subdir = joinpath(subdir, 'aliens.F90')
create_file(file_in_subdir, instring)
config_file_in_subdir = joinpath(subdir, ".fprettify.rc")
# Config file in subdir should take precedence.
create_file(config_file_in_subdir, "case=[2,2,2,2]")

os.mkdir(excluded_subdir)
file_in_excluded_subdir = joinpath(excluded_subdir, 'aliens.F90')
create_file(file_in_excluded_subdir, instring)

p1 = subprocess.Popen([RUNSCRIPT, '--recursive', dirname],
stdout=subprocess.PIPE)
p1.wait()

check_output_file(alien_file, outstring_with_config)
# Excluded files and directories should not be touched at all.
Copy link
Author

Choose a reason for hiding this comment

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

This part of the test would fail before the fix.

check_output_file(excluded_file, instring)
check_output_file(file_in_excluded_subdir, instring)
# subdir contains a different config file which should take precedence.
check_output_file(file_in_subdir, outstring_without_config)

except: # pragma: no cover
self.removeTmpDir()
raise
else:
os.remove(alien_file)
self.removeTmpDir()

def test_multi_alias(self):
"""test for issue #11 (multiple alias and alignment)"""
Expand Down