Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce localized sorting 2 #40041

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,9 @@ std::vector<std::string> find_file_if_bfs( const std::string &root_path,

// Keep files and directories to recurse ordered consistently
// by sorting from the old end to the new end.
// NOLINTNEXTLINE(cata-use-localized-sorting)
std::sort( std::begin( directories ) + n_dirs, std::end( directories ) );
// NOLINTNEXTLINE(cata-use-localized-sorting)
std::sort( std::begin( results ) + n_results, std::end( results ) );
}

Expand Down
5 changes: 3 additions & 2 deletions src/main_menu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -468,10 +468,11 @@ void main_menu::load_char_templates()
true ) ) {
path = native_to_utf8( path );
path.erase( path.find( ".template" ), std::string::npos );
path.erase( 0, path.find_last_of( "\\//" ) + 1 );
path.erase( 0, path.find_last_of( "\\/" ) + 1 );
templates.push_back( path );
}
std::sort( templates.begin(), templates.end(), std::greater<std::string>() );
std::sort( templates.begin(), templates.end(), localized_compare );
std::reverse( templates.begin(), templates.end() );
}

bool main_menu::opening_screen()
Expand Down
2 changes: 1 addition & 1 deletion src/martialarts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1465,7 +1465,7 @@ bool ma_style_callback::key( const input_context &ctxt, const input_event &event
std::transform( ma.weapons.begin(), ma.weapons.end(),
std::back_inserter( weapons ), []( const std::string & wid )-> std::string { return item::nname( wid ); } );
// Sorting alphabetically makes it easier to find a specific weapon
std::sort( weapons.begin(), weapons.end() );
std::sort( weapons.begin(), weapons.end(), localized_compare );
// This removes duplicate names (e.g. a real weapon and a replica sharing the same name) from the weapon list.
auto last = std::unique( weapons.begin(), weapons.end() );
weapons.erase( last, weapons.end() );
Expand Down
4 changes: 2 additions & 2 deletions src/requirements.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ std::string requirement_data::print_all_objs( const std::string &header,
[]( const T & t ) {
return t.to_string();
} );
std::sort( alternatives.begin(), alternatives.end() );
std::sort( alternatives.begin(), alternatives.end(), localized_compare );
buffer += join( alternatives, _( " or " ) );
}
if( buffer.empty() ) {
Expand Down Expand Up @@ -580,7 +580,7 @@ std::vector<std::string> requirement_data::get_folded_list( int width,
}
buffer_has.push_back( text + color_tag );
}
std::sort( list_as_string.begin(), list_as_string.end() );
std::sort( list_as_string.begin(), list_as_string.end(), localized_compare );

const std::string separator = colorize( _( " OR " ), c_white );
const std::string unfolded = join( list_as_string, separator );
Expand Down
2 changes: 1 addition & 1 deletion src/scores_ui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ static std::string get_achievements_text( const achievements_tracker &achievemen
std::back_inserter( sortable_achievements ),
[&]( const achievement * ach ) {
achievement_completion comp = achievements.is_completed( ach->id );
return std::make_tuple( comp, ach->description().translated(), ach );
return std::make_tuple( comp, ach->name().translated(), ach );
} );
std::sort( sortable_achievements.begin(), sortable_achievements.end() );
for( const sortable_achievement &ach : sortable_achievements ) {
Expand Down
84 changes: 73 additions & 11 deletions tools/clang-tidy-plugin/UseLocalizedSortingCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,29 +33,53 @@ namespace tidy
namespace cata
{

inline auto isStringType()
{
return qualType( anyOf( asString( "const std::string" ), asString( "std::string" ) ) );
}

inline bool IsString( QualType T )
{
const TagDecl *TTag = T.getTypePtr()->getAsTagDecl();
if( !TTag ) {
return false;
}
StringRef Name = TTag->getName();
return Name == "basic_string";
}

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" )
expr( hasType( isStringType().bind( "arg0Type" ) ) ).bind( "arg0Expr" )
),
hasOverloadedOperatorName( "<" )
).bind( "call" ),
).bind( "opCall" ),
this
);

Finder->addMatcher(
callExpr(
callee( namedDecl( hasAnyName( "std::sort", "std::stable_sort" ) ) ),
argumentCountIs( 2 ),
hasArgument(
0,
expr( hasType( qualType( anyOf(
pointerType( pointee( isStringType().bind( "valueType" ) ) ),
hasDeclaration( decl().bind( "iteratorDecl" ) )
) ) ) )
)
).bind( "sortCall" ),
this
);
}

static void CheckCall( UseLocalizedSortingCheck &Check, const MatchFinder::MatchResult &Result )
static void CheckOpCall( UseLocalizedSortingCheck &Check, const MatchFinder::MatchResult &Result )
{
const CXXOperatorCallExpr *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>( "call" );
const CXXOperatorCallExpr *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>( "opCall" );
const QualType *Arg0Type = Result.Nodes.getNodeAs<QualType>( "arg0Type" );
const Expr *Arg0Expr = Result.Nodes.getNodeAs<Expr>( "arg0Expr" );
if( !Call || !Arg0Type || !Arg0Expr ) {
Expand All @@ -72,9 +96,47 @@ static void CheckCall( UseLocalizedSortingCheck &Check, const MatchFinder::Match
"translations.h." ) << *Arg0Type;
}

static void CheckSortCall( UseLocalizedSortingCheck &Check, const MatchFinder::MatchResult &Result )
{
const CallExpr *Call = Result.Nodes.getNodeAs<CallExpr>( "sortCall" );
const QualType *BoundValueType = Result.Nodes.getNodeAs<QualType>( "valueType" );
const TagDecl *IteratorDecl = Result.Nodes.getNodeAs<TagDecl>( "iteratorDecl" );
if( !Call || ( !BoundValueType && !IteratorDecl ) ) {
return;
}

QualType ValueType;

if( IteratorDecl ) {
//Check.diag( Call->getBeginLoc(), "Iterator type %0" ) << IteratorDecl;

for( const Decl *D : IteratorDecl->decls() ) {
if( const TypedefNameDecl *ND = dyn_cast<TypedefNameDecl>( D ) ) {
if( ND->getName() == "value_type" ) {
ValueType = ND->getUnderlyingType();
break;
}
}
}

if( !IsString( ValueType ) ) {
return;
}
}

if( BoundValueType ) {
ValueType = *BoundValueType;
}

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

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

} // namespace cata
Expand Down
30 changes: 30 additions & 0 deletions tools/clang-tidy-plugin/test/use-localized-sorting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,16 @@ 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;

template<class RandomIt>
void sort( RandomIt first, RandomIt last );

}

template<class T>
struct iterator {
using value_type = T;
};

class NonString
{
};
Expand All @@ -39,3 +47,25 @@ bool f1( const NonString &l, const NonString &r )
{
return l < r;
}

bool sort0( const std::string *start, const std::string *end )
{
std::sort( start, end );
// CHECK-MESSAGES: warning: Raw sort of 'const std::string' (aka 'const basic_string<char>'). For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
}

bool sort1( const NonString *start, const NonString *end )
{
std::sort( start, end );
}

bool sortit0( iterator<std::string> start, iterator<std::string> end )
{
std::sort( start, end );
// CHECK-MESSAGES: warning: Raw sort of 'std::basic_string<char, std::char_traits<char>, std::allocator<char> >'. For UI purposes please use localized_compare from translations.h. [cata-use-localized-sorting]
}

bool sortit1( iterator<NonString> start, iterator<NonString> end )
{
std::sort( start, end );
}