[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)

[Julian Schmidt via cfe-commits]

================
@@ -0,0 +1,68 @@
+//===--- MathMissingParenthesesCheck.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 "MathMissingParenthesesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void MathMissingParenthesesCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(binaryOperator(unless(hasParent(binaryOperator())),
+                                    hasDescendant(binaryOperator()))
+                         .bind("binOp"),
+                     this);
+}
+
+void addParantheses(
+    const BinaryOperator *BinOp, const BinaryOperator *ParentBinOp,
+    bool &NeedToDiagnose,
+    std::vector<std::pair<clang::SourceLocation, clang::SourceLocation>>
+        &Insertions) {
+  if (!BinOp)
+    return;
+
+  if (ParentBinOp != nullptr &&
+      ParentBinOp->getOpcode() != BinOp->getOpcode()) {
+    NeedToDiagnose = true;
+  }
+  const clang::SourceLocation StartLoc = BinOp->getBeginLoc();
+  const clang::SourceLocation EndLoc = BinOp->getEndLoc().getLocWithOffset(1);
+  Insertions.push_back({StartLoc, EndLoc});
+  addParantheses(dyn_cast<BinaryOperator>(BinOp->getLHS()->IgnoreImpCasts()),
+                 BinOp, NeedToDiagnose, Insertions);
+  addParantheses(dyn_cast<BinaryOperator>(BinOp->getRHS()->IgnoreImpCasts()),
+                 BinOp, NeedToDiagnose, Insertions);
+}
+
+void MathMissingParenthesesCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binOp");
+  bool NeedToDiagnose = false;
+  std::vector<std::pair<clang::SourceLocation, clang::SourceLocation>>
+      Insertions;
+  addParantheses(BinOp, nullptr, NeedToDiagnose, Insertions);
+  const clang::SourceLocation StartLoc = BinOp->getBeginLoc();
+  const clang::SourceLocation EndLoc = BinOp->getEndLoc().getLocWithOffset(1);
+  const clang::SourceRange range(StartLoc, EndLoc);
+  if (!Insertions.empty()) {
+    Insertions.erase(Insertions.begin());
+  }
+  if (NeedToDiagnose) {
+    auto const &Diag = diag(
+        StartLoc, "add parantheses to clarify the precedence of operations");
+    for (const auto &Insertion : Insertions) {
+      Diag << FixItHint::CreateInsertion(Insertion.first, "(");
+      Diag << FixItHint::CreateInsertion(Insertion.second, ")");
----------------
5chmidti wrote:

You can stream a range into the diagnostic here, such that the affected operator is underlined. This would also make it easier to find and hit the range in an editor when using clangd.
Overlapping operators will just be a single line, and holes in the underline may occur (e.g., 2nd diagnostic).

| Without the range | With the range |
| --- | --- |
| ![image](https://github.com/llvm/llvm-project/assets/44101708/e19e01bb-786d-4c2a-9af7-66f156ca9cc2) | ![image](https://github.com/llvm/llvm-project/assets/44101708/fa758c46-e06d-4215-bd43-b71a3ca78c1c) |

The code I used to make the example: `Diag << SourceRange(Insertion.first, Insertion.second);`, and when you change the type that you use to track the insertion locations to a `SourceRange`, like Piotr suggested, then you can just use that (`Diag << Insertion`).

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