mgartmann created this revision.
mgartmann added reviewers: aaron.ballman, njames93, alexfh.
mgartmann added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.
mgartmann requested review of this revision.

**Problem Description**

The `iostream` header file defines multiple global objects like `std:cin`, 
`std::cout`, etc.

The pitfall of using these global objects outside of the `main` function is 
that it negatively affects the code's testability. They cannot be replaced with 
a mocked input stream during testing. Therefore,  `std::cin`, `std::cout` etc. 
should only be used inside the main function. If other functions need an input 
or an output stream, one is encouraged to use the `std::istream` and 
`std::ostream` interfaces and to receive the stream object files as function 
parameters. 
Thus, during testing, mocked stream objects can be handed to the function.

**What this Check Does**

The goal of this check is to find any uses of predefined standard stream 
objects (i.e., `cout, wcout, cerr, wcerr, cin, wcin`) and to check whether they 
are used inside the `main` function or not. If any uses are found outside the 
`main` function, they get flagged.

The use of `clog` and `wclog` does not get flagged by this check. The rationale 
for this is that code with logging functionality rarely needs to be tested.

**Context**

The idea for this check is adopted from the checks implemented in Cevelop IDE 
<https://www.cevelop.com/>.

This contribution is part of a project which aims to port Cevelop's built-in 
checks to other IDEs.

We are happy to receive any suggestions for improvement.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy -std=c++14-or-later %s misc-std-stream-objects-outside-main %t
+
+#include <iostream>
+
+void problematic() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  int I{0};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cin >> I;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcin >> I;
+}
+
+int main() {
+  std::cout << "This should not trigger the check";  // OK
+  std::wcout << "This should not trigger the check"; // OK
+  std::cerr << "This should not trigger the check";  // OK
+  std::wcerr << "This should not trigger the check"; // OK
+
+  int I{0};
+  std::cin >> I;  // OK
+  std::wcin >> I; // OK
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - misc-std-stream-objects-outside-main
+
+misc-std-stream-objects-outside-main
+=============
+
+The check diagnoses when a predefined standard stream object (i.e., ``cin``, ``wcin``,
+``cout``, ``wcout``, ``cerr`` and ``wcerr``) is used outside the ``main`` function.
+
+For instance, in the following code, the use of ``std::cout`` outside of ``main()`` would get
+flagged whereas the use of ``std::cout`` inside ``main()`` is not flagged:
+
+.. code-block:: c++
+
+  #include <iostream>
+
+  void some_function() {
+    std::cout << "This triggers the check.";
+         ~~~~
+  }
+
+  int main() {
+    std::cout << "This does not trigger the check.";
+  }
+
+Since the predefined standard stream objects are global objects, their use outside of ``main()`` worsens a
+program's testability and is thus discouraged. Instead, those objects should only be used inside ``main()``.
+They can then be passed as arguments to other functions like so:
+
+.. code-block:: c++
+
+  #include <iostream>
+
+  void some_function(std::istream & in, std::ostream & out) {
+    out << "This does not trigger the check.";
+
+    int i{0};
+    in >> i;
+  }
+
+  int main() {
+    some_function(std::cin, std::cout);
+  }
+
+In contrast to using ``std::cin`` and ``std::cout`` directly, in the above example, it is possible to inject 
+mocked stream objects into ``some_function()`` during testing.
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -210,6 +210,7 @@
    `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
    `misc-redundant-expression <misc-redundant-expression.html>`_, "Yes"
    `misc-static-assert <misc-static-assert.html>`_, "Yes"
+   `misc-std-stream-objects-outside-main <misc-std-stream-objects-outside-main.html>`_, "Yes"
    `misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
    `misc-unconventional-assign-operator <misc-unconventional-assign-operator.html>`_,
    `misc-uniqueptr-reset-release <misc-uniqueptr-reset-release.html>`_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -95,6 +95,12 @@
   Finds member initializations in the constructor body which can be placed into
   the initialization list instead.
 
+- New :doc:`misc-std-stream-objects-outside-main
+  <clang-tidy/checks/misc-std-stream-objects-outside-main>` check.
+
+  Diagnoses if a predefined standard stream object (cin, wcin,
+  cout, wcout, cerr or wcerr) is used outside the main function.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
Index: clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h
@@ -0,0 +1,38 @@
+//===--- StdStreamObjectsOutsideMainCheck.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_MISC_STDSTREAMOBJECTSOUTSIDEMAINCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STDSTREAMOBJECTSOUTSIDEMAINCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Diagnoses if a predefined standard stream object (`cin`, `wcin`,
+/// `cout`, `wcout`, `cerr` or `wcerr`) is used outside the `main` function.
+///
+/// For the user-facing documentation and examples see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-std-stream-objects-outside-main.html
+class StdStreamObjectsOutsideMainCheck : public ClangTidyCheck {
+public:
+  StdStreamObjectsOutsideMainCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  static bool
+  isInsideMainFunction(const ast_matchers::MatchFinder::MatchResult &Result,
+                       const DynTypedNode &Node);
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STDSTREAMOBJECTSOUTSIDEMAINCHECK_H
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp
@@ -0,0 +1,60 @@
+//===--- StdStreamObjectsOutsideMainCheck.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 "StdStreamObjectsOutsideMainCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void StdStreamObjectsOutsideMainCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      declRefExpr(to(namedDecl(hasAnyName("cin", "wcin", "cout", "wcout",
+                                          "cerr", "wcerr"))))
+          .bind("match"),
+      this);
+}
+
+void StdStreamObjectsOutsideMainCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<DeclRefExpr>("match");
+
+  const bool IsInMain = StdStreamObjectsOutsideMainCheck::isInsideMainFunction(
+      Result, DynTypedNode::create(*MatchedDecl));
+
+  if (IsInMain)
+    return;
+
+  diag(MatchedDecl->getLocation(), "predefined standard stream objects should "
+                                   "not be used outside the main function");
+}
+
+bool StdStreamObjectsOutsideMainCheck::isInsideMainFunction(
+    const MatchFinder::MatchResult &Result, const DynTypedNode &Node) {
+  const auto *AsFunctionDecl = Node.get<FunctionDecl>();
+
+  if (AsFunctionDecl && AsFunctionDecl->getIdentifier() &&
+      AsFunctionDecl->getName().equals("main")) {
+    return true;
+  }
+
+  return llvm::any_of(
+      Result.Context->getParents(Node), [&Result](const DynTypedNode &Parent) {
+        return StdStreamObjectsOutsideMainCheck::isInsideMainFunction(Result,
+                                                                      Parent);
+      });
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -17,6 +17,7 @@
 #include "NonPrivateMemberVariablesInClassesCheck.h"
 #include "RedundantExpressionCheck.h"
 #include "StaticAssertCheck.h"
+#include "StdStreamObjectsOutsideMainCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UnconventionalAssignOperatorCheck.h"
 #include "UniqueptrResetReleaseCheck.h"
@@ -44,6 +45,8 @@
     CheckFactories.registerCheck<RedundantExpressionCheck>(
         "misc-redundant-expression");
     CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert");
+    CheckFactories.registerCheck<StdStreamObjectsOutsideMainCheck>(
+        "misc-std-stream-objects-outside-main");
     CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
         "misc-throw-by-value-catch-by-reference");
     CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -13,6 +13,7 @@
   NonPrivateMemberVariablesInClassesCheck.cpp
   RedundantExpressionCheck.cpp
   StaticAssertCheck.cpp
+  StdStreamObjectsOutsideMainCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp
   UnconventionalAssignOperatorCheck.cpp
   UniqueptrResetReleaseCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to