[clang-tools-extra] 8ce99da - [clang-tidy] Add more documentation about check development (NFC)

[via cfe-commits]

Author: Richard
Date: 2022-01-27T09:44:09-07:00
New Revision: 8ce99dadb007dd0dbbf1ddbe4090e9ff43e780c5

URL: https://github.com/llvm/llvm-project/commit/8ce99dadb007dd0dbbf1ddbe4090e9ff43e780c5
DIFF: https://github.com/llvm/llvm-project/commit/8ce99dadb007dd0dbbf1ddbe4090e9ff43e780c5.diff

LOG: [clang-tidy] Add more documentation about check development (NFC)

- Mention pp-trace
- CMake configuration
- Overriding registerPPCallbacks
- Overriding isLanguageVersionSupported
- Check development tips
  - Guide to useful documentation
  - Using the Transformer library
  - Developing your check incrementally
  - Creating private matchers
  - Unit testing helper code
  - Making your check robust
  - Documenting your check
- Describe the Inputs test folder

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

Added: 
    

Modified: 
    clang-tools-extra/docs/clang-tidy/Contributing.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/docs/clang-tidy/Contributing.rst b/clang-tools-extra/docs/clang-tidy/Contributing.rst
index 7ebfac3123603..b9eb0e7627cc1 100644
--- a/clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ b/clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -22,6 +22,8 @@ There are a few tools particularly useful when developing clang-tidy checks:
     check, it will create the check, update the CMake file and create a test;
   * ``rename_check.py`` does what the script name suggests, renames an existing
     check;
+  * :program:`pp-trace` logs method calls on `PPCallbacks` for a source file
+    and is invaluable in understanding the preprocessor mechanism;
   * :program:`clang-query` is invaluable for interactive prototyping of AST
     matchers and exploration of the Clang AST;
   * `clang-check`_ with the ``-ast-dump`` (and optionally ``-ast-dump-filter``)
@@ -70,6 +72,14 @@ let's start!
 .. _Using Clang Tools: https://clang.llvm.org/docs/ClangTools.html
 .. _How To Setup Clang Tooling For LLVM: https://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
 
+When you `configure the CMake build <https://llvm.org/docs/GettingStarted.html#local-llvm-configuration>`_,
+make sure that you enable the ``clang`` and ``clang-tools-extra`` projects to
+build :program:`clang-tidy`.
+Because your new check will have associated documentation, you will also want to install
+`Sphinx <https://www.sphinx-doc.org/en/master/>`_ and enable it in the CMake configuration.
+To save build time of the core Clang libraries you may want to only enable the ``X86``
+target in the CMake configuration.
+
 
 The Directory Structure
 -----------------------
@@ -215,11 +225,215 @@ can further inspect them and report diagnostics.
 and `clang-tidy/google/ExplicitConstructorCheck.cpp
 <https://reviews.llvm.org/
diff usion/L/browse/clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp>`_).
 
+If you need to interact with macros or preprocessor directives, you will want to
+override the method ``registerPPCallbacks``.  The ``add_new_check.py`` script
+does not generate an override for this method in the starting point for your
+new check.
+
+If your check applies only under a specific set of language options, be sure
+to override the method ``isLanguageVersionSupported`` to reflect that.
+
+Check development tips
+----------------------
+
+Writing your first check can be a daunting task, particularly if you are unfamiliar
+with the LLVM and Clang code bases.  Here are some suggestions for orienting yourself
+in the codebase and working on your check incrementally.
+
+Guide to useful documentation
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Many of the support classes created for LLVM are used by Clang, such as `StringRef
+<https://llvm.org/docs/ProgrammersManual.html#the-stringref-class>`_
+and `SmallVector <https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h>`_.
+These and other commonly used classes are described in the `Important and useful LLVM APIs
+<https://llvm.org/docs/ProgrammersManual.html#important-and-useful-llvm-apis>`_ and
+`Picking the Right Data Structure for the Task
+<https://llvm.org/docs/ProgrammersManual.html#picking-the-right-data-structure-for-a-task>`_
+sections of the `LLVM Programmer's Manual
+<https://llvm.org/docs/ProgrammersManual.html>`_.  You don't need to memorize all the
+details of these classes; the generated `doxygen documentation <https://llvm.org/doxygen/>`_
+has everything if you need it.  In the header `LLVM/ADT/STLExtras.h
+<https://llvm.org/doxygen/STLExtras_8h.html>`_ you'll find useful versions of the STL
+algorithms that operate on LLVM containers, such as `llvm::all_of
+<https://llvm.org/doxygen/STLExtras_8h.html#func-members>`_.
+
+Clang is implemented on top of LLVM and introduces its own set of classes that you
+will interact with while writing your check.  When a check issues diagnostics and
+fix-its, these are associated with locations in the source code.  Source code locations,
+source files, ranges of source locations and the `SourceManager
+<https://clang.llvm.org/doxygen/classclang_1_1SourceManager.html>`_ class provide
+the mechanisms for describing such locations.  These and
+other topics are described in the `"Clang" CFE Internals Manual
+<https://clang.llvm.org/docs/InternalsManual.html>`_.  Whereas the doxygen generated
+documentation serves as a reference to the internals of Clang, this document serves
+as a guide to other developers.  Topics in that manual of interest to a check developer
+are:
+
+- `The Clang "Basic" Library
+  <https://clang.llvm.org/docs/InternalsManual.html#the-clang-basic-library>`_ for
+  information about diagnostics, fix-it hints and source locations.
+- `The Lexer and Preprocessor Library
+  <https://clang.llvm.org/docs/InternalsManual.html#the-lexer-and-preprocessor-library>`_
+  for information about tokens, lexing (transforming characters into tokens) and the
+  preprocessor.
+- `The AST Library
+  <https://clang.llvm.org/docs/InternalsManual.html#the-lexer-and-preprocessor-library>`_
+  for information about how C++ source statements are represented as an abstract syntax
+  tree (AST).
+
+Most checks will interact with C++ source code via the AST.  Some checks will interact
+with the preprocessor.  The input source file is lexed and preprocessed and then parsed
+into the AST.  Once the AST is fully constructed, the check is run by applying the check's
+registered AST matchers against the AST and invoking the check with the set of matched
+nodes from the AST.  Monitoring the actions of the preprocessor is detached from the
+AST construction, but a check can collect information during preprocessing for later
+use by the check when nodes are matched by the AST.
+
+Every syntactic (and sometimes semantic) element of the C++ source code is represented by
+
diff erent classes in the AST.  You select the portions of the AST you're interested in
+by composing AST matcher functions.  You will want to study carefully the `AST Matcher
+Reference <https://clang.llvm.org/docs/LibASTMatchersReference.html>`_ to understand
+the relationship between the 
diff erent matcher functions.
+
+Using the Transformer library
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The Transformer library allows you to write a check that transforms source code by
+expressing the transformation as a ``RewriteRule``.  The Transformer library provides
+functions for composing edits to source code to create rewrite rules.  Unless you need
+to perform low-level source location manipulation, you may want to consider writing your
+check with the Transformer library.  The `Clang Transformer Tutorial
+<https://clang.llvm.org/docs/ClangTransformerTutorial.html>`_ describes the Transformer
+library in detail.
+
+To use the Transformer library, make the following changes to the code generated by
+the ``add_new_check.py`` script:
+
+- Include ``../utils/TransformerClangTidyCheck.h`` instead of ``../ClangTidyCheck.h``
+- Change the base class of your check from ``ClangTidyCheck`` to ``TransformerClangTidyCheck``
+- Delete the override of the ``registerMatchers`` and ``check`` methods in your check class.
+- Write a function that creates the ``RewriteRule`` for your check.
+- Call the function in your check's constructor to pass the rewrite rule to
+  ``TransformerClangTidyCheck``'s constructor.
+
+Developing your check incrementally
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The best way to develop your check is to start with the simple test cases and increase
+complexity incrementally.  The test file created by the ``add_new_check.py`` script is
+a starting point for your test cases.  A rough outline of the process looks like this:
+
+- Write a test case for your check.
+- Prototype matchers on the test file using :program:`clang-query`.
+- Capture the working matchers in the ``registerMatchers`` method.
+- Issue the necessary diagnostics and fix-its in the ``check`` method.
+- Add the necessary ``CHECK-MESSAGES`` and ``CHECK-FIXES`` annotations to your
+  test case to validate the diagnostics and fix-its.
+- Build the target ``check-clang-tool`` to confirm the test passes.
+- Repeat the process until all aspects of your check are covered by tests.
+
+The quickest way to prototype your matcher is to use :program:`clang-query` to
+interactively build up your matcher.  For complicated matchers, build up a matching
+expression incrementally and use :program:`clang-query`'s ``let`` command to save named
+matching expressions to simplify your matcher.  Just like breaking up a huge function
+into smaller chunks with intention-revealing names can help you understand a complex
+algorithm, breaking up a matcher into smaller matchers with intention-revealing names
+can help you understand a complicated matcher.  Once you have a working matcher, the
+C++ API will be virtually identical to your interactively constructed matcher.  You can
+use local variables to preserve your intention-revealing names that you applied to
+nested matchers.
+
+Creating private matchers
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Sometimes you want to match a specific aspect of the AST that isn't provided by the
+existing AST matchers.  You can create your own private matcher using the same
+infrastructure as the public matchers.  A private matcher can simplify the processing
+in your ``check`` method by eliminating complex hand-crafted AST traversal of the
+matched nodes.  Using the private matcher allows you to select the desired portions
+of the AST directly in the matcher and refer to it by a bound name in the ``check``
+method.
+
+Unit testing helper code
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+Private custom matchers are a good example of auxiliary support code for your check
+that can be tested with a unit test.  It will be easier to test your matchers or
+other support classes by writing a unit test than by writing a ``FileCheck`` integration
+test.  The ``ASTMatchersTests`` target contains unit tests for the public AST matcher
+classes and is a good source of testing idioms for matchers.
+
+Making your check robust
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+Once you've covered your check with the basic "happy path" scenarios, you'll want to
+torture your check with as many edge cases as you can cover in order to ensure your
+check is robust.  Running your check on a large code base, such as Clang/LLVM, is a
+good way to catch things you forgot to account for in your matchers.  However, the
+LLVM code base may be insufficient for testing purposes as it was developed against a
+particular set of coding styles and quality measures.  The larger the corpus of code
+the check is tested against, the higher confidence the community will have in the
+check's efficacy and false positive rate.
+
+Some suggestions to ensure your check is robust:
+
+- Create header files that contain code matched by your check.
+- Validate that fix-its are properly applied to test header files with
+  :program:`clang-tidy`.  You will need to perform this test manually until
+  automated support for checking messages and fix-its is added to the
+  ``check_clang_tidy.py`` script.
+- Define macros that contain code matched by your check.
+- Define template classes that contain code matched by your check.
+- Define template specializations that contain code matched by your check.
+- Test your check under both Windows and Linux environments.
+- Watch out for high false positive rates.  Ideally, a check would have no false
+  positives, but given that matching against an AST is not control- or data flow-
+  sensitive, a number of false positives are expected.  The higher the false
+  positive rate, the less likely the check will be adopted in practice.
+  Mechanisms should be put in place to help the user manage false positives.
+- There are two primary mechanisms for managing false positives: supporting a
+  code pattern which allows the programmer to silence the diagnostic in an ad
+  hoc manner and check configuration options to control the behavior of the check.
+- Consider supporting a code pattern to allow the programmer to silence the
+  diagnostic whenever such a code pattern can clearly express the programmer's
+  intent.  For example, allowing an explicit cast to ``void`` to silence an
+  unused variable diagnostic.
+- Consider adding check configuration options to allow the user to opt into
+  more aggressive checking behavior without burdening users for the common
+  high-confidence cases.
+
+Documenting your check
+^^^^^^^^^^^^^^^^^^^^^^
+
+The ``add_new_check.py`` script creates entries in the
+`release notes <https://clang.llvm.org/extra/ReleaseNotes.html>`_, the list of
+checks and a new file for the check documentation itself.  It is recommended that you
+have a concise summation of what your check does in a single sentence that is repeated
+in the release notes, as the first sentence in the doxygen comments in the header file
+for your check class and as the first sentence of the check documentation.  Avoid the
+phrase "this check" in your check summation and check documentation.
+
+If your check relates to a published coding guideline (C++ Core Guidelines, MISRA, etc.)
+or style guide, provide links to the relevant guideline or style guide sections in your
+check documentation.
+
+Provide enough examples of the diagnostics and fix-its provided by the check so that a
+user can easily understand what will happen to their code when the check is run.
+If there are exceptions or limitations to your check, document them thoroughly.  This
+will help users understand the scope of the diagnostics and fix-its provided by the check.
+
+Building the target ``docs-clang-tools-html`` will run the Sphinx documentation generator
+and create documentation HTML files in the tools/clang/tools/extra/docs/html directory in
+your build tree.  Make sure that your check is correctly shown in the release notes and the
+list of checks.  Make sure that the formatting and structure of your check's documentation
+looks correct.
+
 
 Registering your Check
 ----------------------
 
-(The ``add_new_check.py`` takes care of registering the check in an existing
+(The ``add_new_check.py`` script takes care of registering the check in an existing
 module. If you want to create a new module or know the details, read on.)
 
 The check should be registered in the corresponding module with a distinct name:
@@ -316,7 +530,9 @@ YAML format:
 Testing Checks
 --------------
 
-To run tests for :program:`clang-tidy` use the command:
+To run tests for :program:`clang-tidy`, build the ``check-clang-tools`` target.
+For instance, if you configured your CMake build with the ninja project generator,
+use the command:
 
 .. code-block:: console
 
@@ -394,7 +610,6 @@ Here's an example:
    // CHECK-FIXES-USING-B-NOT: using a::B;$
    // CHECK-FIXES-NOT: using a::C;$
 
-
 There are many dark corners in the C++ language, and it may be 
diff icult to make
 your check work perfectly in all cases, especially if it issues fix-it hints. The
 most frequent pitfalls are macros and templates:
@@ -411,6 +626,10 @@ most frequent pitfalls are macros and templates:
    macro expansions/template instantiations, but easily break some other
    expansions/instantiations.
 
+If you need multiple files to exercise all the aspects of your check, it is
+recommended you place them in a subdirectory named for the check under ``Inputs``.
+This keeps the test directory from getting cluttered.
+
 .. _lit: https://llvm.org/docs/CommandGuide/lit.html
 .. _FileCheck: https://llvm.org/docs/CommandGuide/FileCheck.html
 .. _test/clang-tidy/google-readability-casting.cpp: https://reviews.llvm.org/
diff usion/L/browse/clang-tools-extra/trunk/test/clang-tidy/google-readability-casting.cpp