build: GN static_analysis polish
Some python fixes and documentation updates.
- Refine the implementation of the source filter
to match relative paths only.
Bug: 45
Change-Id: Ia38f651599d32d46fe2a720a5c9f4a5f5ec233c3
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/59286
Commit-Queue: Henri Chataing <henrichataing@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
diff --git a/BUILD.gn b/BUILD.gn
index a86c9c8..3daab36 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -125,6 +125,8 @@
}
# Run clang-tidy on pigweed_default with host_clang_debug toolchain options.
+# Make sure to invoke gn clean out when any relevant .clang-tidy
+# file is updated.
group("static_analysis") {
_toolchain_prefix = "$dir_pigweed/targets/host:host_clang_"
deps = [ ":pigweed_default(${_toolchain_prefix}debug.static_analysis)" ]
diff --git a/pw_toolchain/docs.rst b/pw_toolchain/docs.rst
index f13362c..39bd70c 100644
--- a/pw_toolchain/docs.rst
+++ b/pw_toolchain/docs.rst
@@ -87,8 +87,8 @@
toolchain ``${target_name}.static_analysis`` using
``pw_generate_static_analysis_toolchain`` and the toolchain options.
-The build argument ``pw_toolchain_STATIC_ANALYSIS_SOURCE_FILTER`` may be
-used to select and/or exclude source files to output diagnostics from.
+The build argument ``pw_toolchain_STATIC_ANALYSIS_SOURCE_EXCLUDE`` may be
+used to exclude source files from the analysis.
``pw_toolchain`` provides static analysis GN toolchains that may be used to
test host targets:
@@ -98,6 +98,9 @@
- pw_toolchain_host_clang.size_optimized.static_analysis
- pw_toolchain_host_clang.fuzz.static_analysis
(if pw_toolchain_OSS_FUZZ_ENABLED is false)
+ - pw_toolchain_arm_clang.debug.static_analysis
+ - pw_toolchain_arm_clang.speed_optimized.static_analysis
+ - pw_toolchain_arm_clang.size_optimized.static_analysis
For example, to run ``clang-tidy`` on all source dependencies of the
``default`` target:
@@ -112,3 +115,9 @@
group("static_analysis") {
deps = [ ":default(my_toolchain.static_analysis)" ]
}
+
+.. warning::
+ The status of the static analysis checks might change when
+ any relevant .clang-tidy file is updated. You should
+ clean the output directory before invoking
+ ``clang-tidy``.
diff --git a/pw_toolchain/py/pw_toolchain/clang_tidy.py b/pw_toolchain/py/pw_toolchain/clang_tidy.py
index 3b7387e..7bfdc99 100644
--- a/pw_toolchain/py/pw_toolchain/clang_tidy.py
+++ b/pw_toolchain/py/pw_toolchain/clang_tidy.py
@@ -16,22 +16,18 @@
Implements additional features compared to directly calling
clang-tidy:
- - add option `--source-filter` to select which sources to run the
- clang-tidy on. Similar to `--header-filter` implemented by
- clang-tidy.
+ - add option `--source-exclude` to exclude matching sources from the
+ clang-tidy analysis.
- inputs the full compile command, with the cc binary name
- TODO: infer platform options from the full compile command
"""
import argparse
import logging
-import pathlib
-import re
+from pathlib import Path
import subprocess
import sys
-
-from pathlib import Path
-from typing import List, Optional
+from typing import List, Optional, Union
_LOG = logging.getLogger(__name__)
@@ -49,20 +45,33 @@
default='clang-tidy',
help='Path to clang-tidy executable.')
- parser.add_argument('--source-file',
- required=True,
- help='Source file to analyze with clang-tidy.')
+ parser.add_argument(
+ '--source-file',
+ required=True,
+ type=Path,
+ help='Path to the source file to analyze with clang-tidy.')
+ parser.add_argument(
+ '--source-root',
+ required=True,
+ type=Path,
+ help=('Path to the root source directory.'
+ ' The relative path from the root directory is matched'
+ ' against source filter rather than the absolute path.'))
+ parser.add_argument(
+ '--export-fixes',
+ required=False,
+ type=Path,
+ help=('YAML file to store suggested fixes in. The '
+ 'stored fixes can be applied to the input source '
+ 'code with clang-apply-replacements.'))
- parser.add_argument('--export-fixes',
- required=False,
- help='YAML file to store suggested fixes in. The ' +
- 'stored fixes can be applied to the input source ' +
- 'code with clang-apply-replacements.')
-
- parser.add_argument('--source-filter',
- default='.*',
- help='Regular expression matching the names of' +
- ' the source files to output diagnostics from')
+ parser.add_argument('--source-exclude',
+ default=[],
+ action='append',
+ type=str,
+ help=('Glob-style patterns matching the paths of'
+ ' source files to be excluded from the'
+ ' analysis.'))
# Add a silent placeholder arg for everything that was left over.
parser.add_argument('extra_args',
@@ -71,7 +80,9 @@
parsed_args = parser.parse_args()
- assert parsed_args.extra_args[0] == '--', 'arguments not correctly split'
+ if parsed_args.extra_args[0] != '--':
+ parser.error('arguments not correctly split')
+
parsed_args.extra_args = parsed_args.extra_args[1:]
return parsed_args
@@ -79,28 +90,31 @@
def run_clang_tidy(clang_tidy: str, verbose: bool, source_file: Path,
export_fixes: Optional[Path], extra_args: List[str]) -> int:
"""Executes clang_tidy via subprocess. Returns true if no failures."""
- command = [clang_tidy, str(source_file)]
+ command: List[Union[str, Path]] = [clang_tidy, source_file, '--use-color']
if not verbose:
command.append('--quiet')
if export_fixes is not None:
- command.extend(['--export-fixes', str(export_fixes)])
+ command.extend(['--export-fixes', export_fixes])
# Append extra compilation flags. extra_args[0] is skipped as it contains
# the compiler binary name.
command.append('--')
command.extend(extra_args[1:])
- process = subprocess.run(command,
- stdout=subprocess.PIPE,
- stderr=subprocess.PIPE)
+ process = subprocess.run(
+ command,
+ stdout=subprocess.PIPE,
+ # clang-tidy prints regular information on
+ # stderr, even with the option --quiet.
+ stderr=subprocess.PIPE)
if process.stdout:
- _LOG.warning(process.stdout.decode('utf-8').strip())
+ _LOG.warning(process.stdout.decode().strip())
if process.stderr and process.returncode != 0:
- _LOG.error(process.stderr.decode('utf-8').strip())
+ _LOG.error(process.stderr.decode().strip())
return process.returncode
@@ -108,18 +122,27 @@
def main(
verbose: bool,
clang_tidy: str,
- source_file: str,
- export_fixes: Optional[str],
- source_filter: str,
+ source_file: Path,
+ source_root: Path,
+ export_fixes: Optional[Path],
+ source_exclude: List[str],
extra_args: List[str],
) -> int:
+ # Rebase the source file path on source_root.
+ # If source_file is not relative to source_root (which may be the case for
+ # generated files) stick with the original source_file.
+ try:
+ relative_source_file = source_file.relative_to(source_root)
+ except ValueError:
+ relative_source_file = source_file
- if not re.search(source_filter, source_file):
- return 0
+ for pattern in source_exclude:
+ if relative_source_file.match(pattern):
+ return 0
- source_file_path = pathlib.Path(source_file).resolve()
- export_fixes_path = pathlib.Path(
- export_fixes).resolve() if export_fixes is not None else None
+ source_file_path = source_file.resolve()
+ export_fixes_path = (export_fixes.resolve()
+ if export_fixes is not None else None)
return run_clang_tidy(clang_tidy, verbose, source_file_path,
export_fixes_path, extra_args)
diff --git a/pw_toolchain/static_analysis_toolchain.gni b/pw_toolchain/static_analysis_toolchain.gni
index f175642..6912486 100644
--- a/pw_toolchain/static_analysis_toolchain.gni
+++ b/pw_toolchain/static_analysis_toolchain.gni
@@ -17,15 +17,15 @@
import("$dir_pw_toolchain/universal_tools.gni")
declare_args() {
- # Regular expression matching the names of the source files to output
- # diagnostics from. The option is not provided by clang-tidy, even
- # though --header-filter is. Example:
+ # Glob-style patterns matching the paths of the source files to be excluded
+ # from the analysis. clang-tidy provides no alternative option.
+ # Example:
#
# gn gen out \
- # --args='pw_toolchain_STATIC_ANALYSIS_SOURCE_FILTER="^((?!third_party).)*$"'
+ # --args='pw_toolchain_STATIC_ANALYSIS_SOURCE_EXCLUDE=["third_party/.*"]'
#
# disables clang-tidy on all files in the third_party directory.
- pw_toolchain_STATIC_ANALYSIS_SOURCE_FILTER = ".*"
+ pw_toolchain_STATIC_ANALYSIS_SOURCE_EXCLUDE = []
}
# Creates a toolchain target for static analysis.
@@ -41,9 +41,14 @@
# Clang tidy is invoked by a wrapper script which implements the missing
# option --source-filter.
_clang_tidy_py_path =
- rebase_path("$dir_pw_toolchain/py/pw_toolchain/clang_tidy.py")
+ rebase_path("$dir_pw_toolchain/py/pw_toolchain/clang_tidy.py",
+ root_build_dir)
_clang_tidy_py = "${python_path} ${_clang_tidy_py_path}"
- _source_filter = pw_toolchain_STATIC_ANALYSIS_SOURCE_FILTER
+ _source_root = rebase_path("//", root_build_dir)
+ _source_exclude = ""
+ foreach(pattern, pw_toolchain_STATIC_ANALYSIS_SOURCE_EXCLUDE) {
+ _source_exclude = _source_exclude + " --source-exclude '${pattern}'"
+ }
toolchain(target_name) {
# Uncomment this line to see which toolchains generate other toolchains.
@@ -68,8 +73,9 @@
command = string_join(" ",
[
_clang_tidy_py,
- "--source-filter '${_source_filter}'",
+ _source_exclude,
"--source-file {{source}}",
+ "--source-root '${_source_root}'",
"--export-fixes {{output}}.yaml",
"--",
invoker.cc,
@@ -93,8 +99,9 @@
command = string_join(" ",
[
_clang_tidy_py,
- "--source-filter '${_source_filter}'",
+ _source_exclude,
"--source-file {{source}}",
+ "--source-root '${_source_root}'",
"--export-fixes {{output}}.yaml",
"--",
invoker.cxx,
