From 5308fb6dcc1e6c36c979991e632f8500918d5e53 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 26 Nov 2025 16:25:30 -0800 Subject: [PATCH] Nullability (internal): extend `checkDiagnostics` to allow inspecting diagnostics in tests. PiperOrigin-RevId: 837292611 --- bazel/llvm.bzl | 2 +- nullability/test/BUILD | 2 + nullability/test/check_diagnostics.cc | 50 ++++++++++---- nullability/test/check_diagnostics.h | 14 +++- nullability/test/check_diagnostics_test.cc | 76 ++++++++++++++++++++-- 5 files changed, 125 insertions(+), 19 deletions(-) diff --git a/bazel/llvm.bzl b/bazel/llvm.bzl index 3c196aecb..c008e9da6 100644 --- a/bazel/llvm.bzl +++ b/bazel/llvm.bzl @@ -53,7 +53,7 @@ def _llvm_loader_repository(repository_ctx): executable = False, ) -LLVM_COMMIT_SHA = "4f39a4ff0ada92870ca1c2dccad382ea04947da8" +LLVM_COMMIT_SHA = "0c2701fe7fa002e1befc5f86c268a7964f96d286" def llvm_loader_repository_dependencies(): # This *declares* the dependency, but it won't actually be *downloaded* unless it's used. diff --git a/nullability/test/BUILD b/nullability/test/BUILD index f368d1d8f..116efd282 100644 --- a/nullability/test/BUILD +++ b/nullability/test/BUILD @@ -36,6 +36,8 @@ cc_test( srcs = ["check_diagnostics_test.cc"], deps = [ ":check_diagnostics", + "//nullability:pointer_nullability_diagnosis", + "@llvm-project//third-party/unittest:gmock", "@llvm-project//third-party/unittest:gtest", "@llvm-project//third-party/unittest:gtest_main", ], diff --git a/nullability/test/check_diagnostics.cc b/nullability/test/check_diagnostics.cc index e1b6b2e4e..2cc0955d2 100644 --- a/nullability/test/check_diagnostics.cc +++ b/nullability/test/check_diagnostics.cc @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -36,25 +37,26 @@ namespace clang::tidy::nullability { -bool checkDiagnostics(ASTContext &AST, llvm::Annotations AnnotatedCode, - const NullabilityPragmas &Pragmas, bool AllowUntracked) { +static std::optional> +checkAndGetDiagnostics(ASTContext& AST, llvm::Annotations AnnotatedCode, + const NullabilityPragmas& Pragmas, bool AllowUntracked) { using ast_matchers::BoundNodes; using ast_matchers::hasAnyName; using ast_matchers::match; - using ast_matchers::stmt; using ast_matchers::valueDecl; SmallVector MatchResult = match(valueDecl(hasAnyName("target", "~target")).bind("target"), AST); if (MatchResult.empty()) { ADD_FAILURE() << "didn't find target declaration"; - return false; + return std::nullopt; } bool Success = true; // We already checked MatchResult is not empty above, but we skip // isTemplated() functions. Make sure we didn't skip every MatchResult. bool FoundMatch = false; + std::vector AllActualDiagnostics; for (const ast_matchers::BoundNodes &BN : MatchResult) { const auto *Target = BN.getNodeAs("target"); // Skip templates and only analyze instantiations @@ -71,7 +73,7 @@ bool checkDiagnostics(ASTContext &AST, llvm::Annotations AnnotatedCode, if (llvm::Error Err = diagnosePointerNullability(Target, Pragmas).moveInto(Diagnostics)) { ADD_FAILURE() << Err; - return false; + return std::nullopt; } // Note: use sorted sets for expected and actual lines to improve @@ -88,6 +90,7 @@ bool checkDiagnostics(ASTContext &AST, llvm::Annotations AnnotatedCode, ActualLines.insert(AST.getSourceManager().getPresumedLineNumber( Diag.Range.getBegin())); } + AllActualDiagnostics.push_back(Diag); } if (ActualLines != ExpectedLines) { Success = false; @@ -126,14 +129,21 @@ bool checkDiagnostics(ASTContext &AST, llvm::Annotations AnnotatedCode, if (!FoundMatch) { ADD_FAILURE() << "didn't find target declaration (after skipping templated " "functions) -- add an instantiation?"; - return false; + return std::nullopt; } - return Success; + return Success ? std::optional(AllActualDiagnostics) : std::nullopt; } -static bool checkDiagnostics(llvm::StringRef SourceCode, TestLanguage Lang, - bool AllowUntracked) { +bool checkDiagnostics(ASTContext& AST, llvm::Annotations AnnotatedCode, + const NullabilityPragmas& Pragmas, bool AllowUntracked) { + return checkAndGetDiagnostics(AST, AnnotatedCode, Pragmas, AllowUntracked) + .has_value(); +} + +static std::optional> +checkAndGetDiagnostics(llvm::StringRef SourceCode, TestLanguage Lang, + bool AllowUntracked) { llvm::Annotations AnnotatedCode(SourceCode); clang::TestInputs Inputs(AnnotatedCode.code()); Inputs.Language = Lang; @@ -163,8 +173,13 @@ static bool checkDiagnostics(llvm::StringRef SourceCode, TestLanguage Lang, return std::make_unique(Pragmas); }; clang::TestAST AST(Inputs); - return checkDiagnostics(AST.context(), AnnotatedCode, Pragmas, - AllowUntracked); + return checkAndGetDiagnostics(AST.context(), AnnotatedCode, Pragmas, + AllowUntracked); +} + +static bool checkDiagnostics(llvm::StringRef SourceCode, TestLanguage Lang, + bool AllowUntracked) { + return checkAndGetDiagnostics(SourceCode, Lang, AllowUntracked).has_value(); } // Run in C++17 and C++20 mode to cover differences in the AST between modes @@ -172,6 +187,19 @@ static bool checkDiagnostics(llvm::StringRef SourceCode, TestLanguage Lang, static constexpr std::array CXXLanguagesToTest = { TestLanguage::Lang_CXX17, TestLanguage::Lang_CXX20}; +std::optional> checkAndGetDiagnostics( + llvm::StringRef SourceCode) { + std::vector AllDiagnostics; + for (TestLanguage Lang : CXXLanguagesToTest) { + const auto Diagnostics = + checkAndGetDiagnostics(SourceCode, Lang, /*AllowUntracked=*/false); + if (!Diagnostics.has_value()) return std::nullopt; + AllDiagnostics.insert(AllDiagnostics.end(), Diagnostics->begin(), + Diagnostics->end()); + } + return AllDiagnostics; +} + bool checkDiagnostics(llvm::StringRef SourceCode) { for (TestLanguage Lang : CXXLanguagesToTest) if (!checkDiagnostics(SourceCode, Lang, /*AllowUntracked=*/false)) diff --git a/nullability/test/check_diagnostics.h b/nullability/test/check_diagnostics.h index 23e3d5be3..e13220c6f 100644 --- a/nullability/test/check_diagnostics.h +++ b/nullability/test/check_diagnostics.h @@ -5,6 +5,10 @@ #ifndef CRUBIT_NULLABILITY_TEST_CHECK_DIAGNOSTICS_H_ #define CRUBIT_NULLABILITY_TEST_CHECK_DIAGNOSTICS_H_ +#include +#include + +#include "nullability/pointer_nullability_diagnosis.h" #include "nullability/pragma.h" #include "clang/AST/ASTContext.h" #include "clang/Testing/CommandLineArgs.h" @@ -18,8 +22,6 @@ namespace nullability { /// Runs nullability verification on `SourceCode` and returns whether /// diagnostics are produced on those lines marked in the source code with /// `llvm::Annotations` style annotations (and no other lines). -/// TODO(mboehme): So far, we only check the locations of the diagnostics; it -/// would be desirable to check their actual content too. bool checkDiagnostics(llvm::StringRef SourceCode); /// Same as `checkDiagnostics`, but allows for untracked errors. @@ -31,6 +33,14 @@ bool checkDiagnostics(ASTContext& AST, llvm::Annotations AnnotatedCode, const NullabilityPragmas& Pragmas = NullabilityPragmas(), bool AllowUntracked = false); +/// Variation of `checkDiagnostics` which returns the ordered list of all actual +/// diagnostics if their locations match those in the source code annotations, +/// and otherwise returns `std::nullopt` to indicate a mismatch or failure. +/// Note that this iterates over the target C++ versions, so produces repeated +/// diagnostics if they occur in multiple versions. +std::optional> checkAndGetDiagnostics( + llvm::StringRef SourceCode); + } // namespace nullability } // namespace tidy } // namespace clang diff --git a/nullability/test/check_diagnostics_test.cc b/nullability/test/check_diagnostics_test.cc index 9bcf0f156..b6ebf35a8 100644 --- a/nullability/test/check_diagnostics_test.cc +++ b/nullability/test/check_diagnostics_test.cc @@ -6,28 +6,71 @@ #include "nullability/test/check_diagnostics.h" +#include +#include + +#include "nullability/pointer_nullability_diagnosis.h" +#include "external/llvm-project/third-party/unittest/googlemock/include/gmock/gmock.h" #include "external/llvm-project/third-party/unittest/googletest/include/gtest/gtest-spi.h" #include "external/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h" namespace clang::tidy::nullability { namespace { -TEST(PointerNullabilityTest, NoDiagnostics) { +using ::testing::AllOf; +using ::testing::ElementsAre; +using ::testing::Field; +using ::testing::IsEmpty; +using ::testing::Optional; + +TEST(PointerNullabilityTest, CheckNoDiagnostics) { EXPECT_TRUE(checkDiagnostics(R"cc( void target() {} )cc")); } -TEST(PointerNullabilityTest, ExpectedDiagnostic) { +TEST(PointerNullabilityTest, GetNoDiagnostics) { + EXPECT_THAT(checkAndGetDiagnostics(R"cc( + void target() {} + )cc"), + Optional(IsEmpty())); +} + +TEST(PointerNullabilityTest, CheckExpectedDiagnostic) { EXPECT_TRUE(checkDiagnostics(R"cc( void target() { - int *p = nullptr; + int* p = nullptr; *p; // [[unsafe]] } )cc")); } -TEST(PointerNullabilityTest, UnexpectedDiagnostic) { +TEST(PointerNullabilityTest, GetExpectedDiagnostic) { + EXPECT_THAT( + checkAndGetDiagnostics(R"cc( + void target() { + int* p = nullptr; + *p; // [[unsafe]] + } + )cc"), + Optional(ElementsAre( + AllOf( + Field("Code", &PointerNullabilityDiagnostic::Code, + PointerNullabilityDiagnostic::ErrorCode::ExpectedNonnull), + Field("Ctx", &PointerNullabilityDiagnostic::Ctx, + PointerNullabilityDiagnostic::Context::NullableDereference), + Field("NoteMessage", &PointerNullabilityDiagnostic::NoteMessage, + "")), + AllOf( + Field("Code", &PointerNullabilityDiagnostic::Code, + PointerNullabilityDiagnostic::ErrorCode::ExpectedNonnull), + Field("Ctx", &PointerNullabilityDiagnostic::Ctx, + PointerNullabilityDiagnostic::Context::NullableDereference), + Field("NoteMessage", &PointerNullabilityDiagnostic::NoteMessage, + ""))))); +} + +TEST(PointerNullabilityTest, CheckUnexpectedDiagnostic) { bool Result = true; EXPECT_NONFATAL_FAILURE(Result = checkDiagnostics(R"cc( void target() { @@ -38,7 +81,18 @@ TEST(PointerNullabilityTest, UnexpectedDiagnostic) { EXPECT_EQ(Result, false); } -TEST(PointerNullabilityTest, MissingDiagnostic) { +TEST(PointerNullabilityTest, GetUnexpectedDiagnostic) { + std::optional> Result; + EXPECT_NONFATAL_FAILURE(Result = checkAndGetDiagnostics(R"cc( + void target() { + 1; // [[unsafe]] + } + )cc"), + "Expected diagnostics but didn't find them"); + EXPECT_EQ(Result, std::nullopt); +} + +TEST(PointerNullabilityTest, CheckMissingDiagnostic) { bool Result = true; EXPECT_NONFATAL_FAILURE(Result = checkDiagnostics(R"cc( void target() { @@ -50,5 +104,17 @@ TEST(PointerNullabilityTest, MissingDiagnostic) { EXPECT_EQ(Result, false); } +TEST(PointerNullabilityTest, GetMissingDiagnostic) { + std::optional> Result; + EXPECT_NONFATAL_FAILURE(Result = checkAndGetDiagnostics(R"cc( + void target() { + int* p = nullptr; + *p; // Missing diagnostic + } + )cc"), + "Found diagnostics but didn't expect them"); + EXPECT_EQ(Result, std::nullopt); +} + } // namespace } // namespace clang::tidy::nullability