[PATCH] D39332: [clang-refactor] Introduce a new rename rule for qualified symbols

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:101
+                              std::string NewQualifiedName) {
+  return QualifiedRenameRule(std::move(OldQualifiedName),
+                             std::move(NewQualifiedName));
----------------
arphaman wrote:
> It might be better to find the declaration (and report error if needed) during in initiation, and then pass the `ND` to the class. Maybe then both `RenameOccurrences` and `QualifiedRenameRule` could subclass from one base class that actually does just this:
> 
> ``` 
> auto USRs = getUSRsForDeclaration(ND, Context.getASTContext());
>   assert(!USRs.empty());
>   return tooling::createRenameAtomicChanges(
>       USRs, NewQualifiedName, Context.getASTContext().getTranslationUnitDecl());
> ```
Done. Introduced a common interface `USRRenameRule`.


================
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:105
+
+const RefactoringDescriptor &QualifiedRenameRuledescribe() {
+  static const RefactoringDescriptor Descriptor = {
----------------
jklaehn wrote:
> missing `::`? → `::describe()`
Good Catch!


https://reviews.llvm.org/D39332