Skip to content

Commit

Permalink
Omit needless words: stop splitting first selector pieces for Boolean…
Browse files Browse the repository at this point in the history 
… parameters.

Swift SVN r32692
  • Loading branch information
@DougGregor
DougGregor committed Oct 14, 2015
1 parent 8d77703 commit 382746c
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 164 deletions.
163 changes: 14 additions & 149 deletions lib/Basic/StringExtras.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,83 +642,6 @@ static StringRef omitNeedlessWords(StringRef name,
return name;
}

/// Whether this word can be used to split a first selector piece where the
/// first parameter is of Boolean type.
static bool canSplitForBooleanParameter(StringRef word) {
return camel_case::sameWordIgnoreFirstCase(word, "with") ||
camel_case::sameWordIgnoreFirstCase(word, "for") ||
camel_case::sameWordIgnoreFirstCase(word, "when") ||
camel_case::sameWordIgnoreFirstCase(word, "and");
}

/// Omit needless words by matching the first argument label at the end of a
/// method's base name.
///
/// \param name The method base name which may be updated in the process.
/// \param argName The argument name, which may be updated in the process.
/// \param scratch Scratch space.
///
/// \return Indicates whether any changes were made.
static bool omitNeedlessWordsMatchingFirstArgumentLabel(
StringRef &name,
StringRef &argName,
StringScratchSpace &scratch) {
if (argName.empty())
return false;

// Get the camel-case words in the name and type name.
auto nameWords = camel_case::getWords(name);
auto argNameWords = camel_case::getWords(argName);

// Match the last words in the method base name to the last words in the
// argument name.
auto nameWordRevIter = nameWords.rbegin(),
nameWordRevIterEnd = nameWords.rend();
auto argNameWordRevIter = argNameWords.rbegin(),
argNameWordRevIterEnd = argNameWords.rend();
bool anyMatches = false;
while (nameWordRevIter != nameWordRevIterEnd &&
argNameWordRevIter != argNameWordRevIterEnd) {
// If the names match, continue.
auto nameWord = *nameWordRevIter;
if (matchNameWordToTypeWord(nameWord, *argNameWordRevIter)) {
anyMatches = true;
++nameWordRevIter;
++argNameWordRevIter;
continue;
}

// Skip extra words in the argument name.
++argNameWordRevIter;
}

// If nothing matched, we're done.
if (!anyMatches) return false;

// If the word before the match is a preposition, grab it as part of the
// argument name.
if (nameWordRevIter != nameWordRevIterEnd &&
getPartOfSpeech(*nameWordRevIter) == PartOfSpeech::Preposition) {
auto nextNameWordRevIter = nameWordRevIter;
++nextNameWordRevIter;
if (nameWordRevIter.base().getPosition() > 0) {
nameWordRevIter = nextNameWordRevIter;
}
}

// Determine where to perform the split.
unsigned splitPos = nameWordRevIter.base().getPosition();
if (splitPos == 0) return false;

// If we would be left with a vacuous argument name, don't split.
if (isVacuousName(name.substr(splitPos))) return false;

// Split into base name/first argument label.
argName = ::toLowercaseWord(name.substr(splitPos), scratch);
name = name.substr(0, splitPos);
return true;
}

/// Determine whether the given word indicates a boolean result.
static bool nameIndicatesBooleanResult(StringRef name) {
for (auto word: camel_case::getWords(name)) {
Expand Down Expand Up @@ -807,53 +730,30 @@ bool swift::omitNeedlessWords(StringRef &baseName,

// If the first parameter has a default argument, and there is a
// preposition in the base name, split the base name at that preposition.
// Alternatively, if the first parameter is of Boolean type and there is
// one of a small set of conjunctions and prepositions, split at that
// conjunction or preposition.
if (role == NameRole::BaseName && argNames[0].empty() &&
(paramTypes[0].hasDefaultArgument() ||
(paramTypes[0].isBoolean() && !isVacuousName(getFirstWord(newName))))){
// Scan backwards for a preposition or Boolean-splitting word.
paramTypes[0].hasDefaultArgument()) {
// Scan backwards for a preposition.
auto nameWords = camel_case::getWords(newName);
auto nameWordRevIter = nameWords.rbegin(),
nameWordRevIterEnd = nameWords.rend();
bool found = false, done = false, isAnd = false;
bool found = false, done = false;
while (nameWordRevIter != nameWordRevIterEnd && !done) {
// If this is a boolean
if (paramTypes[0].isBoolean() &&
canSplitForBooleanParameter(*nameWordRevIter)) {
switch (getPartOfSpeech(*nameWordRevIter)) {
case PartOfSpeech::Preposition:
found = true;
done = true;
if (sameWordIgnoreFirstCase(*nameWordRevIter, "and"))
isAnd = true;
break;

case PartOfSpeech::Verb:
case PartOfSpeech::Gerund:
// Don't skip over verbs or gerunds.
done = true;
break;
}

switch (getPartOfSpeech(*nameWordRevIter)) {
case PartOfSpeech::Preposition:
if (paramTypes[0].hasDefaultArgument())
found = true;

done = true;
break;

case PartOfSpeech::Verb:
case PartOfSpeech::Gerund:
// Don't skip over verbs or gerunds unless we have a Boolean
// first parameter.
if (paramTypes[0].isBoolean()) {
++nameWordRevIter;
break;
}

done = true;
break;

case PartOfSpeech::Unknown:
case PartOfSpeech::AuxiliaryVerb:
++nameWordRevIter;
break;
case PartOfSpeech::Unknown:
case PartOfSpeech::AuxiliaryVerb:
++nameWordRevIter;
break;
}
}

Expand All @@ -865,10 +765,6 @@ bool swift::omitNeedlessWords(StringRef &baseName,
if (splitPos > 0) {
unsigned afterSplitPos = splitPos;

// Adjust to skip the "and", if that's what we matched.
if (isAnd && splitPos + 3 < newName.size())
afterSplitPos += 3;

// Create a first argument name with the remainder of the base name,
// lowercased. If we would end up with a vacuous name, go
// back and get the original.
Expand All @@ -887,37 +783,6 @@ bool swift::omitNeedlessWords(StringRef &baseName,
}
}

// If the first parameter is of Boolean type and we don't have a label for
// the first argument, use the parameter name as the first argument label.
// Don't do this if the first word of the base name is vacuous.
if (role == NameRole::BaseName && argNames[0].empty() &&
paramTypes[0].isBoolean() &&
!isVacuousName(camel_case::getFirstWord(newName)) &&
camel_case::sameWordIgnoreFirstCase(camel_case::getLastWord(name),
camel_case::getLastWord(newName))) {
// Drop the "flag" suffix from the first parameter name, if it's there.
if (camel_case::sameWordIgnoreFirstCase(
camel_case::getLastWord(firstParamName),
"flag")) {
firstParamName = toLowercaseWord(firstParamName.drop_back(4), scratch);
}

// Adopt the first parameter name as the first argument label.
argNames[0] = firstParamName;

// Did anything change?
if (!firstParamName.empty())
anyChanges = true;

// Try to eliminate redundancy between the base name and the first
// parameter name.
if (omitNeedlessWordsMatchingFirstArgumentLabel(newName,
argNames[0],
scratch)) {
anyChanges = true;
}
}

if (name == newName) continue;

// Record this change.
Expand Down
6 changes: 1 addition & 5 deletions test/ClangModules/omit_needless_words.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
import Foundation
import AppKit

func renameFirst(vc: NSViewController) {
vc.dismissAnimated(true) // expected-warning{{dismissAnimated' could be named 'dismiss(animated:)'}}{{6-21=dismiss}}{{22-22=animated: }}
}

func dropDefaultedNil(array: NSArray, sel: Selector,
body: ((AnyObject!, Int, UnsafeMutablePointer<ObjCBool>) -> Void)?) {
array.makeObjectsPerformSelector(sel, withObject: nil) // expected-warning{{'makeObjectsPerformSelector(_:withObject:)' could be named 'makeObjectsPerform(_:withObject:)'}}{{9-35=makeObjectsPerform}}
Expand All @@ -20,7 +16,7 @@ func dropDefaultedNil(array: NSArray, sel: Selector,
func dropDefaultedOptionSet(array: NSArray) {
array.enumerateObjectsWithOptions([]) { obj, idx, stop in print("foo") } // expected-warning{{'enumerateObjectsWithOptions(_:usingBlock:)' could be named 'enumerateObjects(withOptions:usingBlock:)'}}{{9-36=enumerateObjects}}{{36-40=}}
array.enumerateObjectsWithOptions([], usingBlock: { obj, idx, stop in print("foo") }) // expected-warning{{'enumerateObjectsWithOptions(_:usingBlock:)' could be named 'enumerateObjects(withOptions:usingBlock:)'}}{{9-36=enumerateObjects}}{{37-41=}}
array.enumerateObjectsWhileOrderingPizza(true, withOptions: [], usingBlock: { obj, idx, stop in print("foo") }) // expected-warning{{'enumerateObjectsWhileOrderingPizza(_:withOptions:usingBlock:)' could be named 'enumerateObjectsWhileOrdering(pizza:withOptions:usingBlock:)'}}{{48-65=}}{{44-44=pizza: }}
array.enumerateObjectsWhileOrderingPizza(true, withOptions: [], usingBlock: { obj, idx, stop in print("foo") }) // expected-warning{{call to 'enumerateObjectsWhileOrderingPizza(_:withOptions:usingBlock:)' has extraneous arguments that could use defaults}}{{48-65=}}
}

func dropDefaultedWithoutRename(domain: String, code: Int, array: NSArray) {
Expand Down
15 changes: 5 additions & 10 deletions test/IDE/print_omit_needless_words.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@
// Don't introduce default arguments for lone parameters to setters.
// CHECK-FOUNDATION: func setDefaultEnumerationOptions(_: NSEnumerationOptions)

// Introducing argument labels without pruning.
// CHECK-FOUNDATION: func normalizingXMLPreservingComments(preserve _: Bool)
// CHECK-FOUNDATION: func normalizingXMLPreservingComments(_: Bool)

// Collection element types.
// CHECK-FOUNDATION: func adding(_: AnyObject) -> Set<NSObject>
Expand Down Expand Up @@ -183,18 +182,14 @@
// beginning and the end of a property.
// CHECK-APPKIT: var flattening: NSBezierPath { get }

// Introducing argument labels and pruning the base name.
// CHECK-APPKIT: func dismiss(animated _: Bool)
// CHECK-APPKIT: func dismissAnimated(_: Bool)

// Introducing argument labels and pruning the base name with a preposition.
// CHECK-APPKIT: func shouldCollapseAutoExpandedItems(forDeposited _: Bool) -> Bool
// CHECK-APPKIT: func shouldCollapseAutoExpandedItemsForDeposited(_: Bool) -> Bool

// Introducing argument labels and pruning the base name.
// CHECK-APPKIT: func rectForCancelButton(whenCentered _: Bool)
// CHECK-APPKIT: func rectForCancelButtonWhenCentered(_: Bool)

// Introducing argument labels and pruning the base name by splitting
// at "and".
// CHECK-APPKIT: func openUntitledDocument(display _: Bool)
// CHECK-APPKIT: func openUntitledDocumentAndDisplay(_: Bool)

// Don't strip due to weak type information.
// CHECK-APPKIT: func setContentHuggingPriority(_: NSLayoutPriority)
Expand Down

0 comments on commit 382746c

Please sign in to comment.