[clang-tools-extra] Enforce SL.con.3: Add check to replace operator[] with at() (PR #90043)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Sebastian Wolf (sebwolf-de)

<details>
<summary>Changes</summary>

The string based test to find out whether the check is applicable on the class is not ideal, but I did not find a more elegant way, yet. If there is more clang-query magic available, that I'm not aware of, I'm happy to adapt that.

---
Full diff: https://github.com/llvm/llvm-project/pull/90043.diff


8 Files Affected:

- (added) clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp (+81) 
- (added) clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h (+32) 
- (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt (+1) 
- (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp (+3) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) 
- (added) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst (+20) 
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) 
- (added) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp (+66) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
new file mode 100644
index 00000000000000..524c21b5bdb818
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp
@@ -0,0 +1,81 @@
+//===--- AvoidBoundsErrorsCheck.cpp - clang-tidy --------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AvoidBoundsErrorsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#include <iostream>
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+bool isApplicable(const QualType &Type) {
+  const auto TypeStr = Type.getAsString();
+  bool Result = false;
+  // Only check for containers in the std namespace
+  if (TypeStr.find("std::vector") != std::string::npos) {
+    Result = true;
+  }
+  if (TypeStr.find("std::array") != std::string::npos) {
+    Result = true;
+  }
+  if (TypeStr.find("std::deque") != std::string::npos) {
+    Result = true;
+  }
+  if (TypeStr.find("std::map") != std::string::npos) {
+    Result = true;
+  }
+  if (TypeStr.find("std::unordered_map") != std::string::npos) {
+    Result = true;
+  }
+  if (TypeStr.find("std::flat_map") != std::string::npos) {
+    Result = true;
+  }
+  // TODO Add std::span with C++26
+  return Result;
+}
+
+void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(callee(cxxMethodDecl(hasName("operator[]")).bind("f")))
+          .bind("x"),
+      this);
+}
+
+void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) {
+  const ASTContext &Context = *Result.Context;
+  const SourceManager &Source = Context.getSourceManager();
+  const auto *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("x");
+  const auto *MatchedFunction = Result.Nodes.getNodeAs<CXXMethodDecl>("f");
+  const auto Type = MatchedFunction->getThisType();
+  if (!isApplicable(Type)) {
+    return;
+  }
+
+  // Get original code.
+  const SourceLocation b(MatchedExpr->getBeginLoc());
+  const SourceLocation e(MatchedExpr->getEndLoc());
+  const std::string OriginalCode =
+      Lexer::getSourceText(CharSourceRange::getTokenRange(b, e), Source,
+                           getLangOpts())
+          .str();
+  const auto Range = SourceRange(b, e);
+
+  // Build replacement.
+  std::string NewCode = OriginalCode;
+  const auto BeginOpen = NewCode.find("[");
+  NewCode.replace(BeginOpen, 1, ".at(");
+  const auto BeginClose = NewCode.find("]");
+  NewCode.replace(BeginClose, 1, ")");
+
+  diag(MatchedExpr->getBeginLoc(), "Do not use operator[], use at() instead.")
+      << FixItHint::CreateReplacement(Range, NewCode);
+}
+
+} // namespace clang::tidy::cppcoreguidelines
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
new file mode 100644
index 00000000000000..f915729cd7bbee
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidBoundsErrorsCheck.h - clang-tidy ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::cppcoreguidelines {
+
+/// Enforce CPP core guidelines SL.con.3
+///
+/// See
+/// https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.html
+class AvoidBoundsErrorsCheck : public ClangTidyCheck {
+public:
+  AvoidBoundsErrorsCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::cppcoreguidelines
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
index eb35bbc6a538fe..4972d20417928d 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
+  AvoidBoundsErrorsCheck.cpp
   AvoidCapturingLambdaCoroutinesCheck.cpp
   AvoidConstOrRefDataMembersCheck.cpp
   AvoidDoWhileCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
index e9f0201615616f..525bbc7a42adaa 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -19,6 +19,7 @@
 #include "../performance/NoexceptMoveConstructorCheck.h"
 #include "../performance/NoexceptSwapCheck.h"
 #include "../readability/MagicNumbersCheck.h"
+#include "AvoidBoundsErrorsCheck.h"
 #include "AvoidCapturingLambdaCoroutinesCheck.h"
 #include "AvoidConstOrRefDataMembersCheck.h"
 #include "AvoidDoWhileCheck.h"
@@ -57,6 +58,8 @@ namespace cppcoreguidelines {
 class CppCoreGuidelinesModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<AvoidBoundsErrorsCheck>(
+        "cppcoreguidelines-avoid-bounds-errors");
     CheckFactories.registerCheck<AvoidCapturingLambdaCoroutinesCheck>(
         "cppcoreguidelines-avoid-capturing-lambda-coroutines");
     CheckFactories.registerCheck<modernize::AvoidCArraysCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5b1feffb89ea06..1949f18a586ed8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -131,6 +131,11 @@ New checks
   to reading out-of-bounds data due to inadequate or incorrect string null
   termination.
 
+- New :doc:`cppcoreguidelines-avoid-bounds-errors
+  <clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors>` check.
+
+  Flags the unsafe `operator[]` and replaces it with `at()`.
+
 - New :doc:`modernize-use-designated-initializers
   <clang-tidy/checks/modernize/use-designated-initializers>` check.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
new file mode 100644
index 00000000000000..8fb2e3bfde0981
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst
@@ -0,0 +1,20 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-bounds-errors
+
+cppcoreguidelines-avoid-bounds-errors
+=====================================
+
+This check enforces the `SL.con.3 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors>` guideline.
+It flags all uses of `operator[]` on `std::vector`, `std::array`, `std::deque`, `std::map`, `std::unordered_map`, and `std::flat_map` and suggests to replace it with `at()`.
+Note that `std::span` and `std::mdspan` do not support `at()` as of C++23, so the use of `operator[]` is not flagged.
+
+For example the code
+
+.. code-block:: c++
+  std::array<int, 3> a;
+  int b = a[4];
+
+will be replaced by 
+
+.. code-block:: c++
+  std::vector<int, 3> a;
+  int b = a.at(4);
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5d9d487f75f9cb..8b472fca2d8ca1 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -174,6 +174,7 @@ Clang-Tidy Checks
    :doc:`cert-oop58-cpp <cert/oop58-cpp>`,
    :doc:`concurrency-mt-unsafe <concurrency/mt-unsafe>`,
    :doc:`concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous>`,
+   :doc:`cppcoreguidelines-avoid-bounds-errors <cppcoreguidelines/avoid-bounds-errors>`, "Yes"
    :doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines <cppcoreguidelines/avoid-capturing-lambda-coroutines>`,
    :doc:`cppcoreguidelines-avoid-const-or-ref-data-members <cppcoreguidelines/avoid-const-or-ref-data-members>`,
    :doc:`cppcoreguidelines-avoid-do-while <cppcoreguidelines/avoid-do-while>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
new file mode 100644
index 00000000000000..23453b1f2df218
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp
@@ -0,0 +1,66 @@
+namespace std {
+  template<typename T, unsigned size>
+  struct array {
+    T operator[](unsigned i) {
+      return T{1};
+    }
+    T at(unsigned i) {
+      return T{1};
+    }
+  };
+
+  template<typename T>
+  struct unique_ptr {
+    T operator[](unsigned i) {
+      return T{1};
+    }
+  };
+
+  template<typename T>
+  struct span {
+    T operator[](unsigned i) {
+      return T{1};
+    }
+  };
+} // namespace std
+
+namespace json {
+  template<typename T>
+  struct node{
+    T operator[](unsigned i) {
+      return T{1};
+    }
+  };
+} // namespace json
+
+
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-bounds-errors %t
+std::array<int, 3> a;
+
+auto b = a[0];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
+// CHECK-FIXES: auto b = a.at(0);
+auto c = a[1+1];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
+// CHECK-FIXES: auto c = a.at(1+1);
+constexpr int index = 1;
+auto d = a[index];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
+// CHECK-FIXES: auto d = a.at(index);
+
+int e(int index) {
+  return a[index];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Do not use operator[], use at() instead. [cppcoreguidelines-avoid-bounds-errors]
+// CHECK-FIXES: return a.at(index);
+}
+
+auto f = a.at(0);
+
+std::unique_ptr<int> p;
+auto q = p[0];
+
+std::span<int> s;
+auto t = s[0];
+
+json::node<int> n;
+auto m = n[0];

``````````

</details>


https://github.com/llvm/llvm-project/pull/90043