[clang-tidy] new check: bugprone-branch-clone

Summary:
Implement a check for detecting if/else if/else chains where two or more
branches are Type I clones of each other (that is, they contain identical code)
and for detecting switch statements where two or more consecutive branches are
Type I clones of each other.

Patch by donat.nagy.

Reviewers: alexfh, hokein, aaron.ballman, JonasToth

Reviewed By: JonasToth

Subscribers: MTC, lebedev.ri, whisperity, xazax.hun, Eugene.Zelenko, mgorny, rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D54757

3 files changed, 98 insertions, 0 deletions

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index cec9645fe65..385730443a0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -122,6 +122,13 @@ Improvements to clang-tidy
   Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests
   ``absl::StrAppend()`` should be used instead.
 
+- New :doc:`bugprone-branch-clone
+  <clang-tidy/checks/bugprone-branch-clone>` check.
+
+  Checks for repeated branches in ``if/else if/else`` chains, consecutive
+  repeated branches in ``switch`` statements and indentical true and false
+  branches in conditional operators.
+
 - New :doc:`bugprone-too-small-loop-variable
   <clang-tidy/checks/bugprone-too-small-loop-variable>` check.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-branch-clone.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-branch-clone.rst
new file mode 100644
index 00000000000..b4c07b4d1a7
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-branch-clone.rst
@@ -0,0 +1,90 @@
+.. title:: clang-tidy - bugprone-branch-clone
+
+bugprone-branch-clone
+=====================
+
+Checks for repeated branches in ``if/else if/else`` chains, consecutive
+repeated branches in ``switch`` statements and indentical true and false
+branches in conditional operators.
+
+.. code-block:: c++
+
+    if (test_value(x)) {
+      y++;
+      do_something(x, y);
+    } else {
+      y++;
+      do_something(x, y);
+    }
+
+In this simple example (which could arise e.g. as a copy-paste error) the
+``then`` and ``else`` branches are identical and the code is equivalent the
+following shorter and cleaner code:
+
+.. code-block:: c++
+
+    test_value(x); // can be omitted unless it has side effects
+    y++;
+    do_something(x, y);
+
+
+If this is the inteded behavior, then there is no reason to use a conditional
+statement; otherwise the issue can be solved by fixing the branch that is
+handled incorrectly.
+
+The check also detects repeated branches in longer ``if/else if/else`` chains
+where it would be even harder to notice the problem.
+
+In ``switch`` statements the check only reports repeated branches when they are
+consecutive, because it is relatively common that the ``case:`` labels have
+some natural ordering and rearranging them would decrease the readability of
+the code. For example:
+
+.. code-block:: c++
+
+    switch (ch) {
+    case 'a':
+      return 10;
+    case 'A':
+      return 10;
+    case 'b':
+      return 11;
+    case 'B':
+      return 11;
+    default:
+      return 10;
+    }
+
+Here the check reports that the ``'a'`` and ``'A'`` branches are identical
+(and that the ``'b'`` and ``'B'`` branches are also identical), but does not
+report that the ``default:`` branch is also idenical to the first two branches.
+If this is indeed the correct behavior, then it could be implemented as:
+
+.. code-block:: c++
+
+    switch (ch) {
+    case 'a':
+    case 'A':
+      return 10;
+    case 'b':
+    case 'B':
+      return 11;
+    default:
+      return 10;
+    }
+
+Here the check does not warn for the repeated ``return 10;``, which is good if
+we want to preserve that ``'a'`` is before ``'b'`` and ``default:`` is the last
+branch.
+
+Finally, the check also examines conditional operators and reports code like:
+
+.. code-block:: c++
+
+    return test_value(x) ? x : x;
+
+Unlike if statements, the check does not detect chains of conditional
+operators.
+
+Note: This check also reports situations where branches become identical only
+after preprocession.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index e3528ae90e8..7b810f5954b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -31,6 +31,7 @@ Clang-Tidy Checks
    bugprone-argument-comment
    bugprone-assert-side-effect
    bugprone-bool-pointer-implicit-conversion
+   bugprone-branch-clone
    bugprone-copy-constructor-init
    bugprone-dangling-handle
    bugprone-exception-escape