diff --git a/scripts/clang-tidy-plugin/CMakeLists.txt b/scripts/clang-tidy-plugin/CMakeLists.txt new file mode 100644 index 00000000000..e8fdbd2581a --- /dev/null +++ b/scripts/clang-tidy-plugin/CMakeLists.txt @@ -0,0 +1,63 @@ +# Custom clang-tidy plugin for GeneralsGameCode +# This plugin provides checks for custom types like AsciiString and UnicodeString + +cmake_minimum_required(VERSION 3.20) +project(GeneralsGameCodeClangTidyPlugin) + +# Find LLVM and Clang +find_package(LLVM REQUIRED CONFIG) +find_package(Clang REQUIRED CONFIG) + +message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}") +message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}") +message(STATUS "Using ClangConfig.cmake in: ${Clang_DIR}") + +# Set up include directories +include_directories(${LLVM_INCLUDE_DIRS}) +include_directories(${CLANG_INCLUDE_DIRS}) + +# Add definitions +add_definitions(${LLVM_DEFINITIONS}) +add_definitions(${CLANG_DEFINITIONS}) + +# Set C++ standard +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED ON) + +# Source files +set(SOURCES + GeneralsGameCodeTidyModule.cpp + readability/UseIsEmptyCheck.cpp + readability/UseThisInsteadOfSingletonCheck.cpp +) + +# Header files +set(HEADERS + GeneralsGameCodeTidyModule.h + readability/UseIsEmptyCheck.h + readability/UseThisInsteadOfSingletonCheck.h +) + +# Create the module library +add_library(GeneralsGameCodeClangTidyPlugin MODULE ${SOURCES} ${HEADERS}) + +# Link against required libraries +target_link_libraries(GeneralsGameCodeClangTidyPlugin + PRIVATE + clangTidy + clangTidyReadabilityModule + clangAST + clangASTMatchers + clangBasic + clangFrontend + clangLex + clangTooling + LLVMSupport +) + +# Set output directory +set_target_properties(GeneralsGameCodeClangTidyPlugin PROPERTIES + LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib + RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin +) + diff --git a/scripts/clang-tidy-plugin/GeneralsGameCodeTidyModule.cpp b/scripts/clang-tidy-plugin/GeneralsGameCodeTidyModule.cpp new file mode 100644 index 00000000000..ff4222ccebb --- /dev/null +++ b/scripts/clang-tidy-plugin/GeneralsGameCodeTidyModule.cpp @@ -0,0 +1,30 @@ +//===--- GeneralsGameCodeTidyModule.cpp - GeneralsGameCode Tidy Module ---===// +// +// Custom clang-tidy module for GeneralsGameCode +// Provides GeneralsGameCode-specific checks +// +//===----------------------------------------------------------------------===// + +#include "GeneralsGameCodeTidyModule.h" +#include "readability/UseIsEmptyCheck.h" +#include "readability/UseThisInsteadOfSingletonCheck.h" +#include "llvm/Support/Registry.h" + +namespace clang::tidy::generalsgamecode { + +void GeneralsGameCodeTidyModule::addCheckFactories( + ClangTidyCheckFactories &CheckFactories) { + CheckFactories.registerCheck( + "generals-use-is-empty"); + CheckFactories.registerCheck( + "generals-use-this-instead-of-singleton"); +} + +} // namespace clang::tidy::generalsgamecode + +static llvm::Registry<::clang::tidy::ClangTidyModule>::Add< + ::clang::tidy::generalsgamecode::GeneralsGameCodeTidyModule> + X("generalsgamecode", "GeneralsGameCode-specific checks"); + +volatile int GeneralsGameCodeTidyModuleAnchorSource = 0; + diff --git a/scripts/clang-tidy-plugin/GeneralsGameCodeTidyModule.h b/scripts/clang-tidy-plugin/GeneralsGameCodeTidyModule.h new file mode 100644 index 00000000000..f597578acb2 --- /dev/null +++ b/scripts/clang-tidy-plugin/GeneralsGameCodeTidyModule.h @@ -0,0 +1,24 @@ +//===--- GeneralsGameCodeTidyModule.h - GeneralsGameCode Tidy Module -----===// +// +// Custom clang-tidy module for GeneralsGameCode +// Provides checks for custom types like AsciiString and UnicodeString +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GENERALSGAMECODE_GENERALSGAMECODETIDYMODULE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GENERALSGAMECODE_GENERALSGAMECODETIDYMODULE_H + +#include "clang-tidy/ClangTidyModule.h" + +namespace clang::tidy::generalsgamecode { + +/// This module is for GeneralsGameCode-specific checks. +class GeneralsGameCodeTidyModule : public ClangTidyModule { +public: + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override; +}; + +} // namespace clang::tidy::generalsgamecode + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GENERALSGAMECODE_GENERALSGAMECODETIDYMODULE_H + diff --git a/scripts/clang-tidy-plugin/README.md b/scripts/clang-tidy-plugin/README.md new file mode 100644 index 00000000000..cbd9a2f83ef --- /dev/null +++ b/scripts/clang-tidy-plugin/README.md @@ -0,0 +1,250 @@ +# GeneralsGameCode Clang-Tidy Custom Checks + +Custom clang-tidy checks for the GeneralsGameCode codebase. **This is primarily designed for Windows**, where checks are built directly into clang-tidy. Mac/Linux users can optionally use the plugin version. + +## Quick Start + +### Windows + +**Option 1: Use Pre-built clang-tidy** +- Precompiled binaries can be found at: https://github.com/TheSuperHackers/GeneralsTools + +**Option 2: Build It Yourself** + +Follow the manual steps in the [Windows: Building Checks Into clang-tidy](#windows-building-checks-into-clang-tidy) section below. + +**Usage** +```powershell +llvm-project\build\bin\clang-tidy.exe -p build/clang-tidy ` + --checks='-*,generals-use-is-empty,generals-use-this-instead-of-singleton' ` + file.cpp +``` + +### macOS/Linux (Plugin Version) + +If you prefer the plugin approach on macOS/Linux: + +```bash +# Build the plugin +cd scripts/clang-tidy-plugin +mkdir build && cd build +cmake .. -DLLVM_DIR=/path/to/llvm/lib/cmake/llvm +cmake --build . --config Release + +# Use with --load flag +clang-tidy -p build/clang-tidy \ + --checks='-*,generals-use-is-empty' \ + -load scripts/clang-tidy-plugin/build/lib/libGeneralsGameCodeClangTidyPlugin.so \ + file.cpp +``` + +**Note:** On Windows, plugin loading via `--load` is **not supported** due to DLL static initialization limitations. Windows users must use the built-in version. + +## Checks + +### `generals-use-is-empty` + +Finds uses of `getLength() == 0`, `getLength() > 0`, `compare("") == 0`, or `compareNoCase("") == 0` on `AsciiString` and `UnicodeString`, and `Get_Length() == 0` on `StringClass` and `WideStringClass`, and suggests using `isEmpty()`/`Is_Empty()` or `!isEmpty()`/`!Is_Empty()` instead. + +**Examples:** + +```cpp +// Before (AsciiString/UnicodeString) +if (str.getLength() == 0) { ... } +if (str.getLength() > 0) { ... } +if (str.compare("") == 0) { ... } +if (str.compareNoCase("") == 0) { ... } +if (str.compare(AsciiString::TheEmptyString) == 0) { ... } + +// After (AsciiString/UnicodeString) +if (str.isEmpty()) { ... } +if (!str.isEmpty()) { ... } +if (str.isEmpty()) { ... } +if (str.isEmpty()) { ... } +if (str.isEmpty()) { ... } + +// Before (StringClass/WideStringClass) +if (str.Get_Length() == 0) { ... } +if (str.Get_Length() > 0) { ... } + +// After (StringClass/WideStringClass) +if (str.Is_Empty()) { ... } +if (!str.Is_Empty()) { ... } +``` + +### `generals-use-this-instead-of-singleton` + +Finds uses of singleton global variables (like `TheGameLogic->method()` or `TheGlobalData->member`) inside member functions of the same class type, and suggests using the member directly (e.g., `method()` or `member`) instead of the singleton reference. + +**Examples:** + +```cpp +// Before +void GameLogic::update() { + UnsignedInt now = TheGameLogic->getFrame(); + TheGameLogic->setFrame(now + 1); + TheGameLogic->m_frame = 10; +} + +// After +void GameLogic::update() { + UnsignedInt now = getFrame(); + setFrame(now + 1); + m_frame = 10; +} +``` + +## Prerequisites + +Before using clang-tidy, you need to generate a compile commands database: + +```bash +cmake -B build/clang-tidy -DCMAKE_DISABLE_PRECOMPILE_HEADERS=ON -G Ninja +``` + +This creates `build/clang-tidy/compile_commands.json` which tells clang-tidy how to compile each file. + +## Windows: Building Checks Into clang-tidy + +Since plugin loading via `--load` doesn't work on Windows, the checks are integrated directly into the clang-tidy source tree. This is the recommended approach for Windows. + +### Step 1: Clone and Build LLVM + +### Step 2: Copy Plugin Files to LLVM Source Tree + +```powershell +# Create the plugin directory in clang-tools-extra +mkdir llvm-project\clang-tools-extra\clang-tidy\plugins\generalsgamecode + +# Copy the plugin source files +cp -r scripts\clang-tidy-plugin\*.cpp llvm-project\clang-tools-extra\clang-tidy\plugins\generalsgamecode\ +cp -r scripts\clang-tidy-plugin\*.h llvm-project\clang-tools-extra\clang-tidy\plugins\generalsgamecode\ +cp -r scripts\clang-tidy-plugin\readability llvm-project\clang-tools-extra\clang-tidy\plugins\generalsgamecode\ +``` + +**Important:** After copying, you need to update the include paths in the headers: +- Change `#include "clang-tidy/ClangTidyCheck.h"` to `#include "../../../ClangTidyCheck.h"` +- Change `#include "clang-tidy/ClangTidyModule.h"` to `#include "../../ClangTidyModule.h"` + +### Step 3: Create CMakeLists.txt for the Module + +Create `llvm-project/clang-tools-extra/clang-tidy/plugins/generalsgamecode/CMakeLists.txt`: + +```cmake +add_clang_library(clangTidyGeneralsGameCodeModule STATIC + GeneralsGameCodeTidyModule.cpp + readability/UseIsEmptyCheck.cpp + readability/UseThisInsteadOfSingletonCheck.cpp + + LINK_LIBS + clangTidy + clangTidyUtils + ) + +clang_target_link_libraries(clangTidyGeneralsGameCodeModule + PRIVATE + clangAST + clangASTMatchers + clangBasic + clangLex + clangTooling + ) +``` + +### Step 4: Register the Module + +**Modify `llvm-project/clang-tools-extra/clang-tidy/CMakeLists.txt`:** + +Add the subdirectory: +```cmake +add_subdirectory(plugins/generalsgamecode) +``` + +Add to `ALL_CLANG_TIDY_CHECKS`: +```cmake +set(ALL_CLANG_TIDY_CHECKS + ... + clangTidyGeneralsGameCodeModule + ... +) +``` + +**Modify `llvm-project/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h`:** + +Add the anchor to force linker inclusion: +```cpp +// This anchor is used to force the linker to link the GeneralsGameCodeModule. +extern volatile int GeneralsGameCodeModuleAnchorSource; +static int LLVM_ATTRIBUTE_UNUSED GeneralsGameCodeModuleAnchorDestination = + GeneralsGameCodeModuleAnchorSource; +``` + +**Modify `llvm-project/clang-tools-extra/clang-tidy/plugins/generalsgamecode/GeneralsGameCodeTidyModule.cpp`:** + +Ensure the anchor is defined: +```cpp +namespace clang::tidy { +// ... module registration ... + +// Force linker to include this module +volatile int GeneralsGameCodeModuleAnchorSource = 0; +} +``` + +### Step 5: Rebuild clang-tidy + +```powershell +cd llvm-project\build +ninja clang-tidy +``` + +### Step 6: Use the Built-in Checks + +Once rebuilt, the checks are always available - no `--load` flag needed: + +```powershell +llvm-project\build\bin\clang-tidy.exe -p build/clang-tidy ` + --checks='-*,generals-use-is-empty,generals-use-this-instead-of-singleton' ` + file.cpp +``` + + +## macOS/Linux: Building the Plugin + +If you're on macOS or Linux and want to use the plugin version: + +### Prerequisites + +**macOS:** +```bash +brew install llvm@21 +``` + +**Linux (Ubuntu/Debian):** +```bash +sudo apt-get install llvm-21-dev clang-21 libclang-21-dev +``` + +### Building the Plugin + +```bash +cd scripts/clang-tidy-plugin +mkdir build && cd build +cmake .. -DLLVM_DIR=/path/to/llvm/lib/cmake/llvm -DClang_DIR=/path/to/clang/lib/cmake/clang +cmake --build . --config Release +``` + +The plugin will be built as a shared library (`.so` on Linux, `.dylib` on macOS) in the `build/lib/` directory. + +### Using the Plugin + +```bash +clang-tidy -p build/clang-tidy \ + --checks='-*,generals-use-is-empty,generals-use-this-instead-of-singleton' \ + -load scripts/clang-tidy-plugin/build/lib/libGeneralsGameCodeClangTidyPlugin.so \ + file.cpp +``` + +**Important:** The plugin must be built with the same LLVM version as the `clang-tidy` executable in your PATH. The CMake build will display which LLVM version it found (e.g., `Found LLVM 21.1.7`). Verify this matches your `clang-tidy --version` output. + +- Windows plugins not working is a known limitation - see [GitHub issue #159710](https://github.com/llvm/llvm-project/issues/159710) and [LLVM Discourse discussion](https://discourse.llvm.org/t/clang-tidy-is-clang-tidy-out-of-tree-check-plugin-load-mechanism-guaranteed-to-work-with-msvc/84111) diff --git a/scripts/clang-tidy-plugin/readability/UseIsEmptyCheck.cpp b/scripts/clang-tidy-plugin/readability/UseIsEmptyCheck.cpp new file mode 100644 index 00000000000..fe70a15b5ed --- /dev/null +++ b/scripts/clang-tidy-plugin/readability/UseIsEmptyCheck.cpp @@ -0,0 +1,254 @@ +//===--- UseIsEmptyCheck.cpp - Use isEmpty() instead of getLength() == 0 -===// +// +// This check finds patterns like: +// - AsciiString::getLength() == 0 -> AsciiString::isEmpty() +// - AsciiString::getLength() > 0 -> !AsciiString::isEmpty() +// - UnicodeString::getLength() == 0 -> UnicodeString::isEmpty() +// - UnicodeString::getLength() > 0 -> !UnicodeString::isEmpty() +// - StringClass::Get_Length() == 0 -> StringClass::Is_Empty() +// - WideStringClass::Get_Length() == 0 -> WideStringClass::Is_Empty() +// - AsciiString::compare("") == 0 -> AsciiString::isEmpty() +// - UnicodeString::compare(L"") == 0 -> UnicodeString::isEmpty() +// - AsciiString::compare(AsciiString::TheEmptyString) == 0 -> AsciiString::isEmpty() +// - UnicodeString::compare(UnicodeString::TheEmptyString) == 0 -> UnicodeString::isEmpty() +// +//===----------------------------------------------------------------------===// + +#include "UseIsEmptyCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::generalsgamecode::readability { + +void UseIsEmptyCheck::registerMatchers(MatchFinder *Finder) { + // Matcher for AsciiString/UnicodeString with getLength() + auto GetLengthCall = cxxMemberCallExpr( + callee(cxxMethodDecl(hasName("getLength"))), + on(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(cxxRecordDecl( + hasAnyName("AsciiString", "UnicodeString")))))))); + + // Matcher for StringClass/WideStringClass with Get_Length() + auto GetLengthCallWWVegas = cxxMemberCallExpr( + callee(cxxMethodDecl(hasName("Get_Length"))), + on(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(cxxRecordDecl( + hasAnyName("StringClass", "WideStringClass")))))))); + + // Helper function to add matchers for a given GetLength call matcher + auto addMatchersForGetLength = [&](const auto &GetLengthMatcher) { + Finder->addMatcher( + binaryOperator( + hasOperatorName("=="), + hasLHS(ignoringParenImpCasts(GetLengthMatcher.bind("getLengthCall"))), + hasRHS(integerLiteral(equals(0)).bind("zero"))) + .bind("comparison"), + this); + + Finder->addMatcher( + binaryOperator( + hasOperatorName("!="), + hasLHS(ignoringParenImpCasts(GetLengthMatcher.bind("getLengthCall"))), + hasRHS(integerLiteral(equals(0)).bind("zero"))) + .bind("comparison"), + this); + + Finder->addMatcher( + binaryOperator( + hasOperatorName(">"), + hasLHS(ignoringParenImpCasts(GetLengthMatcher.bind("getLengthCall"))), + hasRHS(integerLiteral(equals(0)).bind("zero"))) + .bind("comparison"), + this); + + Finder->addMatcher( + binaryOperator( + hasOperatorName("<="), + hasLHS(ignoringParenImpCasts(GetLengthMatcher.bind("getLengthCall"))), + hasRHS(integerLiteral(equals(0)).bind("zero"))) + .bind("comparison"), + this); + + Finder->addMatcher( + binaryOperator( + hasOperatorName("=="), + hasLHS(integerLiteral(equals(0)).bind("zero")), + hasRHS(ignoringParenImpCasts(GetLengthMatcher.bind("getLengthCall")))) + .bind("comparison"), + this); + + Finder->addMatcher( + binaryOperator( + hasOperatorName("!="), + hasLHS(integerLiteral(equals(0)).bind("zero")), + hasRHS(ignoringParenImpCasts(GetLengthMatcher.bind("getLengthCall")))) + .bind("comparison"), + this); + }; + + addMatchersForGetLength(GetLengthCall); + addMatchersForGetLength(GetLengthCallWWVegas); + + // Matcher for TheEmptyString static member access (AsciiString::TheEmptyString or UnicodeString::TheEmptyString) + auto TheEmptyStringRef = memberExpr( + member(hasName("TheEmptyString")), + hasObjectExpression(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(cxxRecordDecl( + hasAnyName("AsciiString", "UnicodeString")))))))); + + // Matcher for AsciiString/UnicodeString with compare() - we'll check if argument is empty in check() + auto CompareCall = cxxMemberCallExpr( + callee(cxxMethodDecl(hasName("compare"))), + on(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(cxxRecordDecl( + hasAnyName("AsciiString", "UnicodeString"))))))), + hasArgument(0, anyOf( + stringLiteral().bind("stringLiteralArg"), + TheEmptyStringRef.bind("theEmptyStringArg")))); + + // Matcher for AsciiString/UnicodeString with compareNoCase() - we'll check if argument is empty in check() + auto CompareNoCaseCall = cxxMemberCallExpr( + callee(cxxMethodDecl(hasName("compareNoCase"))), + on(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(cxxRecordDecl( + hasAnyName("AsciiString", "UnicodeString"))))))), + hasArgument(0, anyOf( + stringLiteral().bind("stringLiteralArg"), + TheEmptyStringRef.bind("theEmptyStringArg")))); + + // Helper function to add matchers for compare() and compareNoCase() calls + auto addMatchersForCompare = [&](const auto &CompareMatcher, const char *BindingName) { + Finder->addMatcher( + binaryOperator( + hasOperatorName("=="), + hasLHS(ignoringParenImpCasts(CompareMatcher.bind(BindingName))), + hasRHS(integerLiteral(equals(0)).bind("zero"))) + .bind("comparison"), + this); + + Finder->addMatcher( + binaryOperator( + hasOperatorName("!="), + hasLHS(ignoringParenImpCasts(CompareMatcher.bind(BindingName))), + hasRHS(integerLiteral(equals(0)).bind("zero"))) + .bind("comparison"), + this); + + Finder->addMatcher( + binaryOperator( + hasOperatorName("=="), + hasLHS(integerLiteral(equals(0)).bind("zero")), + hasRHS(ignoringParenImpCasts(CompareMatcher.bind(BindingName)))) + .bind("comparison"), + this); + + Finder->addMatcher( + binaryOperator( + hasOperatorName("!="), + hasLHS(integerLiteral(equals(0)).bind("zero")), + hasRHS(ignoringParenImpCasts(CompareMatcher.bind(BindingName)))) + .bind("comparison"), + this); + }; + + addMatchersForCompare(CompareCall, "compareCall"); + addMatchersForCompare(CompareNoCaseCall, "compareNoCaseCall"); +} + +void UseIsEmptyCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Comparison = Result.Nodes.getNodeAs("comparison"); + const auto *GetLengthCall = + Result.Nodes.getNodeAs("getLengthCall"); + const auto *CompareCall = + Result.Nodes.getNodeAs("compareCall"); + const auto *CompareNoCaseCall = + Result.Nodes.getNodeAs("compareNoCaseCall"); + + if (!Comparison) + return; + + const CXXMemberCallExpr *MethodCall = GetLengthCall ? GetLengthCall : + (CompareCall ? CompareCall : CompareNoCaseCall); + if (!MethodCall) + return; + + // For compare() and compareNoCase() calls, verify the argument is actually empty + if (CompareCall || CompareNoCaseCall) { + const auto *StringLit = Result.Nodes.getNodeAs("stringLiteralArg"); + const auto *TheEmptyString = Result.Nodes.getNodeAs("theEmptyStringArg"); + + if (StringLit) { + // Check if string literal is actually empty ("" or L"") + StringRef Str = StringLit->getString(); + if (!Str.empty()) { + return; // Not an empty string literal + } + } else if (!TheEmptyString) { + return; // Not matching empty string pattern + } + } + + const Expr *ObjectExpr = MethodCall->getImplicitObjectArgument(); + if (!ObjectExpr) + return; + + // Determine which method name to use based on the called method + StringRef MethodName = MethodCall->getMethodDecl()->getName(); + std::string IsEmptyMethodName; + std::string MethodNameStr; + + if (MethodName == "Get_Length") { + IsEmptyMethodName = "Is_Empty()"; + MethodNameStr = "Get_Length()"; + } else if (MethodName == "compare") { + IsEmptyMethodName = "isEmpty()"; + MethodNameStr = "compare()"; + } else if (MethodName == "compareNoCase") { + IsEmptyMethodName = "isEmpty()"; + MethodNameStr = "compareNoCase()"; + } else { + IsEmptyMethodName = "isEmpty()"; + MethodNameStr = "getLength()"; + } + + StringRef Operator = Comparison->getOpcodeStr(); + bool ShouldNegate = false; + + if (Operator == "==") { + ShouldNegate = false; + } else if (Operator == "!=") { + ShouldNegate = true; + } else if (Operator == ">") { + ShouldNegate = true; + } else if (Operator == "<=") { + ShouldNegate = false; + } else { + return; + } + + StringRef ObjectText = Lexer::getSourceText( + CharSourceRange::getTokenRange(ObjectExpr->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + + std::string Replacement; + if (ShouldNegate) { + Replacement = "!" + ObjectText.str() + "." + IsEmptyMethodName; + } else { + Replacement = ObjectText.str() + "." + IsEmptyMethodName; + } + + SourceLocation StartLoc = Comparison->getBeginLoc(); + SourceLocation EndLoc = Comparison->getEndLoc(); + + diag(Comparison->getBeginLoc(), + "use %0 instead of comparing %1 with 0") + << Replacement << MethodNameStr + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(StartLoc, EndLoc), Replacement); +} + +} // namespace clang::tidy::generalsgamecode::readability diff --git a/scripts/clang-tidy-plugin/readability/UseIsEmptyCheck.h b/scripts/clang-tidy-plugin/readability/UseIsEmptyCheck.h new file mode 100644 index 00000000000..2d6dd84253d --- /dev/null +++ b/scripts/clang-tidy-plugin/readability/UseIsEmptyCheck.h @@ -0,0 +1,45 @@ +//===--- UseIsEmptyCheck.h - Use isEmpty() instead of getLength() == 0 ---===// +// +// This check finds patterns like: +// - AsciiString::getLength() == 0 -> AsciiString::isEmpty() +// - AsciiString::getLength() > 0 -> !AsciiString::isEmpty() +// - UnicodeString::getLength() == 0 -> UnicodeString::isEmpty() +// - UnicodeString::getLength() > 0 -> !UnicodeString::isEmpty() +// - StringClass::Get_Length() == 0 -> StringClass::Is_Empty() +// - WideStringClass::Get_Length() == 0 -> WideStringClass::Is_Empty() +// - AsciiString::compare("") == 0 -> AsciiString::isEmpty() +// - UnicodeString::compare(L"") == 0 -> UnicodeString::isEmpty() +// - AsciiString::compare(AsciiString::TheEmptyString) == 0 -> AsciiString::isEmpty() +// - UnicodeString::compare(UnicodeString::TheEmptyString) == 0 -> UnicodeString::isEmpty() +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GENERALSGAMECODE_READABILITY_USEISEMPTYCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GENERALSGAMECODE_READABILITY_USEISEMPTYCHECK_H + +#include "clang-tidy/ClangTidyCheck.h" + +namespace clang::tidy::generalsgamecode::readability { + +/// Finds uses of getLength() == 0 or getLength() > 0 on AsciiString and +/// UnicodeString, and Get_Length() == 0 on StringClass and WideStringClass, +/// and suggests using isEmpty()/Is_Empty() or !isEmpty()/!Is_Empty() instead. +/// Also finds uses of compare("") == 0 or compare(TheEmptyString) == 0 and +/// suggests using isEmpty() instead. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/generals-use-is-empty.html +class UseIsEmptyCheck : public ClangTidyCheck { +public: + UseIsEmptyCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::generalsgamecode::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GENERALSGAMECODE_READABILITY_USEISEMPTYCHECK_H diff --git a/scripts/clang-tidy-plugin/readability/UseThisInsteadOfSingletonCheck.cpp b/scripts/clang-tidy-plugin/readability/UseThisInsteadOfSingletonCheck.cpp new file mode 100644 index 00000000000..51eff283ef6 --- /dev/null +++ b/scripts/clang-tidy-plugin/readability/UseThisInsteadOfSingletonCheck.cpp @@ -0,0 +1,225 @@ +#include "UseThisInsteadOfSingletonCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/Type.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Lex/Lexer.h" + +using namespace clang; +using namespace clang::ast_matchers; + +namespace clang::tidy::generalsgamecode::readability { + + +static const CXXMethodDecl *getEnclosingMethod(ASTContext *Context, + const Stmt *S) { + const CXXMethodDecl *Method = nullptr; + + auto Parents = Context->getParents(*S); + while (!Parents.empty()) { + if (const auto *M = Parents[0].get()) { + if (!M->isStatic()) { + Method = M; + break; + } + } + Parents = Context->getParents(Parents[0]); + } + + return Method; +} + +static bool typesMatch(const QualType &SingletonType, + const CXXRecordDecl *EnclosingClass) { + if (!EnclosingClass) { + return false; + } + + const Type *TypePtr = SingletonType.getTypePtrOrNull(); + if (!TypePtr) { + return false; + } + + if (const PointerType *PtrType = TypePtr->getAs()) { + QualType PointeeType = PtrType->getPointeeType(); + if (const RecordType *RecordTy = PointeeType->getAs()) { + if (const CXXRecordDecl *RecordDecl = + dyn_cast(RecordTy->getDecl())) { + return RecordDecl->getCanonicalDecl() == + EnclosingClass->getCanonicalDecl(); + } + } + } + + return false; +} + +void UseThisInsteadOfSingletonCheck::registerMatchers(MatchFinder *Finder) { + auto MemberExprMatcher = memberExpr( + hasObjectExpression(ignoringParenImpCasts(declRefExpr(to(varDecl(anyOf(hasGlobalStorage(), hasExternalFormalLinkage())).bind("singletonVar"))))), + hasDeclaration(fieldDecl()), + unless(hasAncestor(cxxMemberCallExpr()))) + .bind("memberExpr"); + + auto MemberCallMatcher = cxxMemberCallExpr( + on(ignoringParenImpCasts(declRefExpr(to(varDecl(anyOf(hasGlobalStorage(), hasExternalFormalLinkage())).bind("singletonVarCall")))))) + .bind("memberCall"); + + Finder->addMatcher(MemberExprMatcher, this); + Finder->addMatcher(MemberCallMatcher, this); +} + +void UseThisInsteadOfSingletonCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MemExpr = Result.Nodes.getNodeAs("memberExpr"); + const auto *MemberCall = Result.Nodes.getNodeAs("memberCall"); + + if (!MemExpr && !MemberCall) { + return; + } + + const Stmt *TargetStmt = nullptr; + const VarDecl *FoundSingletonVar = nullptr; + bool IsCall = false; + + if (MemberCall) { + IsCall = true; + TargetStmt = MemberCall; + FoundSingletonVar = Result.Nodes.getNodeAs("singletonVarCall"); + if (!FoundSingletonVar) { + const Expr *ImplicitObject = MemberCall->getImplicitObjectArgument(); + if (ImplicitObject) { + ImplicitObject = ImplicitObject->IgnoreParenImpCasts(); + if (const DeclRefExpr *DRE = dyn_cast(ImplicitObject)) { + FoundSingletonVar = dyn_cast(DRE->getDecl()); + } + } + } + } else if (MemExpr) { + TargetStmt = MemExpr; + FoundSingletonVar = Result.Nodes.getNodeAs("singletonVar"); + } + + if (!TargetStmt || !FoundSingletonVar) { + return; + } + + StringRef SingletonName = FoundSingletonVar->getName(); + if (!SingletonName.starts_with("The") || SingletonName.size() <= 3 || + (SingletonName[3] < 'A' || SingletonName[3] > 'Z')) { + return; + } + + const ASTContext *Context = Result.Context; + + const CXXMethodDecl *EnclosingMethod = getEnclosingMethod( + const_cast(Context), TargetStmt); + if (!EnclosingMethod) { + return; + } + + const CXXRecordDecl *EnclosingClass = EnclosingMethod->getParent(); + if (!EnclosingClass) { + return; + } + + QualType SingletonType = FoundSingletonVar->getType(); + if (!typesMatch(SingletonType, EnclosingClass)) { + return; + } + + const ValueDecl *Member = nullptr; + StringRef MemberName; + SourceLocation StartLoc, EndLoc; + + if (IsCall && MemberCall) { + const CXXMethodDecl *Method = MemberCall->getMethodDecl(); + if (!Method) { + return; + } + if (Method->isStatic()) { + return; + } + if (EnclosingMethod->isConst() && !Method->isConst()) { + return; + } + Member = Method; + MemberName = Method->getName(); + StartLoc = MemberCall->getBeginLoc(); + EndLoc = MemberCall->getEndLoc(); + } else if (MemExpr) { + Member = MemExpr->getMemberDecl(); + if (!Member) { + return; + } + MemberName = Member->getName(); + StartLoc = MemExpr->getBeginLoc(); + EndLoc = MemExpr->getEndLoc(); + } else { + return; + } + + SourceManager &SM = *Result.SourceManager; + + std::string Replacement = std::string(MemberName); + + if (IsCall && MemberCall) { + SourceLocation RParenLoc = MemberCall->getRParenLoc(); + const Expr *Callee = MemberCall->getCallee(); + + if (RParenLoc.isValid() && Callee) { + SourceLocation CalleeEnd = Lexer::getLocForEndOfToken( + Callee->getEndLoc(), 0, SM, Result.Context->getLangOpts()); + + if (CalleeEnd.isValid() && CalleeEnd < RParenLoc) { + SourceLocation ArgsStart = CalleeEnd; + SourceLocation ArgsEnd = Lexer::getLocForEndOfToken( + RParenLoc, 0, SM, Result.Context->getLangOpts()); + + if (ArgsStart.isValid() && ArgsEnd.isValid() && ArgsStart < ArgsEnd) { + StringRef ArgsText = Lexer::getSourceText( + CharSourceRange::getCharRange(ArgsStart, ArgsEnd), SM, + Result.Context->getLangOpts()); + ArgsText = ArgsText.ltrim(); + if (!ArgsText.empty()) { + Replacement += ArgsText.str(); + } else { + Replacement += "()"; + } + } else { + Replacement += "()"; + } + } else { + SourceLocation CallEnd = Lexer::getLocForEndOfToken( + MemberCall->getEndLoc(), 0, SM, Result.Context->getLangOpts()); + if (CalleeEnd.isValid() && CallEnd.isValid() && CalleeEnd < CallEnd) { + StringRef ArgsText = Lexer::getSourceText( + CharSourceRange::getCharRange(CalleeEnd, CallEnd), SM, + Result.Context->getLangOpts()); + ArgsText = ArgsText.ltrim(); + if (!ArgsText.empty()) { + Replacement += ArgsText.str(); + } else { + Replacement += "()"; + } + } else { + Replacement += "()"; + } + } + } else { + Replacement += "()"; + } + } + + diag(StartLoc, "use '%0' instead of '%1->%2' when inside a member function") + << Replacement << FoundSingletonVar->getName() << MemberName + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(StartLoc, EndLoc), Replacement); +} + +} // namespace clang::tidy::generalsgamecode::readability + diff --git a/scripts/clang-tidy-plugin/readability/UseThisInsteadOfSingletonCheck.h b/scripts/clang-tidy-plugin/readability/UseThisInsteadOfSingletonCheck.h new file mode 100644 index 00000000000..1005d5d84b5 --- /dev/null +++ b/scripts/clang-tidy-plugin/readability/UseThisInsteadOfSingletonCheck.h @@ -0,0 +1,22 @@ +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GENERALSGAMECODE_READABILITY_USETHISINSTEADOFSINGLETONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GENERALSGAMECODE_READABILITY_USETHISINSTEADOFSINGLETONCHECK_H + +#include "clang-tidy/ClangTidyCheck.h" + +namespace clang::tidy::generalsgamecode::readability { + +class UseThisInsteadOfSingletonCheck : public ClangTidyCheck { +public: + UseThisInsteadOfSingletonCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::generalsgamecode::readability + +#endif + diff --git a/scripts/run-clang-tidy.py b/scripts/run-clang-tidy.py index 49f06b4563a..71b41ab70ce 100755 --- a/scripts/run-clang-tidy.py +++ b/scripts/run-clang-tidy.py @@ -27,7 +27,7 @@ def find_clang_tidy() -> str: - """Find clang-tidy executable in PATH.""" + """Find clang-tidy executable in PATH or common locations.""" try: result = subprocess.run( ['clang-tidy', '--version'], @@ -40,8 +40,38 @@ def find_clang_tidy() -> str: except (FileNotFoundError, subprocess.TimeoutExpired): pass + import platform + if platform.system() == 'Darwin': + import glob + import re + possible_paths = glob.glob('/opt/homebrew/Cellar/llvm*/*/bin/clang-tidy') + possible_paths.extend(glob.glob('/usr/local/Cellar/llvm*/*/bin/clang-tidy')) + + def extract_version(path): + match = re.search(r'llvm@?(\d+)', path) + if match: + return int(match.group(1)) + match = re.search(r'/(\d+)\.(\d+)\.(\d+)', path) + if match: + return int(match.group(1)) * 10000 + int(match.group(2)) * 100 + int(match.group(3)) + return 0 + + for path in sorted(possible_paths, key=extract_version, reverse=True): + try: + result = subprocess.run( + [path, '--version'], + capture_output=True, + text=True, + timeout=5 + ) + if result.returncode == 0: + return path + except (FileNotFoundError, subprocess.TimeoutExpired): + continue + raise RuntimeError( "clang-tidy not found in PATH. Please install clang-tidy:\n" + " macOS: brew install llvm\n" " Windows: Install LLVM from https://llvm.org/builds/" ) @@ -56,6 +86,54 @@ def find_project_root() -> Path: raise RuntimeError("Could not find project root (no CMakeLists.txt found)") +def get_clang_tidy_version(clang_tidy_exe: str) -> Optional[str]: + """Get the version string from clang-tidy.""" + try: + result = subprocess.run( + [clang_tidy_exe, '--version'], + capture_output=True, + text=True, + timeout=5 + ) + if result.returncode == 0: + return result.stdout.strip() + except (FileNotFoundError, subprocess.TimeoutExpired): + pass + return None + + +def extract_llvm_version(version_string: str) -> Optional[str]: + """Extract LLVM version number from clang-tidy version string.""" + import re + patterns = [ + r'LLVM version (\d+\.\d+\.\d+)', + r'llvm version (\d+\.\d+\.\d+)', + r'Homebrew LLVM version (\d+\.\d+\.\d+)', + r'version (\d+\.\d+\.\d+)', + ] + for pattern in patterns: + match = re.search(pattern, version_string, re.IGNORECASE) + if match: + return match.group(1) + return None + + +def find_clang_tidy_plugin(project_root: Path) -> Optional[str]: + """Find the GeneralsGameCode clang-tidy plugin.""" + possible_paths = [ + project_root / "scripts" / "clang-tidy-plugin" / "build" / "lib" / "libGeneralsGameCodeClangTidyPlugin.so", + project_root / "scripts" / "clang-tidy-plugin" / "build" / "lib" / "libGeneralsGameCodeClangTidyPlugin.dylib", + project_root / "scripts" / "clang-tidy-plugin" / "build" / "lib" / "libGeneralsGameCodeClangTidyPlugin.dll", + project_root / "scripts" / "clang-tidy-plugin" / "build" / "bin" / "libGeneralsGameCodeClangTidyPlugin.dll", + ] + + for path in possible_paths: + if path.exists(): + return str(path) + + return None + + def find_compile_commands(build_dir: Optional[Path] = None) -> Path: """Find compile_commands.json from the clang-tidy analysis build.""" project_root = find_project_root() @@ -174,7 +252,7 @@ def _run_batch(args: Tuple) -> Tuple[int, Dict[str, List[str]]]: if is_warning_or_error or verbose: issues_by_file[file_key].append(line) - + return result.returncode, dict(issues_by_file) except FileNotFoundError: error_msg = "Error: clang-tidy not found. Please install LLVM/Clang." @@ -188,7 +266,8 @@ def run_clang_tidy(source_files: List[str], extra_args: List[str], fix: bool = False, jobs: int = 1, - verbose: bool = False) -> int: + verbose: bool = False, + load_plugin: bool = True) -> int: """Run clang-tidy on source files in batches, optionally in parallel.""" if not source_files: print("No source files to analyze.") @@ -196,17 +275,41 @@ def run_clang_tidy(source_files: List[str], clang_tidy_exe = find_clang_tidy() + project_root = find_project_root() + plugin_path = None + if load_plugin: + plugin_path = find_clang_tidy_plugin(project_root) + if plugin_path: + clang_tidy_version_str = get_clang_tidy_version(clang_tidy_exe) + if clang_tidy_version_str: + detected_version = extract_llvm_version(clang_tidy_version_str) + if detected_version: + if verbose: + print(f"Found clang-tidy plugin: {plugin_path}") + print(f"Using clang-tidy LLVM version: {detected_version}") + print("Note: Ensure the plugin was built with the same LLVM version (check CMake build output).\n") + else: + print(f"Note: Verify plugin was built with LLVM {detected_version} (check CMake build output)") + else: + if verbose: + print(f"Found clang-tidy plugin: {plugin_path}\n") + else: + if verbose: + print(f"Found clang-tidy plugin: {plugin_path}\n") + BATCH_SIZE = 50 total_files = len(source_files) batches = [source_files[i:i + BATCH_SIZE] for i in range(0, total_files, BATCH_SIZE)] - project_root = find_project_root() compile_commands_dir = compile_commands_path.parent all_issues = defaultdict(list) files_with_issues = set() total_issues = 0 + if plugin_path and '-load' not in ' '.join(extra_args): + extra_args = ['-load', plugin_path] + extra_args + if jobs > 1: if verbose: print(f"Running clang-tidy on {total_files} file(s) in {len(batches)} batch(es) with {jobs} workers...\n") @@ -232,7 +335,7 @@ def run_clang_tidy(source_files: List[str], all_issues[file_path].extend(file_issues) files_with_issues.add(file_path) total_issues += len(file_issues) - + if not verbose: print(" done.") except KeyboardInterrupt: @@ -249,7 +352,7 @@ def run_clang_tidy(source_files: List[str], try: if verbose: print(f"Batch {batch_num}/{len(batches)}: {len(batch)} file(s)...") - + returncode, issues = _run_batch((batch_num, batch, compile_commands_dir, fix, extra_args, project_root, clang_tidy_exe, verbose)) if returncode != 0: overall_returncode = returncode @@ -259,7 +362,7 @@ def run_clang_tidy(source_files: List[str], all_issues[file_path].extend(file_issues) files_with_issues.add(file_path) total_issues += len(file_issues) - + if not verbose and batch_num < len(batches): print('.', end='', flush=True) except KeyboardInterrupt: @@ -277,7 +380,7 @@ def run_clang_tidy(source_files: List[str], print(f"\n{file_path}:") for issue in all_issues[file_path]: print(f" {issue}") - + return overall_returncode @@ -355,6 +458,12 @@ def main(): help='Show detailed output for each file (default: only show warnings/errors)' ) + parser.add_argument( + '--no-plugin', + action='store_true', + help='Do not automatically load the GeneralsGameCode clang-tidy plugin' + ) + parser.add_argument( 'clang_tidy_args', nargs='*', @@ -390,7 +499,8 @@ def main(): clang_tidy_args, args.fix, args.jobs, - args.verbose + args.verbose, + load_plugin=not args.no_plugin ) compile_commands = load_compile_commands(compile_commands_path) @@ -423,7 +533,8 @@ def main(): clang_tidy_args, args.fix, args.jobs, - args.verbose + args.verbose, + load_plugin=not args.no_plugin ) except Exception as e: