return False
-class IncludeFileChecker(object):
- def __init__(self, deps, options):
- self._deps = deps
- self._options = options
-
- def _check_file(self, checkfile, reporter):
- if not self._options.check_doc:
- return
- if not checkfile.is_documented():
- if self._options.warn_undoc:
- is_legacy = _is_legacy_module(checkfile.module)
- is_external = checkfile.module.name in ('gmx_lapack', 'gmx_blas', 'thread_mpi')
- if not is_legacy and not is_external:
- reporter.error(checkfile.path, 'file not documented')
- elif checkfile.doctype == 'implementation' and \
- checkfile.type in ('publicheader', 'libheader'):
- reporter.error(checkfile.path,
- 'file documentation visibility incorrect')
- elif checkfile.doctype == 'library' and checkfile.type == 'publicheader':
- reporter.error(checkfile.path,
- 'file documentation visibility incorrect')
- elif checkfile.installed and checkfile.doctype not in ('public', 'unknown'):
- reporter.error(checkfile.path,
- 'installed header has no public documentation')
- elif not checkfile.installed and checkfile.doctype == 'public':
- reporter.error(checkfile.path,
- 'non-installed file has public documentation')
- selfmodfullname = checkfile.module.fullname
- docmodule = checkfile.docmodule
- if docmodule and \
- not selfmodfullname.startswith('module_' + docmodule) and \
- not selfmodfullname.startswith('module_gromacs_' + docmodule) and \
- not checkfile.name == docmodule + '.h':
- reporter.error(checkfile.path,
- 'file documented in incorrect module "{0}"'
- .format(docmodule))
-
- def _check_included_file(self, checkfile, includedfile, reporter):
- otherfile = includedfile._included_file
- if includedfile._is_system:
- # TODO: This doesn't report errors with files not listed in
- # the input files, although those could be included.
- # That would produce a massive amount of errors for <config.h>.
- if otherfile:
- reporter.error(checkfile.path,
- 'local file included as <{0}>'
- .format(includedfile._included_path))
- elif not includedfile._is_relative and checkfile.installed:
- if not includedfile._included_path == 'gmx_header_config_gen.h':
- reporter.error(checkfile.path,
- 'installed header includes "{0}", '
- 'which is not found using relative path'
- .format(includedfile._included_path))
- if not otherfile:
- return
- if checkfile.installed and not otherfile.installed:
- reporter.error(checkfile.path,
- 'installed header includes '
- 'non-installed header "{0}"'
- .format(includedfile._included_path))
- if not otherfile.is_documented():
- return
- if not self._options.check_doc:
- return
- intramodule = \
- (checkfile.module.get_top_level_module() == \
- otherfile.module.get_top_level_module())
- if otherfile.type not in ('publicheader', 'libheader', 'test'):
- if not intramodule and not _is_legacy_module(otherfile.module):
- reporter.error(checkfile.path,
- 'included file "{0}" is missing API definition'
- .format(otherfile.path))
- elif checkfile.type == 'publicheader':
- if not otherfile.type == 'publicheader' and not otherfile.doctype == 'public':
- reporter.error(checkfile.path,
- 'public API file includes non-public header "{0}"'
- .format(otherfile.path))
-
- def check_all(self, reporter):
- for checkfile in sorted(self._deps.files.values()):
- self._check_file(checkfile, reporter)
- for includedfile in checkfile.get_included_files():
- self._check_included_file(checkfile, includedfile, reporter)
-
-
class GraphBuilder(object):
def __init__(self, deps):
self._deps = deps
#parser.add_option('--external-at-bottom', action='store_true',
# help='Force external dependencies files at the bottom '
# 'of the graph')
- parser.add_option('--check', action='store_true',
- help='Check for problems in include file dependencies')
- parser.add_option('--check-doc', action='store_true',
- help='Check for problems in Doxygen documentation')
- parser.add_option('--warn-undoc', action='store_true',
- help='Warn for files that do not have Doxygen documentation')
parser.add_option('--left-to-right', action='store_true',
help='Lay out from left to right')
parser.add_option('--file-graph',
help='Write file graphs for each module')
options, args = parser.parse_args()
- if not options.file_graph and not options.module_graph and \
- not options.module_file_graphs:
- options.check = True
-
# Constructs lists of files
filelist = []
ignorelist = []
deps.scan_files(ignorelist, reporter)
- if options.check or options.check_doc:
- checker = IncludeFileChecker(deps, options)
- checker.check_all(reporter)
-
#if options.with_external:
# for filename in extrafiles:
# file = files[filename]
endif()
if (USE_PYTHON_SCRIPTS)
+ # TODO: Consider whether this is the best name and location for this
+ # code, since not all of it is Doxygen-specific (but nearly all of it
+ # relies on the Doxygen XML documentation).
set(doc_check_command
${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/doxygen-check.py
-S ${CMAKE_SOURCE_DIR} -B ${CMAKE_BINARY_DIR}
-DMODE=GRAPHS
-P ${CMAKE_SOURCE_DIR}/admin/includedeps.cmake
COMMENT "Generating include dependency graphs" VERBATIM)
- # TODO: This should be moved away from here and preferably made into
- # a CTest test.
- # TODO: To do the above, issues found by the script should be fixed.
- # A suppression system would also be good to have, since it would be
- # nice to include also legacyheaders in the checks, but those generate
- # a lot of errors about missing documentation. The script is also
- # currently quite strict, so I don't know whether we really want to
- # enforce all the rules.
- # TODO: Would be nice to not generate the file lists for each of these,
- # but what would be a good shared location for the intermediate files?
- add_custom_target(depcheck
- ${CMAKE_COMMAND}
- -DSRCDIR=${CMAKE_SOURCE_DIR}
- -DBUILDDIR=${CMAKE_BINARY_DIR}
- -DOUTDIR=${CMAKE_BINARY_DIR}/doxygen/depgraphs
- -DPYTHON_EXECUTABLE=${PYTHON_EXECUTABLE}
- -DMODE=CHECK
- -P ${CMAKE_SOURCE_DIR}/admin/includedeps.cmake
- COMMENT "Checking include dependencies" VERBATIM)
- add_custom_target(doccheck
- ${CMAKE_COMMAND}
- -DSRCDIR=${CMAKE_SOURCE_DIR}
- -DBUILDDIR=${CMAKE_BINARY_DIR}
- -DOUTDIR=${CMAKE_BINARY_DIR}/doxygen/depgraphs
- -DPYTHON_EXECUTABLE=${PYTHON_EXECUTABLE}
- -DMODE=CHECKDOC
- -P ${CMAKE_SOURCE_DIR}/admin/includedeps.cmake
- COMMENT "Checking some aspects of Doxygen documentation" VERBATIM)
endif()
"is not documented in any module, but {0} exists"
.format(expectedmod.get_name()))
+def check_include(fileobj, includedfile, reporter):
+ """Check an #include directive."""
+ if includedfile.is_system():
+ if includedfile.get_file():
+ reporter.code_issue(includedfile,
+ "includes local file as {0}".format(includedfile))
+ else:
+ otherfile = includedfile.get_file()
+ if not otherfile:
+ reporter.code_issue(includedfile,
+ "includes non-local file as {0}".format(includedfile))
+ elif fileobj.is_installed() and not includedfile.is_relative():
+ reporter.code_issue(includedfile,
+ "installed header includes {0} using non-relative path"
+ .format(includedfile))
+ if not otherfile:
+ return
+ if fileobj.is_installed() and not otherfile.is_installed():
+ reporter.code_issue(includedfile,
+ "installed header includes non-installed {0}"
+ .format(includedfile))
+ filemodule = fileobj.get_module()
+ othermodule = otherfile.get_module()
+ if fileobj.is_documented() and otherfile.is_documented():
+ filetype = fileobj.get_documentation_type()
+ othertype = otherfile.get_documentation_type()
+ if filetype > othertype:
+ reporter.code_issue(includedfile,
+ "{0} file includes {1} file {2}"
+ .format(filetype, othertype, includedfile))
+ check_api = (othermodule and othermodule.is_documented() and
+ filemodule != othermodule)
+ if check_api and otherfile.get_api_type() < DocType.library:
+ reporter.code_issue(includedfile,
+ "included file {0} is not documented as exposed outside its module"
+ .format(includedfile))
+
def check_entity(entity, reporter):
"""Check documentation for a code construct."""
if entity.is_documented():
sys.stderr.write('Scanning source tree...\n')
tree = GromacsTree(options.source_root, options.build_root, reporter)
tree.set_installed_file_list(installedlist)
+ if not options.quiet:
+ sys.stderr.write('Reading source files...\n')
+ tree.scan_files()
if not options.quiet:
sys.stderr.write('Reading Doxygen XML files...\n')
tree.load_xml()
for fileobj in tree.get_files():
check_file(fileobj, reporter)
+ for includedfile in fileobj.get_includes():
+ check_include(fileobj, includedfile, reporter)
for classobj in tree.get_classes():
check_class(classobj, reporter)
if memberobj.is_visible() or options.check_ignored:
check_member(memberobj, reporter)
- # TODO: Check #include statements, like old 'make doccheck'
-
reporter.write_pending()
reporter.report_unused_filters()
reporter.close_log()
Automatic dependency checking
-----------------------------
-The build system provides three targets, `depcheck`, `doccheck`, and
-`depgraphs`, that depend on correct usage of the above commands, in particular
-the visibility and API definitions in file-level comments.
-These checks also provide some level of enforcement for rules about
-dependencies between the modules, but currently the checks are not run
-automatically.
-
-The `depcheck` target checks for issues that either completely or subtly break
-the installed headers: an installed header including a non-installed header, or
-an installed header installing another header using a path that does not
-resolve correctly when the headers are installed. It also checks for
-consistent usage of
-\code
-#include "..." // This should be used for Gromacs headers
-\endcode
-versus
-\code
-#include <...> // This should be used for system and external headers.
-\endcode
-
-The `doccheck` target additionally checks that if an included file is
-documented, it is not used against the API definition specified in the
-file-level comment. For example, including a file without any API definition
-from another module produces an error. Another example of a check is that a
-non-installed header with public documentation triggers an error.
-
-The `depgraphs` target generates include dependency graphs. One graph is
+The build system provides a `doc-check` target that automatically checks some
+aspects of the documentation, as well as checking that header files are
+actually used according to their API specifications (i.e., an internal headers
+are not included from other modules). The checks depend on correct usage of
+the commands listed above, in particular the visibility and API definitions in
+file-level comments. These checks also provide some level of enforcement for
+rules about dependencies between the modules, but currently the checks are not
+run automatically.
+
+The checker currently checks for a few different types of issues:
+* For all Doxygen documentation (currently does not apply for members within
+ anonymous namespaces or members that do not appear in the documentation):
+ * If a member has documentation, it should have a brief description.
+ * A note is issued for in-body documentation for functions, since this is
+ ignored by our current settings.
+ * If a class has documentation, it should have public documentation only if
+ it appears in an installed header.
+ * If a class and its containing file has documentation, the class
+ documentation should not be visible if the file documentation is not.
+* For all files:
+ * Consistent usage of
+
+ #include "..." // This should be used for Gromacs headers
+
+ and
+
+ #include <...> // This should be used for system and external headers
+
+ * Installed headers must not include non-installed headers, and must include
+ all other \Gromacs headers using relative paths so that they resolve
+ correctly also when installed.
+* For documented files:
+ * Installed headers should have public documentation, and other files should
+ not.
+ * The API level specified for a file should not be higher than where its
+ documentation is visible. For example, only publicly documented headers
+ should be specified as part of the public API.
+ * If an \c \\ingroup module_foo exists, it should match the subdirectory
+ that the file is actually part of in the file system.
+ * If a \c \\defgroup module_foo exists for the subdirectory where the file is,
+ the file should contain \c \\ingroup module_foo.
+ * Files should not include other files whose documentation visibility is
+ lower (if the included file is not documented, the check is skipped).
+* For files that are part of documented modules
+ (\c \\defgroup module_foo exists for the subdirectory):
+ * Such files should not be included from outside their module if they are
+ undocumented or are not specified as part of library or public API.
+
+The checker is based on extracting the Doxygen documentation in XML format.
+This information is then read using a Python script, and combined with
+information extracted from the file system and knowledge about the \Gromacs
+source tree layout. The Python scripts are in the `doxygen/` folder.
+In addition to printing the issues, they are also written into
+`doxygen/doxygen-check.log` for later inspection.
+
+The script is not currently perfect (either because of unfinished
+implementation, or because Doxygen bugs or incompleteness of the Doxygen XML
+output), and the current code also contains issues that the script detects, but
+the authors have not fixed. To allow the script to still be used,
+`doxygen/suppressions.txt` contains a list of issues that are filtered out from
+the report. The syntax is simple:
+
+ <file>: <text>
+
+where `<file>` is a path to the file that reports the message, and `<text>` is
+the text reported. Both support `*` as a wildcard. If `<file>` is empty, the
+suppression matches only messages that do not have an associated file.
+`<file>` is matched against the trailing portion of the file name to make it
+work even though the script reports absolute paths.
+Empty lines and lines starting with `#` are ignored.
+
+To add suppression for an issue, the line that reports the issue can be copied
+into `suppressios.txt`, and the line number (if any) removed. If the
+issue does not have a file name (or a pseudo-file) associated, a leading `:`
+must be added. To cover many similar issues, parts of the line can then be
+replaced with wildcards.
+
+For some false positives from the script, the suppression mechanism is the
+easiest way to silence the script, but otherwise the goal would be to minimize
+the number of suppressions.
+
+As a side effect, the XML extraction makes Doxygen parse all comments in the
+code, even if they do not appear in the documentation. This can reveal latent
+issues in the comments, like invalid Doxygen syntax. The messages from the XML
+parsing are stored in `doxygen/doxygen-xml.log` in the build tree, similar to
+other Doxygen runs.
+
+There is an additional target `depgraphs` that generates include dependency
+graphs. This is in the process of getting rewritten to take advantage of the
+mechanisms used for `doc-check`. One graph is
produced that shows all the modules under `src/gromacs/`, and their include
dependencies. Additionally, a file-level graph is produced for each module,
showing the include dependencies within that module. Currently, these are
currently included as comments in `admin/includedeps.py`. The output is put in
`doxygen/depgraphs/`.
-These targets require Python 2.7; the graph generation additionally requires
-`graphviz`.
+These targets require Python 2.7 (other versions may work, but have not been
+tested); the graph generation additionally requires `graphviz`.
Documenting specific code constructs
"""Documentation visibility in the generated documentation."""
# Mapping to string representations for the internal integer values
- _names = ['none', 'internal', 'library', 'public']
+ _names = ['undocumented', 'internal', 'library', 'public']
def __init__(self, value):
"""Initialize a DocType instance.
import os
import os.path
+import re
import doxygenxml as xml
import reporter
# TODO: Check for multiple group membership
return result
+class IncludedFile(object):
+
+ """Information about an #include directive in a file."""
+
+ def __init__(self, path, lineno, included_file, included_path, is_relative, is_system):
+ self._path = path
+ self._line_number = lineno
+ self._included_file = included_file
+ self._included_path = included_path
+ #self._used_include_path = used_include_path
+ self._is_relative = is_relative
+ self._is_system = is_system
+
+ def __str__(self):
+ if self._is_system:
+ return '<{0}>'.format(self._included_path)
+ else:
+ return '"{0}"'.format(self._included_path)
+
+ def is_system(self):
+ return self._is_system
+
+ def is_relative(self):
+ return self._is_relative
+
+ def get_file(self):
+ return self._included_file
+
+ def get_reporter_location(self):
+ return reporter.Location(self._path, self._line_number)
+
class File(object):
"""Source/header file in the GROMACS tree."""
self._sourcefile = (extension in ('.c', '.cc', '.cpp', '.cu'))
self._apitype = DocType.none
self._modules = set()
+ self._includes = []
def set_doc_xml(self, rawdoc, sourcetree):
"""Assiociate Doxygen documentation entity with the file."""
"""Mark the file installed."""
self._installed = True
+ def _process_include(self, lineno, is_system, includedpath, sourcetree):
+ """Process #include directive during scan()."""
+ is_relative = False
+ if is_system:
+ fileobj = sourcetree.find_include_file(includedpath)
+ else:
+ fullpath = os.path.join(self._dir.get_path(), includedpath)
+ fullpath = os.path.abspath(fullpath)
+ if os.path.exists(fullpath):
+ is_relative = True
+ fileobj = sourcetree.get_file(fullpath)
+ else:
+ fileobj = sourcetree.find_include_file(includedpath)
+ self._includes.append(IncludedFile(self.get_path(), lineno, fileobj, includedpath,
+ is_relative, is_system))
+
+ def scan_contents(self, sourcetree):
+ """Scan the file contents and initialize information based on it."""
+ # TODO: Consider a more robust regex.
+ include_re = r'^#\s*include\s+(?P<quote>["<])(?P<path>[^">]*)[">]'
+ with open(self._path, 'r') as scanfile:
+ for lineno, line in enumerate(scanfile, 1):
+ match = re.match(include_re, line)
+ if match:
+ is_system = (match.group('quote') == '<')
+ includedpath = match.group('path')
+ self._process_include(lineno, is_system, includedpath,
+ sourcetree)
+
def get_reporter_location(self):
return reporter.Location(self._path, None)
def is_installed(self):
return self._installed
+ def is_external(self):
+ return self._dir.is_external()
+
def is_source_file(self):
return self._sourcefile
def get_doc_modules(self):
return self._modules
+ def get_module(self):
+ module = self.get_expected_module()
+ if not module and len(self._modules) == 1:
+ module = list(self._modules)[0]
+ return module
+
+ def get_includes(self):
+ return self._includes
+
class GeneratedFile(File):
pass
if parent and parent.is_test_directory() or \
os.path.basename(path) in ('tests', 'legacytests'):
self._is_test_dir = True
+ self._is_external = False
+ if parent and parent.is_external() or \
+ os.path.basename(path) == 'external':
+ self._is_external = True
self._subdirs = set()
if parent:
parent._subdirs.add(self)
def get_reporter_location(self):
return reporter.Location(self._path, None)
+ def get_path(self):
+ return self._path
+
def is_test_directory(self):
return self._is_test_dir
+ def is_external(self):
+ return self._is_external
+
def get_module(self):
if self._module:
return self._module
set_installed_file_list() can be called to set the list of installed
files.
+ scan_files() can be called to read all the files and initialize #include
+ dependencies between the files based on the information. This is done like
+ this instead of relying on Doxygen-extracted include files to make the
+ dependency graph independent from preprocessor macro definitions
+ (Doxygen only sees those #includes that the preprocessor sees, which
+ depends on what #defines it has seen).
+
load_xml() can be called to load information from Doxygen XML data in
the build tree (the Doxygen XML data must have been built separately).
"""
continue
relpath = self._get_rel_path(fullpath)
self._dirs[relpath] = Directory(fullpath, currentdir)
- extensions = ('.h', '.cuh', '.hpp', '.c', '.cc', '.cpp', '.cu')
+ extensions = ('.h', '.cuh', '.hpp', '.c', '.cc', '.cpp', '.cu', '.bm')
for filename in filenames:
basename, extension = os.path.splitext(filename)
if extension in extensions:
rootdir.set_module(moduleobj)
self._modules[name] = moduleobj
+ def scan_files(self):
+ """Read source files to initialize #include dependencies."""
+ for fileobj in self._files.itervalues():
+ if not fileobj.is_external():
+ fileobj.scan_contents(self)
+
def load_xml(self):
"""Load Doxygen XML information."""
xmldir = os.path.join(self._build_root, 'doxygen', 'xml')
"""Get directory object for a path relative to source tree root."""
return self._dirs.get(relpath)
+ def get_file(self, path):
+ """Get file object for a path relative to source tree root."""
+ return self._files.get(self._get_rel_path(path))
+
+ def find_include_file(self, includedpath):
+ """Find a file object corresponding to an include path."""
+ for testdir in ('src', 'src/gromacs/legacyheaders', 'src/external/thread_mpi/include'):
+ testpath = os.path.join(testdir, includedpath)
+ if testpath in self._files:
+ return self._files[testpath]
+
def set_installed_file_list(self, installedfiles):
"""Set list of installed files."""
for path in installedfiles:
src/gromacs/imd/imd.c: error: source file documentation appears outside full documentation
src/gromacs/imd/imdsocket.c: error: source file documentation appears outside full documentation
src/gromacs/imd/imd.h: note: init_IMD: has in-body comments, which are ignored
+src/gromacs/legacyheaders/types/*.h: warning: included file "../../swap/enums.h" is not documented as exposed outside its module
+
+# These would be nice to fix, but can wait for later
+*: warning: includes local file as <config.h>
+src/programs/view/*: warning: includes local file as <*.h>
+src/gromacs/gmxlib/nonbonded/nb_kernel_*/*: warning: included file "gromacs/simd/math_x86_*.h" is not documented as exposed outside its module
+src/gromacs/gmxlib/copyrite.cpp: warning: included file "gromacs/utility/gitversion.h" is not documented as exposed outside its module
+
+# These are specific to Folding@Home, and easiest to suppress here
+*: warning: includes non-local file as "corewrap.h"
+src/config.h.cmakein: warning: includes non-local file as "swindirect.h"
+
+# These are limitations in the current script
+src/gromacs/version.h.cmakein: warning: includes local file as <gromacs/version.h>
+src/gromacs/utility/gmx_header_config.h: warning: includes non-local file as "gmx_header_config_gen.h"
biod.pnpi.spb.ru / alexxy / gromacs.git / commitdiff