build: Run clang-tidy in default build
- Add the "static_analysis" group that runs clang-tidy to the default
build.
- Remove the "clang_tidy" presubmit step. Instead, have the
"static_analysis" step run clang-tidy and python.lint from GN.
- Fix a few clang-tidy warnings.
- Exclude mbedtls headers from checks.
Change-Id: Ied832e48c9ba46efaebb911ddb82ab0592944ee7
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/64624
Pigweed-Auto-Submit: Wyatt Hepler <hepler@google.com>
Reviewed-by: Rob Mohr <mohrr@google.com>
Commit-Queue: Wyatt Hepler <hepler@google.com>
diff --git a/.gn b/.gn
index f898dae..c46f2c8 100644
--- a/.gn
+++ b/.gn
@@ -17,4 +17,13 @@
default_args = {
pw_build_PIP_CONSTRAINTS =
[ "//pw_env_setup/py/pw_env_setup/virtualenv_setup/constraint.list" ]
+
+ # Exclude third-party headers from static analysis.
+ pw_toolchain_STATIC_ANALYSIS_SKIP_SOURCES_GLOBS = [ "**/boringssl/src/**/*" ]
+
+ pw_toolchain_STATIC_ANALYSIS_SKIP_INCLUDE_PATHS = [
+ "mbedtls/include",
+ "boringssl/src/include",
+ "boringssl",
+ ]
}
diff --git a/BUILD.gn b/BUILD.gn
index 6e15344..2604490 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -48,6 +48,7 @@
":docs",
":host",
":integration_tests",
+ ":static_analysis",
":stm32f429i",
"$dir_pw_env_setup:python.lint",
"$dir_pw_env_setup:python.tests",
@@ -126,8 +127,8 @@
# 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)" ]
+ _toolchain = "$dir_pigweed/targets/host:host_clang_debug"
+ deps = [ ":pigweed_default($_toolchain.static_analysis)" ]
}
group("docs") {
diff --git a/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py b/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
index f877e73..07a948a 100755
--- a/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
+++ b/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
@@ -21,7 +21,6 @@
import os
from pathlib import Path
import re
-import shutil
import subprocess
import sys
from typing import Sequence, IO, Tuple, Optional, Callable, List
@@ -465,60 +464,6 @@
json.dump(compile_commands, out_file, indent=2)
-@filter_paths(endswith=('.c', '.cc', '.cpp'), exclude=(r'^third_party/', ))
-def clang_tidy(ctx: PresubmitContext):
- """Run clang-tidy on source c and c++ files.
-
- Header files are indirectly analyzed when included. Source files will
- only be analyzed if built indirectly by the host_clang_debug target.
- """
- c_cpp_system_include_paths = ' '.join(_clang_system_include_paths('c++'))
- tested_files = set()
-
- # The compile commands are modified to explicitely specify the system
- # include paths, as clang-tidy is sometimes unable to find some standard
- # headers.
- # While the correct paths would be those produced by the invocation of the
- # command (which change based on toolchain and options flags), we are using
- # the default clang++ system paths instead to save on execution time.
- # This should not cause any issue with host_clang_debug while clang-analyzer
- # checks are disabled,.
- def _append_system_include_paths(file_path: str, _directory: str,
- command: str) -> str:
- tested_files.add(ctx.output_dir.joinpath(file_path).resolve())
- if '-nostdinc' in command: # catches -nostdinc and -nostdinc++
- return command
- return command + ' ' + c_cpp_system_include_paths
-
- _LOG.info('Generating compile commands')
- compile_commands = ctx.output_dir.joinpath('compile_commands.json')
- compile_commands_gn = ctx.output_dir.joinpath('compile_commands.gn.json')
- compile_commands_clang_tidy = ctx.output_dir.joinpath(
- 'compile_commands.clang-tidy.json')
-
- build.gn_gen(ctx.root,
- ctx.output_dir,
- export_compile_commands='host_clang_debug')
- shutil.copyfile(compile_commands, compile_commands_gn)
- edit_compile_commands(compile_commands_gn, compile_commands_clang_tidy,
- _append_system_include_paths)
- shutil.copyfile(compile_commands_clang_tidy, compile_commands)
-
- # Note: this step is reauired in case the built files depend on
- # generated dependencies.
- _LOG.info('Building host_clang_debug')
- build.ninja(ctx.output_dir, 'host_clang_debug')
-
- _LOG.info('Running clang-tidy')
- untested_files = frozenset([*ctx.paths]) - tested_files
- if len(untested_files) > 0:
- _LOG.warning('The following %d files will not be tested:\n %s',
- len(untested_files),
- '\n '.join(str(f) for f in untested_files))
- call('run-clang-tidy', f'-p={ctx.output_dir}', '-export-fixes',
- ctx.output_dir.joinpath('fixes.yaml'), *ctx.paths)
-
-
# The first line must be regex because of the '20\d\d' date
COPYRIGHT_FIRST_LINE = r'Copyright 20\d\d The Pigweed Authors'
COPYRIGHT_COMMENTS = r'(#|//| \*|REM|::)'
@@ -789,37 +734,11 @@
raise PresubmitFailure
+@filter_paths(endswith=(*format_code.C_FORMAT.extensions, '.py'))
def static_analysis(ctx: PresubmitContext):
- """Check that files pass static analyzer checks."""
- build.gn_gen(ctx.root,
- ctx.output_dir,
- export_compile_commands='host_clang_debug')
- build.ninja(ctx.output_dir, 'host_clang_debug')
- compile_commands = ctx.output_dir.joinpath('compile_commands.json')
- analyzer_output = ctx.output_dir.joinpath('analyze-build-output')
-
- if analyzer_output.exists():
- shutil.rmtree(analyzer_output)
-
- call('analyze-build',
- '--cdb',
- compile_commands,
- '--exclude',
- 'third_party',
- '--output',
- analyzer_output,
- cwd=ctx.root,
- env=build.env_with_clang_vars())
-
- # Search for reports under output directory.
- reports = list(analyzer_output.glob('*/report*'))
- if len(reports) != 0:
- archive = shutil.make_archive(str(analyzer_output), 'zip',
- reports[0].parent)
- _LOG.error('Static analyzer found errors: %s', archive)
- _LOG.error('To view report, open: %s',
- Path(reports[0]).parent.joinpath('index.html'))
- raise PresubmitFailure
+ """Runs all available static analysis tools."""
+ build.gn_gen(ctx.root, ctx.output_dir)
+ build.ninja(ctx.output_dir, 'python.lint', 'static_analysis')
def renode_check(ctx: PresubmitContext):
@@ -849,12 +768,10 @@
gn_clang_build,
gn_gcc_build,
renode_check,
- static_analysis,
stm32f429i,
)
_LINTFORMAT = (
- clang_tidy,
commit_message_format,
copyright_notice,
format_code.presubmit_checks(),
@@ -867,7 +784,7 @@
LINTFORMAT = (
_LINTFORMAT,
- python_checks.gn_python_lint,
+ static_analysis,
pw_presubmit.python_checks.check_python_versions,
)
diff --git a/pw_rpc/nanopb/public/pw_rpc/nanopb/client_call.h b/pw_rpc/nanopb/public/pw_rpc/nanopb/client_call.h
index 18ae943..d9745c4 100644
--- a/pw_rpc/nanopb/public/pw_rpc/nanopb/client_call.h
+++ b/pw_rpc/nanopb/public/pw_rpc/nanopb/client_call.h
@@ -28,6 +28,9 @@
// Non-templated nanopb base class providing protobuf encoding and decoding.
class BaseNanopbClientCall : public BaseClientCall {
public:
+ BaseNanopbClientCall(const BaseNanopbClientCall&) = delete;
+ BaseNanopbClientCall& operator=(const BaseNanopbClientCall&) = delete;
+
Status SendRequest(const void* request_struct);
protected:
@@ -54,9 +57,6 @@
constexpr BaseNanopbClientCall()
: BaseClientCall(), serde_(nullptr, nullptr) {}
- BaseNanopbClientCall(const BaseNanopbClientCall&) = delete;
- BaseNanopbClientCall& operator=(const BaseNanopbClientCall&) = delete;
-
BaseNanopbClientCall(BaseNanopbClientCall&&) = default;
BaseNanopbClientCall& operator=(BaseNanopbClientCall&&) = default;
diff --git a/pw_tls_client/public/pw_tls_client/session.h b/pw_tls_client/public/pw_tls_client/session.h
index a84d133..3840453 100644
--- a/pw_tls_client/public/pw_tls_client/session.h
+++ b/pw_tls_client/public/pw_tls_client/session.h
@@ -29,6 +29,11 @@
// Session provides APIs for performing TLS communication.
class Session : public stream::NonSeekableReaderWriter {
public:
+ Session() = delete;
+
+ Session(const Session&) = delete;
+ Session& operator=(const Session&) = delete;
+
// Resources allocated during Session::Create() will be released in the
// destructor. For example, backend may choose to allocate Session from a pool
// during Session::Create() and returns it in the destructor.
@@ -65,10 +70,6 @@
static Result<Session*> Create(const SessionOptions& option);
private:
- Session() = delete;
- Session(const Session&) = delete;
- Session& operator=(const Session&) = delete;
-
// A Session instance should only be created from Create().
Session(const SessionOptions& option);
diff --git a/pw_tls_client/test_server.cc b/pw_tls_client/test_server.cc
index a721816..7ec91d3 100644
--- a/pw_tls_client/test_server.cc
+++ b/pw_tls_client/test_server.cc
@@ -46,8 +46,8 @@
return Status::Internal();
}
- X509* x509 = d2i_X509_bio(bio, NULL);
- if (x509 == NULL) {
+ X509* x509 = d2i_X509_bio(bio, nullptr);
+ if (x509 == nullptr) {
PW_LOG_DEBUG("Failed to parse x509");
BIO_free(bio);
return Status::Internal();
@@ -171,7 +171,7 @@
}
// Requires PEM format.
- EVP_PKEY* pkey = d2i_PrivateKey_bio(bio, NULL);
+ EVP_PKEY* pkey = d2i_PrivateKey_bio(bio, nullptr);
int ret = SSL_CTX_use_PrivateKey(ctx_.get(), pkey);
if (ret != 1) {
BIO_free(bio);
diff --git a/pw_tls_client_mbedtls/tls_client_mbedtls.cc b/pw_tls_client_mbedtls/tls_client_mbedtls.cc
index 1e33937..14e5bcd 100644
--- a/pw_tls_client_mbedtls/tls_client_mbedtls.cc
+++ b/pw_tls_client_mbedtls/tls_client_mbedtls.cc
@@ -156,7 +156,7 @@
// Set up transport.
// The API does not fail.
- mbedtls_ssl_set_bio(&ssl_ctx_, this, MbedTlsWrite, MbedTlsRead, NULL);
+ mbedtls_ssl_set_bio(&ssl_ctx_, this, MbedTlsWrite, MbedTlsRead, nullptr);
ret = mbedtls_ssl_set_hostname(&ssl_ctx_,
session_options_.server_name().data());
