Adding upstream version 124.0.1.upstream/124.0.1

Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>

1 files changed, 148 insertions, 0 deletions

diff --git a/docs/code-quality/static-analysis/writing-new/advanced-check-features.rst b/docs/code-quality/static-analysis/writing-new/advanced-check-features.rst
new file mode 100644
index 0000000000..e8adcf664f
--- /dev/null
+++ b/docs/code-quality/static-analysis/writing-new/advanced-check-features.rst
@@ -0,0 +1,148 @@
+.. _advanced_check_features:
+
+Advanced Check Features
+=======================
+
+This page covers additional ways to improve and extend the check you've added to build/clang-plugin.
+
+Adding Tests
+------------
+
+No doubt you've seen the tests for existing checks in `build/clang-plugin/tests <https://searchfox.org/mozilla-central/source/build/clang-plugin/tests>`_. Adding tests is straightforward; and your reviewer should insist you do so. Simply copying the existing format of any test and how diagnostics are marked as expected.
+
+One wrinkle - all clang plugin checks are applied to all tests. We try to write tests so that only one check applies to it.  If you write a check that triggers on an existing test, try to fix the existing test slightly so the new check does not trigger on it.
+
+Using Bind To Output More Useful Information
+--------------------------------------------
+
+You've probably been wondering what the heck ``.bind()`` is for. You've been seeing it all over the place but never has it actually been explained what it's for and when to use it.
+
+``.bind()`` is used to give a name to part of the AST discovered through your matcher, so you can use it later.  Let's go back to our sample matcher:
+
+::
+
+  AstMatcher->addMatcher(
+    traverse(TK_IgnoreUnlessSpelledInSource,
+      ifStmt(allOf(
+              has(
+                   binaryOperator(
+                       has(
+                           declRefExpr(hasType(enumDecl()))
+                       )
+                   )
+               ),
+               hasElse(
+                   ifStmt(allOf(
+                      unless(hasElse(anything())),
+                      has(
+                           binaryOperator(
+                               has(
+                                   declRefExpr(hasType(enumDecl()))
+                               )
+                           )
+                       )
+                   ))
+              )
+           ))
+          .bind("node")),
+      this);
+
+Now the ``.bind("node")`` makes more sense. We're naming the If statement we matched, so we can refer to it later when we call ``Result.Nodes.getNodeAs<IfStmt>("node")``.
+
+Let's say we want to provide the *type* of the enum in our warning message.  There are two enums we end up seeing in our matcher - the enum in the first if statement, and the enum in the second.  We're going to arbitrarily pick the first and give it the name ``enumType``:
+
+::
+
+  AstMatcher->addMatcher(
+    traverse(TK_IgnoreUnlessSpelledInSource,
+      ifStmt(allOf(
+              has(
+                   binaryOperator(
+                       has(
+                           declRefExpr(hasType(enumDecl().bind("enumType")))
+                       )
+                   )
+               ),
+               hasElse(
+                   ifStmt(allOf(
+                      unless(hasElse(anything())),
+                      has(
+                           binaryOperator(
+                               has(
+                                   declRefExpr(hasType(enumDecl()))
+                               )
+                           )
+                       )
+                   ))
+              )
+           ))
+          .bind("node")),
+      this);
+
+And in our check() function, we can use it like so:
+
+::
+
+  void MissingElseInEnumComparisons::check(
+      const MatchFinder::MatchResult &Result) {
+    const auto *MatchedDecl = Result.Nodes.getNodeAs<IfStmt>("node");
+    const auto *EnumType = Result.Nodes.getNodeAs<EnumDecl>("enumType");
+
+    diag(MatchedDecl->getIfLoc(),
+         "Enum comparisons to %0 in an if/else if block without a trailing else.",
+         DiagnosticIDs::Warning) << EnumType->getName();
+  }
+
+Repeated matcher calls
+--------------------------
+
+If you find yourself repeating the same several matchers in several spots, you can turn it into a variable to use.
+
+::
+
+  auto isTemporaryLifetimeBoundCall =
+      cxxMemberCallExpr(
+          onImplicitObjectArgument(anyOf(has(cxxTemporaryObjectExpr()),
+                                         has(materializeTemporaryExpr()))),
+          callee(functionDecl(isMozTemporaryLifetimeBound())));
+
+  auto hasTemporaryLifetimeBoundCall =
+      anyOf(isTemporaryLifetimeBoundCall,
+            conditionalOperator(
+                anyOf(hasFalseExpression(isTemporaryLifetimeBoundCall),
+                      hasTrueExpression(isTemporaryLifetimeBoundCall))));
+
+The above example is parameter-less, but if you need to supply a parameter that changes, you can turn it into a lambda:
+
+::
+
+  auto hasConstCharPtrParam = [](const unsigned int Position) {
+    return hasParameter(
+        Position, hasType(hasCanonicalType(pointsTo(asString("const char")))));
+  };
+
+  auto hasParamOfType = [](const unsigned int Position, const char *Name) {
+    return hasParameter(Position, hasType(asString(Name)));
+  };
+
+  auto hasIntegerParam = [](const unsigned int Position) {
+    return hasParameter(Position, hasType(isInteger()));
+  };
+
+  AstMatcher->addMatcher(
+      callExpr(
+        hasName("fopen"),
+        hasConstCharPtrParam(0))
+          .bind("funcCall"),
+      this);
+
+
+Allow-listing existing callsites
+--------------------------------
+
+While it's not a great situation, you can set up an allow-list of existing callsites if you need to.  A simple allow-list is demonstrated in `NoGetPrincipalURI <https://hg.mozilla.org/mozilla-central/rev/fb60b22ee6616521b386d90aec07b03b77905f4e>`_. The `NoNewThreadsChecker <https://hg.mozilla.org/mozilla-central/rev/f400f164b3947b4dd54089a36ea31cca2d72805b>`_ is an example of a more sophisticated way of setting up a larger allow-list.
+
+
+Custom Annotations
+------------------
+It's possible to create custom annotations that will be a no-op when compiled, but can be used by a static analysis check. These can be used to annotate special types of sources and sinks (for example).  We have some examples of this in-tree presently (such as ``MOZ_CAN_RUN_SCRIPT``) but currently don't have a detailed walkthrough in this documentation of how to set these up and use them. (Patches welcome.)