malcolm.parsons updated this revision to Diff 74131.
malcolm.parsons added a comment.

Only insert whitespace when removing stars


https://reviews.llvm.org/D25406

Files:
  clang-tidy/modernize/UseAutoCheck.cpp
  clang-tidy/modernize/UseAutoCheck.h
  test/clang-tidy/modernize-use-auto-new-remove-stars.cpp

Index: test/clang-tidy/modernize-use-auto-new-remove-stars.cpp
===================================================================
--- test/clang-tidy/modernize-use-auto-new-remove-stars.cpp
+++ test/clang-tidy/modernize-use-auto-new-remove-stars.cpp
@@ -6,8 +6,6 @@
 
 class MyDerivedType : public MyType {};
 
-// FIXME: the replacement sometimes results in two consecutive spaces after
-// the word 'auto' (due to the presence of spaces at both sides of '*').
 void auto_new() {
   MyType *a_new = new MyType();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
@@ -29,15 +27,15 @@
   // not "MyType * const".
   static MyType * const d_static = new MyType();
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use auto when initializing with new
-  // CHECK-FIXES: static auto  const d_static = new MyType();
+  // CHECK-FIXES: static auto const d_static = new MyType();
 
   MyType * const a_const = new MyType();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
-  // CHECK-FIXES: auto  const a_const = new MyType();
+  // CHECK-FIXES: auto const a_const = new MyType();
 
   MyType * volatile vol = new MyType();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
-  // CHECK-FIXES: auto  volatile vol = new MyType();
+  // CHECK-FIXES: auto volatile vol = new MyType();
 
   struct SType {} *stype = new SType;
 
@@ -81,15 +79,15 @@
 
     int_p a = new int;
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
-    // CHECK-FIXES: auto  a = new int;
+    // CHECK-FIXES: auto a = new int;
     int_p *b = new int*;
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
     // CHECK-FIXES: auto b = new int*;
 
     // Test for typedefs in declarations lists.
     int_p c = new int, d = new int;
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
-    // CHECK-FIXES: auto  c = new int, d = new int;
+    // CHECK-FIXES: auto c = new int, d = new int;
 
     // Different types should not be transformed.
     int_p e = new int, *f = new int_p;
Index: clang-tidy/modernize/UseAutoCheck.h
===================================================================
--- clang-tidy/modernize/UseAutoCheck.h
+++ clang-tidy/modernize/UseAutoCheck.h
@@ -25,7 +25,7 @@
 
 private:
   void replaceIterators(const DeclStmt *D, ASTContext *Context);
-  void replaceNew(const DeclStmt *D, ASTContext *Context);
+  void replaceNew(const DeclStmt *D, ASTContext *Context, SourceManager &SM);
 
   const bool RemoveStars;
 };
Index: clang-tidy/modernize/UseAutoCheck.cpp
===================================================================
--- clang-tidy/modernize/UseAutoCheck.cpp
+++ clang-tidy/modernize/UseAutoCheck.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
 
 using namespace clang;
 using namespace clang::ast_matchers;
@@ -313,7 +314,8 @@
       << FixItHint::CreateReplacement(Range, "auto");
 }
 
-void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) {
+void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context,
+                              SourceManager &SM) {
   const auto *FirstDecl = dyn_cast<VarDecl>(*D->decl_begin());
   // Ensure that there is at least one VarDecl within the DeclStmt.
   if (!FirstDecl)
@@ -371,18 +373,23 @@
   auto Diag = diag(Range.getBegin(), "use auto when initializing with new"
                                      " to avoid duplicating the type name");
 
+  SourceLocation Next =
+      Lexer::getLocForEndOfToken(Range.getEnd(), 0, SM, Context->getLangOpts());
+  bool Whitespace = isWhitespace(*FullSourceLoc(Next, SM).getCharacterData());
+
   // Space after 'auto' to handle cases where the '*' in the pointer type is
   // next to the identifier. This avoids changing 'int *p' into 'autop'.
-  Diag << FixItHint::CreateReplacement(Range, RemoveStars ? "auto " : "auto")
+  Diag << FixItHint::CreateReplacement(
+              Range, (Whitespace || !RemoveStars) ? "auto" : "auto ")
        << StarRemovals;
 }
 
 void UseAutoCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *Decl = Result.Nodes.getNodeAs<DeclStmt>(IteratorDeclStmtId)) {
     replaceIterators(Decl, Result.Context);
   } else if (const auto *Decl =
                  Result.Nodes.getNodeAs<DeclStmt>(DeclWithNewId)) {
-    replaceNew(Decl, Result.Context);
+    replaceNew(Decl, Result.Context, *Result.SourceManager);
   } else {
     llvm_unreachable("Bad Callback. No node provided.");
   }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to