5
\$\begingroup\$

To ensure there are no naming conflicts within our project (C++) I was tasked with writing a python script to check all of our header files for any occurrences of a using namespace ... within the file. If an occurrence is found, it's appended to a list then written to a log file. It's a fairly simple script, but I feel there could be some optimizations. This script is ran whenever someone commits to the repository.

"""
Checks all the header files in our project to ensure there
aren't occurrences of the namespace string.

AUTHOR: Ben Antonellis
DATE: April 4th, 2020
"""

import os

namespace: str = "using namespace"
working_directory: str = os.path.dirname(os.path.realpath(__file__));
occurrences: list = []

for file in os.listdir(working_directory):
    formatted_file = f"{working_directory}/{file}"
    with open(formatted_file, "r") as source_file:
        for line_number, line in enumerate(source_file):
            if namespace in line and file[-3:] != ".py":
                occurrences.append(f"NAMESPACE FOUND: LINE [{line_number + 1}] IN FILE {formatted_file}")

with open("logs/log.txt", "w") as log_file:
    for line in occurrences:
        log_file.write(line)
\$\endgroup\$

3 Answers 3

7
\$\begingroup\$

Working directory

os.path.dirname(os.path.realpath(__file__))

is not the current working directory. The current working directory is available via os.getcwd. Either you should call that instead, or maybe rename your variable.

Semicolons

are usually discouraged in Python, so you can drop one here:

working_directory: str = os.path.dirname(os.path.realpath(__file__));

pathlib

f"{working_directory}/{file}"

is better represented by making a Path and then using the / operator.

Overall

This approach is fragile. You're better off tapping into something like the LLVM/Clang AST, which actually understands how to parse all of the edge cases of C++.

Here is a suggestion that takes care of everything except the AST:

from pathlib import Path

NAMESPACE = 'using namespace'

log_file_name = Path('logs/log.txt')

if __name__ == '__main__':
    working_dir = Path.cwd()
    with log_file_name.open('w') as log_file:
        for prefix in ('h', 'c'):
            for file_name in working_dir.glob(f'*.{prefix}*'):
                with file_name.open() as source_file:
                    for line_number, line in enumerate(source_file):
                        if NAMESPACE in line:
                            log_file.write(f'NAMESPACE FOUND: LINE [{line_number + 1}] IN FILE {file_name}\n')

This also takes into account @AJNeufeld's feedback which is good.

\$\endgroup\$
2
  • \$\begingroup\$ Did you mean suffix instead of prefix? \$\endgroup\$ Commented Apr 6, 2020 at 11:40
  • \$\begingroup\$ Nope. It's a prefix of the extension. \$\endgroup\$
    – Reinderien
    Commented Apr 6, 2020 at 13:11
5
\$\begingroup\$

Your file[-3:] == ".py" check is later that it should be. It is part of the check executed for every line of a file, instead of only being done once per file. You should use:

for file in os.listdir(working_directory):
    if file[-3:] != ".py":
        ...

Are there other files in the directory? Maybe a README, Makefile.mak or .gitignore? Maybe you want to only examine .h files, and/or .hpp files, instead of every file in the directory?

valid_exts = { ".h", ".hpp"}
for file in os.listdir(working_directory):
    if os.path.splitext(file)[1] in valid_exts:
        ...

Using {line_number + 1} in your format string is not very pretty. Line numbers start at one, and enumerate() allows you to specify the starting number:

        for line_number, line in enumerate(source_file, 1):

Why accumulate the results in occurrences, and then write them out afterwords? Why not write them out as they are found?

with open("logs/log.txt", "w") as log_file:
    for file in os.listdir(working_directory):
        ...
\$\endgroup\$
4
\$\begingroup\$

Usage considerations

Given that file extensions actually mean nothing, and that a script that simply outputs lines with using namespace in its file argument (or stdin) would be more composable, I’d take the following approach:

Have the script read its arguments as files, or stdin if none given. Then just search for using namespace and output line numbers.

Hm, that sounds like grep... you could do

  1. git ls-files -z src | xargs -0 grep 'using namespace' > logfile
  2. git grep 'using namespace' src > logfile

And you probably need some grep flags to control the output you want.

\$\endgroup\$
2
  • \$\begingroup\$ Git was my invention. The OP just said "whenever someone commits to the repository". They could be using Mecurial, Subversion, or even CVS. But excellent alternate outside the Python box solution. \$\endgroup\$
    – AJNeufeld
    Commented Apr 6, 2020 at 2:07
  • \$\begingroup\$ @AJNeufeld yeah i realized after I was almost done that OP might not be on git. But this is the design I’d prefer a non-git interface to have in the long term anyway :) \$\endgroup\$ Commented Apr 6, 2020 at 3:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.