[clangd] Add a tweak refactoring to wrap Objective-C string literals in `NSLocalizedString` macros

[arphaman]

Summary

This is a patch to add a refactoring to Clangd that mimics the existing refactoring action in Xcode that wraps around an Objective-C string literal in an NSLocalizedString macro.

[kadircet]

clang-tools-extra/clangd/unittests/TweakTests.cpp

127

make sure this is rebased on top of http://github.com/llvm/llvm-project/commit/1a9c01c7f6c7e88676440869bbbe9f43fa45b109

129

maybe use raw string literals?

R"(@"teststring")"

[arphaman]

I figured out what I was doing wrong in the tests, and fixed them. This is no longer WIP and is ready for review.

[kadircet]

implementation lgtm with a few nits.

main concern is about the new getlangopts helper

clang-tools-extra/clangd/ParsedAST.h

80 ↗

(On Diff #228548)

can we introduce this helper in a new patch, while changing other occurrences in clangd?

clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp

35

NSLocalizedString(@"description", @"")

70

maybe extract Inputs.AST.getASTContext().getLangOpts() into a variable and make use of it in the following places as well? (line 72 and 75)

clang-tools-extra/clangd/unittests/TweakTests.cpp

131

nit: you can combine all of this into a single test

136

nit: you can combine all of this into a single test

[arphaman]

Fixed nits.

clang-tools-extra/clangd/ParsedAST.h

80 ↗

(On Diff #228548)

Sounds good, I'll do that.

clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp

35

Great catch, thanks!

[kadircet]

Thanks, LGTM!

clang-tools-extra/clangd/ParsedAST.h

80 ↗

(On Diff #228548)

can you also revert this change?

[arphaman]

clang-tools-extra/clangd/ParsedAST.h

80 ↗

(On Diff #228548)

I precommited it in c0ee0224c4cf52bc6ba74dec88b30b850deca523

clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt

add_clang_library(clangDaemonTweaks OBJECT
   AnnotateHighlightings.cpp
   DumpAST.cpp
   DefineInline.cpp
   DefineOutline.cpp
   ExpandAutoType.cpp
   ExpandMacro.cpp
   ExtractFunction.cpp
   ExtractVariable.cpp
+  ObjCLocalizeStringLiteral.cpp
   RawStringLiteral.cpp
   RemoveUsingNamespace.cpp
   SwapIfBranches.cpp
 
   LINK_LIBS
   clangAST
   clangBasic
   clangDaemon
   clangLex
   clangToolingCore
   clangToolingRefactoring
   clangToolingSyntax
   )

clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp

+//===--- ObjcLocalizeStringLiteral.cpp ---------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Logger.h"
+#include "ParsedAST.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ExprObjC.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+/// Wraps an Objective-C string literal with the NSLocalizedString macro.
+/// Before:
+///   @"description"
+///   ^^^
+/// After:
+///   NSLocalizedString(@"description", @"")
+class ObjCLocalizeStringLiteral : public Tweak {
+public:
+  const char *id() const override final;
+  Intent intent() const override { return Intent::Refactor; }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected<Tweak::Effect> apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  const clang::ObjCStringLiteral *Str = nullptr;
+};
+
+REGISTER_TWEAK(ObjCLocalizeStringLiteral)
+
+bool ObjCLocalizeStringLiteral::prepare(const Selection &Inputs) {
+  const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor();
+  if (!N)
+    return false;
+  // Allow the refactoring even if the user selected only the C string part
+  // of the expression.
+  if (N->ASTNode.get<StringLiteral>()) {
+    if (N->Parent)
+      N = N->Parent;
+  }
+  Str = dyn_cast_or_null<ObjCStringLiteral>(N->ASTNode.get<Stmt>());
+  return Str;
+}
+
+Expected<Tweak::Effect>
+ObjCLocalizeStringLiteral::apply(const Selection &Inputs) {
+  auto &SM = Inputs.AST.getSourceManager();
+  auto &LangOpts = Inputs.AST.getASTContext().getLangOpts();
+  auto Reps = tooling::Replacements(tooling::Replacement(
+      SM, CharSourceRange::getCharRange(Str->getBeginLoc()),
+      "NSLocalizedString(", LangOpts));
+  SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+      Str->getEndLoc(), 0, Inputs.AST.getSourceManager(), LangOpts);
+  if (auto Err = Reps.add(tooling::Replacement(
+          SM, CharSourceRange::getCharRange(EndLoc), ", @\"\")", LangOpts)))
+    return std::move(Err);
+  return Effect::mainFileEdit(SM, std::move(Reps));
+}
+
+std::string ObjCLocalizeStringLiteral::title() const {
+  return "Wrap in NSLocalizedString";
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/unittests/TweakTests.cpp

 token)" "\nst^ring\n" "literal")cpp";
   const char *Output = R"cpp(R"(multi
 token
 string
 literal)")cpp";
   EXPECT_EQ(apply(Input), Output);
 }
 
+TWEAK_TEST(ObjCLocalizeStringLiteral);
+TEST_F(ObjCLocalizeStringLiteralTest, Test) {
+  ExtraArgs.push_back("-x");
+  ExtraArgs.push_back("objective-c");
+
+  // Ensure the the action can be initiated in the string literal.
+  EXPECT_AVAILABLE(R"(id x = ^[[@[[^"^t^est^"]]]];)");
+
+  // Ensure that the action can't be initiated in other places.
+  EXPECT_UNAVAILABLE(R"([[i^d ^[[x]] ^= @"test";^]])");
+
+  // Ensure that the action is not available for regular C strings.
+  EXPECT_UNAVAILABLE(R"(const char * x= "^test";)");
+
+  const char *Input = R"(id x = [[@"test"]];)";
+  const char *Output = R"(id x = NSLocalizedString(@"test", @"");)";
+  EXPECT_EQ(apply(Input), Output);
+}
+
 TWEAK_TEST(DumpAST);
 TEST_F(DumpASTTest, Test) {
   EXPECT_AVAILABLE("^int f^oo() { re^turn 2 ^+ 2; }");
   EXPECT_UNAVAILABLE("/*c^omment*/ int foo() return 2 ^ + 2; }");
   EXPECT_THAT(apply("int x = 2 ^+ 2;"),
               AllOf(StartsWith("message:"), HasSubstr("BinaryOperator"),
                     HasSubstr("'+'"), HasSubstr("|-IntegerLiteral"),
                     HasSubstr("<col:9> 'int' 2\n`-IntegerLiteral"),