Use new NameConvention in XYCheck

This should catch more cases where names match.
CleverRaven · Apr 28, 2020 · 454eb0b · 454eb0b
1 parent 94889c5
commit 454eb0b
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 26 deletions.
diff --git a/tools/clang-tidy-plugin/Utils.cpp b/tools/clang-tidy-plugin/Utils.cpp
@@ -30,7 +30,7 @@ NameConvention::NameConvention( StringRef xName )
     }
 }
 
-NameConvention::MatchResult NameConvention::Match( StringRef name )
+NameConvention::MatchResult NameConvention::Match( StringRef name ) const
 {
     if( name.empty() ) {
         return None;

diff --git a/tools/clang-tidy-plugin/Utils.h b/tools/clang-tidy-plugin/Utils.h
@@ -104,6 +104,12 @@ inline auto isXParam()
     return matchesName( "[xX]" );
 }
 
+inline auto isYParam()
+{
+    using namespace clang::ast_matchers;
+    return matchesName( "[yY]" );
+}
+
 inline bool isPointType( const CXXRecordDecl *R )
 {
     if( !R ) {
@@ -127,7 +133,7 @@ class NameConvention
             None
         };
 
-        MatchResult Match( StringRef name );
+        MatchResult Match( StringRef name ) const;
 
         bool operator!() const {
             return !valid;

diff --git a/tools/clang-tidy-plugin/XYCheck.cpp b/tools/clang-tidy-plugin/XYCheck.cpp
@@ -9,6 +9,8 @@
 #include <clang/Basic/DiagnosticIDs.h>
 #include <clang/Basic/Specifiers.h>
 
+#include "Utils.h"
+
 using namespace clang::ast_matchers;
 
 namespace clang
@@ -23,10 +25,10 @@ void XYCheck::registerMatchers( MatchFinder *Finder )
     Finder->addMatcher(
         fieldDecl(
             hasType( asString( "int" ) ),
-            matchesName( "x$" ),
+            isXParam(),
             hasParent(
                 cxxRecordDecl(
-                    forEachDescendant( fieldDecl( matchesName( "y$" ) ).bind( "yfield" ) )
+                    forEachDescendant( fieldDecl( isYParam() ).bind( "yfield" ) )
                 ).bind( "record" )
             )
         ).bind( "xfield" ),
@@ -35,12 +37,8 @@ void XYCheck::registerMatchers( MatchFinder *Finder )
     Finder->addMatcher(
         parmVarDecl(
             hasType( asString( "int" ) ),
-            matchesName( "x$" ),
-            hasAncestor(
-                functionDecl(
-                    hasAnyParameter( parmVarDecl( matchesName( "y$" ) ).bind( "yparam" ) )
-                ).bind( "function" )
-            )
+            isXParam(),
+            hasAncestor( functionDecl().bind( "function" ) )
         ).bind( "xparam" ),
         this
     );
@@ -54,16 +52,14 @@ static void CheckField( XYCheck &Check, const MatchFinder::MatchResult &Result )
     if( !XVar || !YVar || !Record ) {
         return;
     }
-    llvm::StringRef XPrefix = XVar->getName().drop_back();
-    llvm::StringRef YPrefix = YVar->getName().drop_back();
-    if( XPrefix != YPrefix ) {
+    const NameConvention NameMatcher( XVar->getName() );
+    if( NameMatcher.Match( YVar->getName() ) != NameConvention::YName ) {
         return;
     }
 
     const FieldDecl *ZVar = nullptr;
     for( FieldDecl *Field : Record->fields() ) {
-        StringRef Name = Field->getName();
-        if( Name.endswith( "z" ) && Name.drop_back() == XPrefix ) {
+        if( NameMatcher.Match( Field->getName() ) == NameConvention::ZName ) {
             ZVar = Field;
             break;
         }
@@ -94,28 +90,28 @@ static void CheckField( XYCheck &Check, const MatchFinder::MatchResult &Result )
 static void CheckParam( XYCheck &Check, const MatchFinder::MatchResult &Result )
 {
     const ParmVarDecl *XParam = Result.Nodes.getNodeAs<ParmVarDecl>( "xparam" );
-    const ParmVarDecl *YParam = Result.Nodes.getNodeAs<ParmVarDecl>( "yparam" );
     const FunctionDecl *Function = Result.Nodes.getNodeAs<FunctionDecl>( "function" );
-    if( !XParam || !YParam || !Function ) {
+    if( !XParam || !Function ) {
         return;
     }
-    llvm::StringRef XPrefix = XParam->getName().drop_back();
+    const NameConvention NameMatcher( XParam->getName() );
 
+    const ParmVarDecl *YParam = nullptr;
     const ParmVarDecl *ZParam = nullptr;
     for( ParmVarDecl *Parameter : Function->parameters() ) {
-        StringRef Name = Parameter->getName();
-        if( Name.drop_back() == XPrefix ) {
-            if( Name.endswith( "z" ) ) {
+        switch( NameMatcher.Match( Parameter->getName() ) ) {
+            case NameConvention::ZName:
                 ZParam = Parameter;
-            }
-            if( Name.endswith( "y" ) ) {
+                break;
+            case NameConvention::YName:
                 YParam = Parameter;
-            }
+                break;
+            default:
+                break;
         }
     }
 
-    llvm::StringRef YPrefix = YParam->getName().drop_back();
-    if( XPrefix != YPrefix ) {
+    if( !YParam ) {
         return;
     }
 

diff --git a/tools/clang-tidy-plugin/test/xy-params.cpp b/tools/clang-tidy-plugin/test/xy-params.cpp
@@ -27,3 +27,6 @@ int x1 = h<1>( 0, 0 );
 
 // Verify that there are no warnings for non-int types
 void i( float x, float y );
+
+void f5( int x1, int y1 );
+// CHECK-MESSAGES: warning: 'f5' has parameters 'x1' and 'y1'.  Consider combining into a single point parameter. [cata-xy]