Selector splitting: don't split selectors in the middle of whitelisted "multi-words".

Swift SVN r16016

Author: DougGregor

include/swift/Basic/StringExtras.h

@@ -18,8 +18,11 @@
 #define SWIFT_BASIC_STRINGEXTRAS_HPP
 
 #include "swift/Basic/LLVM.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include <iterator>
+#include <string>
 
 namespace swift {
   /// Describes the kind of preposition a word is.
@@ -183,6 +186,10 @@ namespace swift {
     /// Retrieve the camelCase words in the given string.
     inline Words getWords(StringRef string) { return Words(string); }
 
+    /// Check whether the two words are the same, ignoring the case of the
+    /// first letter.
+    bool sameWordIgnoreFirstCase(StringRef word1, StringRef word2);
+
     /// Lowercase the first word within the given camelCase string.
     ///
     /// \param string The string to lowercase.
@@ -201,6 +208,46 @@ namespace swift {
     ///
     /// \returns the string in sentence case.
     StringRef toSentencecase(StringRef string, SmallVectorImpl<char> &scratch);
+
+    /// Data structures that stores a set of multi-words, which are camelCase
+    /// word sequences that should be kept together by an algorithm that splits
+    /// up camelCase names.
+    ///
+    /// This data structure stores the multiwords backwards. The key is the last
+    /// word in the multi-word. The element is a vector of multi-words, where
+    /// each element in the vector is a multi-word stored backwards with the
+    /// last word removed.
+    ///
+    /// FIXME: This is a crummy data structure. We should optimize this to avoid
+    /// redundant storage and provide faster lookup.
+    class MultiWordMap {
+      llvm::StringMap<SmallVector<SmallVector<std::string, 1>, 1>> Data;
+
+    public:
+      /// Whether the map is empty.
+      bool empty() const { return Data.empty(); }
+
+      // Basis case: add an entry in multi-word map with \p last as the last word.
+      SmallVectorImpl<std::string> &insert(StringRef last) {
+        auto &words = Data[last];
+        words.push_back({});
+        return words.back();
+      }
+
+      // Recursive case: add the current word to the end of the current
+      // multiword.
+      template<typename ...Args>
+      SmallVectorImpl<std::string> &insert(StringRef current, Args... args) {
+        auto &multiWord = insert(args...);
+        multiWord.push_back(current.str());
+        return multiWord;
+      }
+
+      /// Match the longest (multi-)word possible, advancing the reverse
+      /// iterator to point at the beginning of the longest (multi-)word match.
+      Words::reverse_iterator match(Words::reverse_iterator first,
+                                    Words::reverse_iterator last) const;
+    };
   } // end namespace camel_case
 }
 

lib/Basic/StringExtras.cpp

@@ -15,8 +15,12 @@
 //
 //===----------------------------------------------------------------------===//
 #include "swift/Basic/StringExtras.h"
+#include "swift/Basic/Optional.h"
 #include "clang/Basic/CharInfo.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
+#include <algorithm>
+
 using namespace swift;
 using namespace camel_case;
 
@@ -101,7 +105,17 @@ StringRef camel_case::getLastWord(StringRef string) {
   return *--WordIterator(string, string.size());
 }
 
-StringRef camel_case::toLowercaseWord(StringRef string, 
+bool camel_case::sameWordIgnoreFirstCase(StringRef word1, StringRef word2) {
+  if (word1.size() != word2.size())
+    return false;
+
+  if (clang::toLowercase(word1[0]) != clang::toLowercase(word2[0]))
+    return false;
+
+  return word1.substr(1) == word2.substr(1);
+}
+
+StringRef camel_case::toLowercaseWord(StringRef string,
                                       SmallVectorImpl<char> &scratch) {
   if (string.empty())
     return string;
@@ -136,3 +150,41 @@ StringRef camel_case::toSentencecase(StringRef string,
   scratch.append(string.begin() + 1, string.end());
   return StringRef(scratch.data(), scratch.size());  
 }
+
+Words::reverse_iterator MultiWordMap::match(Words::reverse_iterator first,
+                                            Words::reverse_iterator last) const{
+  assert(first != last && "Empty sequence cannot be matched");
+
+  // If the current word isn't at the start of the sequence, we're done.
+  llvm::SmallString<32> scratch;
+  auto known = Data.find(toLowercaseWord(*first, scratch));
+  if (known == Data.end())
+    return first;
+
+  /// The best result (if we found anything) and the length of the match.
+  Optional<std::pair<Words::reverse_iterator, unsigned>> bestResult;
+  for (const auto &multiword : known->second) {
+    // Match the words in the sequence to the words in this multiword.
+    auto matchNext = first;
+    ++matchNext;
+
+    bool matched = true;
+    for (const auto &word : multiword) {
+      if (matchNext == last || !sameWordIgnoreFirstCase(word, *matchNext++)) {
+        matched = false;
+        break;
+      }
+    }
+
+    if (!matched)
+      continue;
+
+    // If this is the first result, or the previous result was a shorter match,
+    // we just found a better result.
+    if (!bestResult || bestResult->second < multiword.size()) {
+      bestResult = { std::prev(matchNext), multiword.size() };
+    }
+  }
+
+  return bestResult ? bestResult->first : first;
+}

lib/ClangImporter/ClangImporter.cpp

@@ -62,6 +62,9 @@ STATISTIC(NumPrepositionSplitMethodNames,
 STATISTIC(NumWordSplitMethodNames,
           "selectors where the first selector piece was split on the last "
           "word");
+STATISTIC(NumMultiWordSplitMethodNames,
+          "selectors where the first slector piece was split on the last "
+          "multiword");
 STATISTIC(NumPrepositionTrailingFirstPiece,
           "selectors where the first piece ends in a preposition");
 STATISTIC(NumMethodsMissingFirstArgName,
@@ -531,7 +534,6 @@ splitSelectorPieceAt(StringRef selector, unsigned index,
            camel_case::toLowercaseWord(selector.substr(index), buffer) };
 }
 
-  
 /// Split the first selector piece into a function name and first
 /// parameter name, if possible.
 ///
@@ -542,7 +544,9 @@ splitSelectorPieceAt(StringRef selector, unsigned index,
 /// the split. If no split is possible, the function name will be \c
 /// selector and the parameter name will be empty.
 static std::pair<StringRef, StringRef> 
-splitFirstSelectorPiece(StringRef selector,  SmallVectorImpl<char> &scratch) {
+splitFirstSelectorPiece(StringRef selector,
+                        const camel_case::MultiWordMap &multiWords,
+                        SmallVectorImpl<char> &scratch) {
   // Get the camelCase words in this selector.
   auto words = camel_case::getWords(selector);
   if (words.empty())
@@ -574,13 +578,21 @@ splitFirstSelectorPiece(StringRef selector,  SmallVectorImpl<char> &scratch) {
 
   // We did not find a preposition.
 
-  // If there is more than one word, split off the last word.
+  // If there is more than one word, split off the last word, taking care
+  // to avoid splitting in the middle of a multi-word.
   auto last = words.rbegin();
   if (std::next(last) != words.rend()) {
-    ++NumWordSplitMethodNames;
-    return splitSelectorPieceAt(selector,
-                                std::prev(last.base()).getPosition(),
-                                scratch);
+    auto splitPoint = multiWords.match(last, words.rend());
+    auto beforeSplitPoint = std::next(splitPoint);
+    if (beforeSplitPoint != words.rend()) {
+      if (splitPoint == last)
+        ++NumWordSplitMethodNames;
+      else
+        ++NumMultiWordSplitMethodNames;
+      return splitSelectorPieceAt(selector,
+                                  std::prev(splitPoint.base()).getPosition(),
+                                  scratch);
+    }
   }
 
   // Nothing to split.
@@ -614,7 +626,8 @@ static Identifier importArgName(ASTContext &ctx, StringRef name) {
 /// Map an Objective-C selector name to a Swift method name.
 static DeclName mapSelectorName(ASTContext &ctx,
                                 clang::Selector selector,
-                                bool isInitializer) {
+                                bool isInitializer,
+                                const camel_case::MultiWordMap &multiWords) {
   assert(!selector.isNull() && "Null selector?");
 
   // Zero-argument selectors.
@@ -649,6 +662,7 @@ static DeclName mapSelectorName(ASTContext &ctx,
     llvm::SmallString<16> scratch;
     StringRef funcName, firstArgName;
     std::tie(funcName, firstArgName) = splitFirstSelectorPiece(firstPiece,
+                                                               multiWords,
                                                                scratch);
     baseName = ctx.getIdentifier(funcName);
     argumentNames.push_back(importArgName(ctx, firstArgName));
@@ -749,13 +763,23 @@ DeclName ClangImporter::Implementation::importName(clang::Selector selector,
     return known->second;
 
   // Map the selector.
-  auto result = mapSelectorName(SwiftContext, selector, isInitializer);
+  populateMultiWords();
+  auto result = mapSelectorName(SwiftContext, selector, isInitializer,
+                                MultiWords);
 
   // Cache the result and return.
   SelectorMappings[{selector, isInitializer}] = result;
   return result;
 }
 
+void ClangImporter::Implementation::populateMultiWords() {
+  if (!MultiWords.empty())
+    return;
+
+#define MULTI_WORD(...) MultiWords.insert(__VA_ARGS__);
+#include "MultiWords.def"
+}
+
 void ClangImporter::Implementation::populateKnownDesignatedInits() {
   if (!KnownDesignatedInits.empty())
     return;

lib/ClangImporter/ImporterImpl.h

@@ -20,6 +20,7 @@
 #include "swift/ClangImporter/ClangImporter.h"
 #include "swift/AST/LazyResolver.h"
 #include "swift/AST/Type.h"
+#include "swift/Basic/StringExtras.h"
 #include "swift/Basic/Optional.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -230,6 +231,12 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   /// Mapping from Objective-C selectors to method names.
   llvm::DenseMap<std::pair<clang::Selector, char>, DeclName> SelectorMappings;
 
+  /// Multiwords used for selector splitting.
+  camel_case::MultiWordMap MultiWords;
+
+  /// Populate the MultiWords map from \c MultiWords.def.
+  void populateMultiWords();
+
   /// Mapping that describes the designated initializers of
   /// Objective-C classes.
   ///

lib/ClangImporter/MultiWords.def

@@ -0,0 +1,27 @@
+//===--- MultiWords.def - Multiple Word Database ----------------*- C++ -*-===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2015 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See http://swift.org/LICENSE.txt for license information
+// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines multiple-word sequences that should not be split up 
+// as part of Objective-C selector splitting.
+// ===---------------------------------------------------------------------===//
+
+/// MULTI_WORD(Words)
+///   Words: A comma-separated sequence of words that make up
+///   the multi-word.
+
+#ifndef MULTI_WORD
+#  error define MULTI_WORD before including MultiWords.def
+#endif
+
+MULTI_WORD("attributed", "string")
+
+#undef MULTI_WORD

test/Inputs/clang-importer-sdk/usr/include/Foundation.h

@@ -216,3 +216,7 @@ typedef struct _NSRange {
     NSUInteger location;
     NSUInteger length;
 } NSRange;
+
+@interface NSAttributedString : NSString
+- (NSAttributedString *)sliceAttributedString:(NSInteger)startIndex;
+@end

unittests/Basic/StringExtrasTest.cpp

@@ -73,3 +73,26 @@ TEST(ToSentencecaseTest, Words) {
   EXPECT_EQ(camel_case::toSentencecase("URL", scratch), "URL");
   EXPECT_EQ(camel_case::toSentencecase("", scratch), "");
 }
+
+TEST(MultiWordsTest, Match) {
+  camel_case::MultiWordMap multiWords;
+  multiWords.insert("attributed", "string");
+  multiWords.insert("table", "view");
+  multiWords.insert("table", "view", "controller");
+  multiWords.insert("table", "top");
+
+  auto words1 = camel_case::getWords("createTableView");
+  EXPECT_EQ(*multiWords.match(words1.rbegin(), words1.rend()), "Table");
+
+  auto words2 = camel_case::getWords("createTableViewController");
+  EXPECT_EQ(*multiWords.match(words2.rbegin(), words2.rend()), "Table");
+
+  auto words3 = camel_case::getWords("createSubview");
+  EXPECT_EQ(*multiWords.match(words3.rbegin(), words3.rend()), "Subview");
+
+  auto words4 = camel_case::getWords("addView");
+  EXPECT_EQ(*multiWords.match(words4.rbegin(), words4.rend()), "View");
+
+  auto words5 = camel_case::getWords("insertAttributedString");
+  EXPECT_EQ(*multiWords.match(words5.rbegin(), words5.rend()), "Attributed");
+}