Skip to content

Commit

Permalink
Initial implementation of localized sort check
Browse files Browse the repository at this point in the history
The intention is to detect cases where a simple string comparison is
being performed, but a localized comparison would be more appropriate.
  • Loading branch information
jbytheway committed Apr 30, 2020
1 parent 60cf7b5 commit 60372f0
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 0 deletions.
1 change: 1 addition & 0 deletions tools/clang-tidy-plugin/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ add_library(
TestFilenameCheck.cpp
TextStyleCheck.cpp
TranslatorCommentsCheck.cpp
UseLocalizedSortingCheck.cpp
UseNamedPointConstantsCheck.cpp
UsePointApisCheck.cpp
UsePointArithmeticCheck.cpp
Expand Down
2 changes: 2 additions & 0 deletions tools/clang-tidy-plugin/CataTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "TestFilenameCheck.h"
#include "TextStyleCheck.h"
#include "TranslatorCommentsCheck.h"
#include "UseLocalizedSortingCheck.h"
#include "UseNamedPointConstantsCheck.h"
#include "UsePointApisCheck.h"
#include "UsePointArithmeticCheck.h"
Expand Down Expand Up @@ -39,6 +40,7 @@ class CataModule : public ClangTidyModule
CheckFactories.registerCheck<TestFilenameCheck>( "cata-test-filename" );
CheckFactories.registerCheck<TextStyleCheck>( "cata-text-style" );
CheckFactories.registerCheck<TranslatorCommentsCheck>( "cata-translator-comments" );
CheckFactories.registerCheck<UseLocalizedSortingCheck>( "cata-use-localized-sorting" );
CheckFactories.registerCheck<UseNamedPointConstantsCheck>(
"cata-use-named-point-constants" );
CheckFactories.registerCheck<UsePointApisCheck>( "cata-use-point-apis" );
Expand Down
82 changes: 82 additions & 0 deletions tools/clang-tidy-plugin/UseLocalizedSortingCheck.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
#include "UseLocalizedSortingCheck.h"

#include <algorithm>
#include <clang/AST/ASTContext.h>
#include <clang/AST/Decl.h>
#include <clang/AST/DeclBase.h>
#include <clang/AST/DeclTemplate.h>
#include <clang/AST/Expr.h>
#include <clang/AST/ExprCXX.h>
#include <clang/AST/Type.h>
#include <clang/ASTMatchers/ASTMatchFinder.h>
#include <clang/ASTMatchers/ASTMatchers.h>
#include <clang/ASTMatchers/ASTMatchersInternal.h>
#include <clang/Basic/Diagnostic.h>
#include <clang/Basic/DiagnosticIDs.h>
#include <clang/Basic/LLVM.h>
#include <clang/Basic/SourceLocation.h>
#include <clang/Lex/Lexer.h>
#include <climits>
#include <llvm/ADT/Twine.h>
#include <llvm/Support/Casting.h>
#include <string>

#include "Utils.h"
#include "clang/Basic/OperatorKinds.h"

using namespace clang::ast_matchers;

namespace clang
{
namespace tidy
{
namespace cata
{

void UseLocalizedSortingCheck::registerMatchers( MatchFinder *Finder )
{
Finder->addMatcher(
cxxOperatorCallExpr(
hasArgument(
0,
expr(
hasType(
qualType(
anyOf( asString( "const std::string" ), asString( "std::string" ) )
).bind( "arg0Type" )
)
).bind( "arg0Expr" )
),
hasOverloadedOperatorName( "<" )
).bind( "call" ),
this
);
}

static void CheckCall( UseLocalizedSortingCheck &Check, const MatchFinder::MatchResult &Result )
{
const CXXOperatorCallExpr *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>( "call" );
const QualType *Arg0Type = Result.Nodes.getNodeAs<QualType>( "arg0Type" );
const Expr *Arg0Expr = Result.Nodes.getNodeAs<Expr>( "arg0Expr" );
if( !Call || !Arg0Type || !Arg0Expr ) {
return;
}

StringRef Arg0Text = getText( Result, Arg0Expr );
if( Arg0Text.endswith( "id" ) ) {
return;
}

Check.diag( Call->getBeginLoc(),
"Raw comparison of %0. For UI purposes please use localized_compare from "
"translations.h." ) << *Arg0Type;
}

void UseLocalizedSortingCheck::check( const MatchFinder::MatchResult &Result )
{
CheckCall( *this, Result );
}

} // namespace cata
} // namespace tidy
} // namespace clang
32 changes: 32 additions & 0 deletions tools/clang-tidy-plugin/UseLocalizedSortingCheck.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#ifndef CATA_TOOLS_CLANG_TIDY_PLUGIN_USELOCALIZEDSORTINGCHECK_H
#define CATA_TOOLS_CLANG_TIDY_PLUGIN_USELOCALIZEDSORTINGCHECK_H

#include <clang/ASTMatchers/ASTMatchFinder.h>
#include <llvm/ADT/StringRef.h>

#include "ClangTidy.h"

namespace clang
{

namespace tidy
{
class ClangTidyContext;

namespace cata
{

class UseLocalizedSortingCheck : public ClangTidyCheck
{
public:
UseLocalizedSortingCheck( StringRef Name, ClangTidyContext *Context )
: ClangTidyCheck( Name, Context ) {}
void registerMatchers( ast_matchers::MatchFinder *Finder ) override;
void check( const ast_matchers::MatchFinder::MatchResult &Result ) override;
};

} // namespace cata
} // namespace tidy
} // namespace clang

#endif // CATA_TOOLS_CLANG_TIDY_PLUGIN_USELOCALIZEDSORTINGCHECK_H
41 changes: 41 additions & 0 deletions tools/clang-tidy-plugin/test/use-localized-sorting.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// RUN: %check_clang_tidy %s cata-use-localized-sorting %t -- -plugins=%cata_plugin -- -isystem %cata_include

namespace std
{

template<class T>
struct allocator;

template<class CharT>
class char_traits;

template <
class CharT,
class Traits = std::char_traits<CharT>,
class Allocator = std::allocator<CharT>
> class basic_string;

typedef basic_string<char> string;

template<class CharT, class Traits, class Alloc>
bool operator<( const std::basic_string<CharT, Traits, Alloc> &lhs,
const std::basic_string<CharT, Traits, Alloc> &rhs ) noexcept;

}

class NonString
{
};

bool operator<( const NonString &, const NonString &rhs ) noexcept;

bool f0( const std::string &l, const std::string &r )
{
return l < r;
// CHECK-MESSAGES: warning: Raw comparison of 'const std::string' (aka 'const basic_string<char>'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
}

bool f1( const NonString &l, const NonString &r )
{
return l < r;
}

0 comments on commit 60372f0

Please sign in to comment.