Commit 2c9cf9f4 authored by Jon Roelofs's avatar Jon Roelofs
Browse files

[clang-tidy] New check: bugprone-suspicious-include 

Detects and fixes suspicious code like: `#include "foo.cpp"`.

Inspired by: https://twitter.com/lefticus/status/1228458240364687360?s=20

https://reviews.llvm.org/D74669
parent d6883126

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

+3 −0
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ 
#include "StringIntegerAssignmentCheck.h"

#include "StringLiteralWithEmbeddedNulCheck.h"

#include "SuspiciousEnumUsageCheck.h"

#include "SuspiciousIncludeCheck.h"

#include "SuspiciousMemsetUsageCheck.h"

#include "SuspiciousMissingCommaCheck.h"

#include "SuspiciousSemicolonCheck.h"
@@ -140,6 +141,8 @@ public: 
        "bugprone-string-literal-with-embedded-nul");

    CheckFactories.registerCheck<SuspiciousEnumUsageCheck>(

        "bugprone-suspicious-enum-usage");

    CheckFactories.registerCheck<SuspiciousIncludeCheck>(

        "bugprone-suspicious-include");

    CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(

        "bugprone-suspicious-memset-usage");

    CheckFactories.registerCheck<SuspiciousMissingCommaCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

+1 −0
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ add_clang_library(clangTidyBugproneModule 
  StringIntegerAssignmentCheck.cpp

  StringLiteralWithEmbeddedNulCheck.cpp

  SuspiciousEnumUsageCheck.cpp

  SuspiciousIncludeCheck.cpp

  SuspiciousMemsetUsageCheck.cpp

  SuspiciousMissingCommaCheck.cpp

  SuspiciousSemicolonCheck.cpp

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp

0 → 100644
+108 −0
Original line number Diff line number Diff line 
//===--- SuspiciousIncludeCheck.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 "SuspiciousIncludeCheck.h"

#include "clang/AST/ASTContext.h"



namespace clang {

namespace tidy {

namespace bugprone {



namespace {

class SuspiciousIncludePPCallbacks : public PPCallbacks {

public:

  explicit SuspiciousIncludePPCallbacks(SuspiciousIncludeCheck &Check,

                                        const SourceManager &SM,

                                        Preprocessor *PP)

      : Check(Check), PP(PP) {}



  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,

                          StringRef FileName, bool IsAngled,

                          CharSourceRange FilenameRange, const FileEntry *File,

                          StringRef SearchPath, StringRef RelativePath,

                          const Module *Imported,

                          SrcMgr::CharacteristicKind FileType) override;



private:

  SuspiciousIncludeCheck &Check;

  Preprocessor *PP;

};

} // namespace



SuspiciousIncludeCheck::SuspiciousIncludeCheck(StringRef Name,

                                               ClangTidyContext *Context)

    : ClangTidyCheck(Name, Context),

      RawStringHeaderFileExtensions(Options.getLocalOrGlobal(

          "HeaderFileExtensions", utils::defaultHeaderFileExtensions())),

      RawStringImplementationFileExtensions(Options.getLocalOrGlobal(

          "ImplementationFileExtensions",

          utils::defaultImplementationFileExtensions())) {

  if (!utils::parseFileExtensions(RawStringImplementationFileExtensions,

                                  ImplementationFileExtensions,

                                  utils::defaultFileExtensionDelimiters())) {

    llvm::errs() << "Invalid implementation file extension: "

                 << RawStringImplementationFileExtensions << "\n";

  }



  if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,

                                  HeaderFileExtensions,

                                  utils::defaultFileExtensionDelimiters())) {

    llvm::errs() << "Invalid header file extension: "

                 << RawStringHeaderFileExtensions << "\n";

  }

}



void SuspiciousIncludeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {

  Options.store(Opts, "ImplementationFileExtensions",

                RawStringImplementationFileExtensions);

  Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);

}



void SuspiciousIncludeCheck::registerPPCallbacks(

    const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {

  PP->addPPCallbacks(

      ::std::make_unique<SuspiciousIncludePPCallbacks>(*this, SM, PP));

}



void SuspiciousIncludePPCallbacks::InclusionDirective(

    SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,

    bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,

    StringRef SearchPath, StringRef RelativePath, const Module *Imported,

    SrcMgr::CharacteristicKind FileType) {

  if (IncludeTok.getIdentifierInfo()->getPPKeywordID() == tok::pp_import)

    return;



  SourceLocation DiagLoc = FilenameRange.getBegin().getLocWithOffset(1);



  const Optional<StringRef> IFE =

      utils::getFileExtension(FileName, Check.ImplementationFileExtensions);

  if (!IFE)

    return;



  Check.diag(DiagLoc, "suspicious #%0 of file with '%1' extension")

      << IncludeTok.getIdentifierInfo()->getName() << *IFE;



  for (const auto &HFE : Check.HeaderFileExtensions) {

    SmallString<128> GuessedFileName(FileName);

    llvm::sys::path::replace_extension(GuessedFileName,

                                       (HFE.size() ? "." : "") + HFE);



    const DirectoryLookup *CurDir;

    Optional<FileEntryRef> File =

        PP->LookupFile(DiagLoc, GuessedFileName, IsAngled, nullptr, nullptr,

                       CurDir, nullptr, nullptr, nullptr, nullptr, nullptr);

    if (File) {

      Check.diag(DiagLoc, "did you mean to include '%0'?", DiagnosticIDs::Note)

          << GuessedFileName;

    }

  }

}



} // namespace bugprone

} // namespace tidy

} // namespace clang

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h

0 → 100644
+57 −0
Original line number Diff line number Diff line 
//===--- SuspiciousIncludeCheck.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_BUGPRONE_SUSPICIOUSINCLUDECHECK_H

#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H



#include "../ClangTidyCheck.h"

#include "../utils/FileExtensionsUtils.h"



namespace clang {

namespace tidy {

namespace bugprone {



/// Warns on inclusion of files whose names suggest that they're implementation

/// files, instead of headers. E.g:

///

///     #include "foo.cpp"  // warning

///     #include "bar.c"    // warning

///     #include "baz.h"    // no diagnostic

///

/// The check supports these options:

///   - `HeaderFileExtensions`: a semicolon-separated list of filename

///     extensions of header files (The filename extensions should not contain

///     "." prefix) ";h;hh;hpp;hxx" by default. For extension-less header

///     files, using an empty string or leaving an empty string between ";" if

///     there are other filename extensions.

///

///   - `ImplementationFileExtensions`: likewise, a semicolon-separated list of

///     filename extensions of implementation files. "c;cc;cpp;cxx" by default.

///

/// For the user-facing documentation see:

/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html

class SuspiciousIncludeCheck : public ClangTidyCheck {

public:

  SuspiciousIncludeCheck(StringRef Name, ClangTidyContext *Context);

  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,

                           Preprocessor *ModuleExpanderPP) override;

  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;



  utils::FileExtensionsSet HeaderFileExtensions;

  utils::FileExtensionsSet ImplementationFileExtensions;



private:

  const std::string RawStringHeaderFileExtensions;

  const std::string RawStringImplementationFileExtensions;

};



} // namespace bugprone

} // namespace tidy

} // namespace clang



#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp

+11 −4
Original line number Diff line number Diff line
@@ -53,13 +53,20 @@ bool parseFileExtensions(StringRef AllFileExtensions, 
  return true;

}



bool isFileExtension(StringRef FileName,

                     const FileExtensionsSet &FileExtensions) {

llvm::Optional<StringRef>

getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions) {

  StringRef Extension = llvm::sys::path::extension(FileName);

  if (Extension.empty())

    return false;

    return llvm::None;

  // Skip "." prefix.

  return FileExtensions.count(Extension.substr(1)) > 0;

  if (!FileExtensions.count(Extension.substr(1)))

    return llvm::None;

  return Extension;

}



bool isFileExtension(StringRef FileName,

                     const FileExtensionsSet &FileExtensions) {

  return getFileExtension(FileName, FileExtensions).hasValue();

}



} // namespace utils