Omit needless words: split base names on the last preposition.

For methods with at least a single parameter, split the base name at
the last preposition, so long as there are no other verbs or gerunds
in between. This separates out the action of the method from the
description of the first parameter, the latter of which can still have
needless words removed. This is particularly fun with
appendBezierPath*:

  func appendBezierPath(with _: NSRect)
  func appendBezierPath(withPoints _: NSPointArray, count: Int)
  func appendBezierPathWithOval(`in` _: NSRect)
  func appendBezierPathWithArc(withCenter _: NSPoint, radius: CGFloat,
  startAngle: CGFloat, endAngle: CGFloat, clockwise: Bool)
  func appendBezierPathWithArc(withCenter _: NSPoint, radius: CGFloat,
  startAngle: CGFloat, endAngle: CGFloat)
  func appendBezierPathWithArc(from _: NSPoint, to: NSPoint, radius:
  CGFloat)
  func appendBezierPath(with _: NSGlyph, `in`: NSFont)
  func appendBezierPath(withGlyphs _: UnsafeMutablePointer<NSGlyph>,
  count: Int, `in`: NSFont)
  func appendBezierPath(withPackedGlyphs _: UnsafePointer<Int8>)
  @available(OSX 10.5, *)
  func appendBezierPath(withRoundedRect _: NSRect, xRadius: CGFloat,
  yRadius: CGFloat)

Note: "point" and "name" are terrible verbs.

Swift SVN r31976

lib/Basic/PartsOfSpeech.def

@@ -472,7 +472,6 @@ VERB(mug)
 VERB(multiply)
 VERB(murder)
 VERB(nail)
-VERB(name)
 VERB(need)
 VERB(nest)
 VERB(nod)
@@ -516,7 +515,6 @@ VERB(plant)
 VERB(play)
 VERB(please)
 VERB(plug)
-VERB(point)
 VERB(poke)
 VERB(polish)
 VERB(pop)

lib/Basic/StringExtras.cpp

@@ -440,10 +440,11 @@ StringRef swift::omitNeedlessWords(StringRef name, OmissionTypeName typeName,
                                    StringScratchSpace &scratch) {
   if (name.empty() || typeName.empty()) return name;
 
-  // "usingBlock" -> "body" for block parameters.
+  // "usingBlock" -> "body" and "withBlock" -> "body" for block parameters.
   if ((role == NameRole::FirstParameter ||
        role == NameRole::SubsequentParameter) &&
-      name == "usingBlock" && typeName.Name == "Block")
+      (name == "usingBlock" || name == "withBlock") &&
+      typeName.Name == "Block")
     return "body";
 
   // Get the camel-case words in the name and type name.
@@ -565,19 +566,6 @@ StringRef swift::omitNeedlessWords(StringRef name, OmissionTypeName typeName,
           break;
         }
 
-        // If removing the type information would leave us with a
-        // parameter named "with", lowercase the type information and
-        // keep that instead.
-        if ((role == NameRole::FirstParameter ||
-             role == NameRole::SubsequentParameter) &&
-            *nameWordRevIter == "with" &&
-            std::next(nameWordRevIter) == nameWordRevIterEnd) {
-          name = toLowercaseWord(
-                   name.substr(nameWordRevIter.base().getPosition()),
-                   scratch);
-          break;
-        }
-
         SWIFT_FALLTHROUGH;
 
       case PartOfSpeech::Verb:
@@ -670,66 +658,62 @@ bool swift::omitNeedlessWords(StringRef &baseName,
       : NameRole::FirstParameter;
 
     StringRef name = role == NameRole::BaseName ? baseName : argNames[i];
-    StringRef newName = omitNeedlessWords(name, paramTypes[i], role, scratch);
 
-    // Split the base name on the preposition "with", if it's available.
+    // If there is a preposition in the base name, split the base name
+    // at that preposition.
     if (role == NameRole::BaseName) {
-      // Scan backwards for the preposition "With".
+      // Scan backwards for a preposition.
-      auto nameWords = camel_case::getWords(newName);
+      auto nameWords = camel_case::getWords(name);
       auto nameWordRevIter = nameWords.rbegin(),
         nameWordRevIterEnd = nameWords.rend();
-      while (nameWordRevIter != nameWordRevIterEnd &&
+      bool found = false, done = false;
-             !(*nameWordRevIter == "With")) {
+      while (nameWordRevIter != nameWordRevIterEnd && !done) {
-        ++nameWordRevIter;
+        switch (getPartOfSpeech(*nameWordRevIter)) {
+        case PartOfSpeech::Preposition:
+          found = true;
+          done = true;
+          break;
+
+        case PartOfSpeech::Verb:
+        case PartOfSpeech::Gerund:
+          // Don't skip over verbs or gerunds.
+          done = true;
+          break;
+
+        case PartOfSpeech::Unknown:
+          ++nameWordRevIter;
+          break;
+        }
       }
 
-      // If we found "With" somewhere...
+      // If we found a preposition that's not at the beginning of the
-      if (nameWordRevIter != nameWordRevIterEnd) {
+      // name, split there.
-        // That isn't at the end...
+      if (found) {
-        unsigned afterWithPos = nameWordRevIter.base().getPosition();
+        ++nameWordRevIter;
-        if (afterWithPos > 4) {
+        unsigned splitPos = nameWordRevIter.base().getPosition();
-          // Find the text that follows "with". If there is nothing
+        if (splitPos > 0) {
-          // following "With" in the new name, use the original name.
+          // Update the base name by splitting at the preposition.
-          StringRef remainingName;
-          if (afterWithPos == newName.size())
-            remainingName = name.substr(afterWithPos);
-          else
-            remainingName = newName.substr(afterWithPos);
-
-          // The first argument name is either "body" (for
-          // "WithBlock") or the lowercased text that followed "with".
-          if (remainingName == "Block")
-            argNames[0] = "body";
-          else
-            argNames[0] = toLowercaseWord(remainingName, scratch);
-
-          // The base name is everything up to (but not including) the "with".
-          baseName = name.substr(0, afterWithPos-4);
-
           anyChanges = true;
-          continue;
+          baseName = name.substr(0, splitPos);
+
+          // Create a first argument name with the remainder of the base name,
+          // lowercased.
+          name = toLowercaseWord(name.substr(splitPos), scratch);
+          argNames[0] = name;
+          role = NameRole::FirstParameter;
         }
       }
     }
 
+    // Omit needless words from the name.
+    StringRef newName = omitNeedlessWords(name, paramTypes[i], role, scratch);
     if (name == newName) continue;
 
     anyChanges = true;
 
     // Record this change.
     if (role == NameRole::BaseName) {
-      // If, after dropping type information, the last word of the
+      baseName = newName;
-      // base name is "Using", drop the "Using" from the base name and
-      // use "body" as a label for the first parameter.
-      StringRef remainingName = name.substr(newName.size());
-      if (camel_case::getLastWord(newName) == "Using" &&
-          remainingName == "Block") {
-        argNames[0] = "body";
-        baseName = newName.substr(0, newName.size() - 5);
-      } else {
-        // Otherwise, adopt the new base name.
-        baseName = newName;
-      }
     } else {
       argNames[i] = newName;
     }

test/IDE/print_omit_needless_words.swift

@@ -21,24 +21,24 @@
 // RUN: %target-swift-ide-test(mock-sdk: -sdk %S/../Inputs/clang-importer-sdk -I %t -I %S/../ClangModules/Inputs/custom-modules) -print-module -source-filename %s -module-to-print=CoreCooling -function-definitions=false -prefer-type-repr=true -enable-omit-needless-words -skip-parameter-names > %t.CoreCooling.txt
 // RUN: FileCheck %s -check-prefix=CHECK-CORECOOLING -strict-whitespace < %t.CoreCooling.txt
 
+// Note: Class -> "Class"
+// CHECK-OBJECTIVEC: func isKind(of aClass: AnyClass) -> Bool
+
 // Note: SEL -> "Selector"
 // CHECK-FOUNDATION: func makeObjectsPerform(_: Selector)
 
 // Note: "with" parameters drop the "with".
-// CHECK-FOUNDATION: func makeObjectsPerform(_: Selector, object: AnyObject?)
+// CHECK-FOUNDATION: func makeObjectsPerform(_: Selector, with: AnyObject?)
 
 // Note: id -> "Object".
-// CHECK-FOUNDATION: func indexOf(_: AnyObject) -> Int
+// CHECK-FOUNDATION: func index(of _: AnyObject) -> Int
-
-// Note: Class -> "Class"
-// CHECK-OBJECTIVEC: func isKindOf(aClass: AnyClass) -> Bool
 
 // Note: Pointer-to-struct name matching; "with" splits the first piece.
-// CHECK-FOUNDATION: func copy(zone _: NSZone) -> AnyObject!
+// CHECK-FOUNDATION: func copy(with _: NSZone) -> AnyObject!
 
 // Note: Objective-C type parameter names.
-// CHECK-FOUNDATION: func objectFor(_: NSCopying) -> AnyObject?
+// CHECK-FOUNDATION: func object(`for` _: NSCopying) -> AnyObject?
-// CHECK-FOUNDATION: func removeObjectFor(_: NSCopying)
+// CHECK-FOUNDATION: func removeObject(`for` _: NSCopying)
 
 // Note: Allow argument labels that are keywords.
 // CHECK-FOUNDATION: func setObject(_: AnyObject, `for`: NSCopying)
@@ -53,10 +53,10 @@
 // CHECK-FOUNDATION: func add(_: Double) -> NSNumber
 
 // Note: multi-word enum name matching; "with" splits the first piece.
-// CHECK-FOUNDATION: func someMethod(deprecatedOptions _: NSDeprecatedOptions)
+// CHECK-FOUNDATION: func someMethod(with _: NSDeprecatedOptions)
 
-// Note: class name matching; don't drop "With".
+// Note: class name matching; split at "with".
-// CHECK-FOUNDATION: class func request(string _: String!) -> Self!
+// CHECK-FOUNDATION: class func request(with _: String!) -> Self!
 
 // Note: Make sure NSURL works in various places
 // CHECK-FOUNDATION: open(_: NSURL!, completionHandler: ((Bool) -> Void)!)
@@ -76,7 +76,7 @@
 // CHECK-FOUNDATION: func add(_: [AnyObject])
 
 // Note: Int and Index match.
-// CHECK-FOUNDATION: func sliceFrom(_: Int, to: Int) -> String
+// CHECK-FOUNDATION: func slice(from _: Int, to: Int) -> String
 
 // Note: <result type>By<gerund> --> <gerund>.
 // CHECK-FOUNDATION: func appending(_: String) -> String
@@ -85,7 +85,7 @@
 // CHECK-FOUNDATION: func withString(_: String) -> String
 
 // Note: Splitting on "With".
-// CHECK-FOUNDATION: func URL(addedString _: String) -> NSURL?
+// CHECK-FOUNDATION: func URL(withAddedString _: String) -> NSURL?
 
 // Note: <property type>By<gerund> --> <gerund>.
 // CHECK-FOUNDATION: var deletingLastPathComponent: NSURL? { get }
@@ -95,7 +95,7 @@
 
 // Note: usingBlock -> body
 // CHECK-FOUNDATION: func enumerateObjects(body _: ((AnyObject!, Int, UnsafeMutablePointer<ObjCBool>) -> Void)!)
-// CHECK-FOUNDATION: func enumerateObjects(options _: NSEnumerationOptions, body: ((AnyObject!, Int, UnsafeMutablePointer<ObjCBool>) -> Void)!)
+// CHECK-FOUNDATION: func enumerateObjects(with _: NSEnumerationOptions, body: ((AnyObject!, Int, UnsafeMutablePointer<ObjCBool>) -> Void)!)
 
 // Note: WithBlock -> body
 // CHECK-FOUNDATION: func enumerateObjectsRandomly(body _: ((AnyObject!, Int, UnsafeMutablePointer<ObjCBool>) -> Void)!)
@@ -107,7 +107,7 @@
 // CHECK-APPKIT: func same() -> Self
 
 // Note: Skipping over "3D"
-// CHECK-APPKIT: func drawInAirAt(_: Point3D)
+// CHECK-APPKIT: func drawInAir(at _: Point3D)
 
 // Note: Don't strip names that aren't preceded by a verb or preposition.
 // CHECK-APPKIT: func setTextColor(_: NSColor)

test/Sema/omit_needless_words.swift

@@ -2,8 +2,8 @@
 
 class C1 {
   init(tasteString: String) { } // expected-warning{{'init(tasteString:)' could be named 'init(taste:)'}}{{8-8=taste }}
-  func processWithString(string: String, forInt: Int) { } // expected-warning{{'processWithString(_:forInt:)' could be named 'process(string:for:)'}}{{8-25=process}}{{26-26=string }}{{42-42=for }}
+  func processWithString(string: String, forInt: Int) { } // expected-warning{{'processWithString(_:forInt:)' could be named 'process(with:for:)'}}{{8-25=process}}{{26-26=with }}{{42-42=for }}
-  func processWithInt(value: Int) { } // expected-warning{{'processWithInt' could be named 'process(int:)'}}{{8-22=process}}{{23-23=int }}
+  func processWithInt(value: Int) { } // expected-warning{{'processWithInt' could be named 'process(with:)'}}{{8-22=process}}{{23-23=with }}
 }
 
 extension String {
@@ -13,8 +13,8 @@ extension String {
 
 func callSites(s: String) {
   let c1 = C1(tasteString: "blah") // expected-warning{{'init(tasteString:)' could be named 'init(taste:)'}}{{15-26=taste}}
-  c1.processWithString("a", forInt: 1) // expected-warning{{'processWithString(_:forInt:)' could be named 'process(string:for:)'}}{{6-23=process}}{{24-24=string: }}{{29-35=for}}
+  c1.processWithString("a", forInt: 1) // expected-warning{{'processWithString(_:forInt:)' could be named 'process(with:for:)'}}{{6-23=process}}{{24-24=with: }}{{29-35=for}}
-  c1.processWithInt(5) // expected-warning{{'processWithInt' could be named 'process(int:)'}}{{6-20=process}}{{21-21=int: }}
+  c1.processWithInt(5) // expected-warning{{'processWithInt' could be named 'process(with:)'}}{{6-20=process}}{{21-21=with: }}
   _ = String.randomString // expected-warning{{'randomString' could be named 'random'}}{{14-26=random}}
   _ = s.wonkycasedString // expected-warning{{'wonkycasedString' could be named 'wonkycased'}}{{9-25=wonkycased}}
 }