commit 9a497cce4fecf99e3503d062491bb4ae778b2874
author Martin Brænne <mboehme@google.com> Thu Aug 22 00:25:20 2024 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Aug 22 00:26:20 2024 -0700
tree 21d97e750c9db8d1bf7d44a37b1992f63e6cd943
parent d74fd07f7f90313123d6fcb5bc5bf6efa7e019cd

[nullability] Change range reported for invalid `->` dereference.

Currently, the source range that is reported for an invalid `->` dereference
encompasses the pointer expression that is potentially null. Some systems,
however, report only the beginning of the range; for chained dereferences such
as `p1->p2->field`, this makes it ambiguous which of the two dereferences is the
issue (and which of the pointers is potentially null).

We change this to instead report the source range of the `->` operator, which
makes it unambiguous where the issue lies.

PiperOrigin-RevId: 666219792
Change-Id: Iabd46ed95430c24e25ccb680c48cc40085ef3194

nullability/pointer_nullability_diagnosis.cc

diff --git a/nullability/pointer_nullability_diagnosis.cc b/nullability/pointer_nullability_diagnosis.cc
index 6dec0ab..2fce9ee 100644
--- a/nullability/pointer_nullability_diagnosis.cc
+++ b/nullability/pointer_nullability_diagnosis.cc
@@ -99,12 +99,15 @@
     absl::Nonnull<const Expr *> E, const Environment &Env,
     PointerNullabilityDiagnostic::Context DiagCtx,
     absl::Nullable<const clang::NamedDecl *> Callee = nullptr,
-    absl::Nullable<const clang::IdentifierInfo *> ParamName = nullptr) {
+    absl::Nullable<const clang::IdentifierInfo *> ParamName = nullptr,
+    CharSourceRange Range = {}) {
   if (PointerValue *ActualVal = getPointerValue(E, Env)) {
-    if (isNullable(*ActualVal, Env))
+    if (isNullable(*ActualVal, Env)) {
+      if (!Range.isValid())
+        Range = CharSourceRange::getTokenRange(E->getSourceRange());
       return {{PointerNullabilityDiagnostic::ErrorCode::ExpectedNonnull,
-               DiagCtx, CharSourceRange::getTokenRange(E->getSourceRange()),
-               Callee, ParamName}};
+               DiagCtx, Range, Callee, ParamName}};
+    }
     return {};
   }
 
@@ -164,7 +167,15 @@
     const MatchFinder::MatchResult &Result, const DiagTransferState &State) {
   return diagnoseNonnullExpected(
       MemberExpr->getBase(), State.Env,
-      PointerNullabilityDiagnostic::Context::NullableDereference);
+      PointerNullabilityDiagnostic::Context::NullableDereference,
+      /*Callee=*/nullptr, /*ParamName=*/nullptr,
+      // Attach the diagnostic to the source range of the `->` operator, rather
+      // than the source range of `MemberExpr->getBase()`.
+      // In a chain of dereferences, such as `p1->p2->field`, this ensures that
+      // the specific dereference that the diagnostic refers to is unambiguously
+      // clear, even if some system consuming the range only preserves the start
+      // of the range.
+      CharSourceRange::getTokenRange(MemberExpr->getOperatorLoc()));
 }
 
 SmallVector<PointerNullabilityDiagnostic> diagnoseAssignment(