Cheer up spell-checking code

chrome/browser/spellchecker/feedback_sender_unittest.cc

Index: chrome/browser/spellchecker/feedback_sender_unittest.cc
diff --git a/chrome/browser/spellchecker/feedback_sender_unittest.cc b/chrome/browser/spellchecker/feedback_sender_unittest.cc
index 7f3bf90d4a6b27a51e28dd4c0b24e35ec52a8a5a..809da888421ef323b07d339c8274b8848d9f36ad 100644
--- a/chrome/browser/spellchecker/feedback_sender_unittest.cc
+++ b/chrome/browser/spellchecker/feedback_sender_unittest.cc
@@ -41,18 +41,15 @@ const int kUrlFetcherId = 0;
 
 // Builds a simple spellcheck result.
 SpellCheckResult BuildSpellCheckResult() {
-  return SpellCheckResult(SpellCheckResult::SPELLING,
-                          kMisspellingStart,
-                          kMisspellingLength,
-                          base::UTF8ToUTF16("Hello"));
+  return SpellCheckResult(SpellCheckResult::SPELLING, kMisspellingStart,
+                          kMisspellingLength, base::UTF8ToUTF16("Hello"));
 }
 
 // Returns the number of times that |needle| appears in |haystack| without
 // overlaps. For example, CountOccurences("bananana", "nana") returns 1.
 int CountOccurences(const std::string& haystack, const std::string& needle) {
   int number_of_occurrences = 0;
-  for (size_t pos = haystack.find(needle);
-       pos != std::string::npos;
+  for (size_t pos = haystack.find(needle); pos != std::string::npos;
        pos = haystack.find(needle, pos + needle.length())) {
     ++number_of_occurrences;
   }
@@ -65,7 +62,7 @@ int CountOccurences(const std::string& haystack, const std::string& needle) {
 class FeedbackSenderTest : public testing::Test {
  public:
   FeedbackSenderTest() : ui_thread_(content::BrowserThread::UI, &loop_) {
-    feedback_.reset(new FeedbackSender(NULL, kLanguage, kCountry));
+    feedback_.reset(new FeedbackSender(nullptr, kLanguage, kCountry));
     feedback_->StartFeedbackCollection();
   }
 
@@ -79,7 +76,7 @@ class FeedbackSenderTest : public testing::Test {
     // TODO(rouslan): Remove the command-line switch. http://crbug.com/247726
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kEnableSpellingFeedbackFieldTrial);
-    feedback_.reset(new FeedbackSender(NULL, kLanguage, kCountry));
+    feedback_.reset(new FeedbackSender(nullptr, kLanguage, kCountry));
     feedback_->StartFeedbackCollection();
   }
 
@@ -92,16 +89,14 @@ class FeedbackSenderTest : public testing::Test {
     field_trial_ = base::FieldTrialList::CreateFieldTrial(
         kFeedbackFieldTrialName, kFeedbackFieldTrialEnabledGroupName);
     field_trial_->group();
-    feedback_.reset(new FeedbackSender(NULL, kLanguage, kCountry));
+    feedback_.reset(new FeedbackSender(nullptr, kLanguage, kCountry));
     feedback_->StartFeedbackCollection();
   }
 
   uint32_t AddPendingFeedback() {
     std::vector<SpellCheckResult> results(1, BuildSpellCheckResult());
-    feedback_->OnSpellcheckResults(kRendererProcessId,
-                                   base::UTF8ToUTF16(kText),
-                                   std::vector<SpellCheckMarker>(),
-                                   &results);
+    feedback_->OnSpellcheckResults(kRendererProcessId, base::UTF8ToUTF16(kText),
+                                   std::vector<SpellCheckMarker>(), &results);
     return results[0].hash;
   }
 
@@ -121,8 +116,9 @@ class FeedbackSenderTest : public testing::Test {
                           int number_of_occurrences) const {
     const net::TestURLFetcher* fetcher =
         fetchers_.GetFetcherByID(kUrlFetcherId);
-    return fetcher && CountOccurences(fetcher->upload_data(), data) ==
-                          number_of_occurrences;
+    return fetcher &&
+           CountOccurences(fetcher->upload_data(), data) ==
+               number_of_occurrences;
   }
 
   // Returns true if the feedback sender would be uploading data now. The test
@@ -131,9 +127,7 @@ class FeedbackSenderTest : public testing::Test {
     return !!fetchers_.GetFetcherByID(kUrlFetcherId);
   }
 
-  void ClearUploadData() {
-    fetchers_.RemoveFetcherFromMap(kUrlFetcherId);
-  }
+  void ClearUploadData() { fetchers_.RemoveFetcherFromMap(kUrlFetcherId); }
 
   std::string GetUploadData() const {
     const net::TestURLFetcher* fetcher =
@@ -188,15 +182,19 @@ TEST_F(FeedbackSenderTest, NoActionFeedback) {
   EXPECT_TRUE(UploadDataContains("\"actionType\":\"NO_ACTION\""));
 }
 
+#define SUGGESTIONINDEX 0

[please use gerrit instead, 2016/02/03 23:59:10]

const int kSuggestionIndex = 0;

if possible.

[Kevin Bailey, 2016/02/04 16:34:11]

Rewritten.

+#define STR(x) #x
+#define STREVAL(x) STR(x)

[please use gerrit instead, 2016/02/03 23:59:10]

Let's put the "#define" statements before all of the namespaces to avoid
thinking that namespaces affect scoping of pre-processor defines. (The don't.)

[Kevin Bailey, 2016/02/04 16:34:11]

They're gone.

+

[groby-ooo-7-16, 2016/02/04 03:43:15]

Let's just not do this. I can't see any value in these macros.

[Kevin Bailey, 2016/02/04 16:34:11]

There was an honest to goodness bug in the original code. I was trying to fix it
while preserving the author's intent to use the same symbol in both places. I've
now changed it to use StringPrintf. Let me know if that's preferable (in the
future).

 // Send SELECT feedback message if the user has selected a spelling suggestion.
 TEST_F(FeedbackSenderTest, SelectFeedback) {
   uint32_t hash = AddPendingFeedback();
-  static const int kSuggestionIndex = 0;
-  feedback_->SelectedSuggestion(hash, kSuggestionIndex);
+  feedback_->SelectedSuggestion(hash, SUGGESTIONINDEX);
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId,
                                       std::vector<uint32_t>());
   EXPECT_TRUE(UploadDataContains("\"actionType\":\"SELECT\""));
-  EXPECT_TRUE(UploadDataContains("\"actionTargetIndex\":" + kSuggestionIndex));
+  EXPECT_TRUE(
+      UploadDataContains("\"actionTargetIndex\":" STREVAL(SUGGESTIONINDEX)));
 }
 
 // Send ADD_TO_DICT feedback message if the user has added the misspelled word
@@ -256,19 +254,15 @@ TEST_F(FeedbackSenderTest, ManuallyCorrectedFeedback) {
 TEST_F(FeedbackSenderTest, BatchFeedback) {
   std::vector<SpellCheckResult> results;
   results.push_back(SpellCheckResult(SpellCheckResult::SPELLING,
-                                     kMisspellingStart,
-                                     kMisspellingLength,
+                                     kMisspellingStart, kMisspellingLength,
                                      base::ASCIIToUTF16("Hello")));
   static const int kSecondMisspellingStart = 7;
   static const int kSecondMisspellingLength = 5;
-  results.push_back(SpellCheckResult(SpellCheckResult::SPELLING,
-                                     kSecondMisspellingStart,
-                                     kSecondMisspellingLength,
-                                     base::ASCIIToUTF16("world")));
-  feedback_->OnSpellcheckResults(kRendererProcessId,
-                                 base::UTF8ToUTF16(kText),
-                                 std::vector<SpellCheckMarker>(),
-                                 &results);
+  results.push_back(
+      SpellCheckResult(SpellCheckResult::SPELLING, kSecondMisspellingStart,
+                       kSecondMisspellingLength, base::ASCIIToUTF16("world")));
+  feedback_->OnSpellcheckResults(kRendererProcessId, base::UTF8ToUTF16(kText),
+                                 std::vector<SpellCheckMarker>(), &results);
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId,
                                       std::vector<uint32_t>());
   EXPECT_TRUE(UploadDataContains("\"actionType\":\"NO_ACTION\"", 2));
@@ -309,15 +303,10 @@ TEST_F(FeedbackSenderTest, SameHashFeedback) {
 // 3) Existing markers get new hash identifiers.
 TEST_F(FeedbackSenderTest, SessionExpirationFeedback) {
   std::vector<SpellCheckResult> results(
-      1,
-      SpellCheckResult(SpellCheckResult::SPELLING,
-                       kMisspellingStart,
-                       kMisspellingLength,
-                       base::ASCIIToUTF16("Hello")));
-  feedback_->OnSpellcheckResults(kRendererProcessId,
-                                 base::UTF8ToUTF16(kText),
-                                 std::vector<SpellCheckMarker>(),
-                                 &results);
+      1, SpellCheckResult(SpellCheckResult::SPELLING, kMisspellingStart,

[groby-ooo-7-16, 2016/02/04 03:43:15]

If we're only reformatting all this code to make clang-format happy, let's not.
As I pointed out on the other CL, "git cl format" restricts itself to modified
lines, so formatting changes over time naturally.

If we absolutely want the entire file clang-format compatible, I'd ask that
that's the _only_ change in the CL, not additional fixes like the nullptr
changes. (Fair warning: If we want that, I'll ask we'll do this for all of
spellchecker, and we add a presubmit check for clang-format compatibility. :)

+                          kMisspellingLength, base::ASCIIToUTF16("Hello")));
+  feedback_->OnSpellcheckResults(kRendererProcessId, base::UTF8ToUTF16(kText),
+                                 std::vector<SpellCheckMarker>(), &results);
   uint32_t original_hash = results[0].hash;
   std::vector<uint32_t> remaining_markers(1, original_hash);
 
@@ -347,12 +336,11 @@ TEST_F(FeedbackSenderTest, SessionExpirationFeedback) {
   // document marker hash identifiers.
   std::vector<SpellCheckMarker> original_markers(
       1, SpellCheckMarker(results[0].hash, results[0].location));
-  results[0] = SpellCheckResult(SpellCheckResult::SPELLING,
-                                kMisspellingStart,
-                                kMisspellingLength,
-                                base::ASCIIToUTF16("Hello"));
-  feedback_->OnSpellcheckResults(
-      kRendererProcessId, base::UTF8ToUTF16(kText), original_markers, &results);
+  results[0] =
+      SpellCheckResult(SpellCheckResult::SPELLING, kMisspellingStart,
+                       kMisspellingLength, base::ASCIIToUTF16("Hello"));
+  feedback_->OnSpellcheckResults(kRendererProcessId, base::UTF8ToUTF16(kText),
+                                 original_markers, &results);
   uint32_t updated_hash = results[0].hash;
   EXPECT_NE(updated_hash, original_hash);
   remaining_markers[0] = updated_hash;
@@ -422,9 +410,9 @@ TEST_F(FeedbackSenderTest, FeedbackAPI) {
   scoped_ptr<base::DictionaryValue> actual(static_cast<base::DictionaryValue*>(
       base::JSONReader::Read(actual_data).release()));
   actual->SetString("params.key", "TestDummyKey");
-  base::ListValue* suggestions = NULL;
+  base::ListValue* suggestions = nullptr;
   actual->GetList("params.suggestionInfo", &suggestions);
-  base::DictionaryValue* suggestion = NULL;
+  base::DictionaryValue* suggestion = nullptr;
   suggestions->GetDictionary(0, &suggestion);
   suggestion->SetString("suggestionId", "42");
   suggestion->SetString("timestamp", "9001");
@@ -505,16 +493,13 @@ TEST_F(FeedbackSenderTest, InternalApiVersion) {
 TEST_F(FeedbackSenderTest, MatchDupliateResultsWithExistingMarkers) {
   uint32_t hash = AddPendingFeedback();
   std::vector<SpellCheckResult> results(
-      1,
-      SpellCheckResult(SpellCheckResult::SPELLING,
-                       kMisspellingStart,
-                       kMisspellingLength,
-                       base::ASCIIToUTF16("Hello")));
+      1, SpellCheckResult(SpellCheckResult::SPELLING, kMisspellingStart,
+                          kMisspellingLength, base::ASCIIToUTF16("Hello")));
   std::vector<SpellCheckMarker> markers(
       1, SpellCheckMarker(hash, results[0].location));
   EXPECT_EQ(static_cast<uint32_t>(0), results[0].hash);
-  feedback_->OnSpellcheckResults(
-      kRendererProcessId, base::UTF8ToUTF16(kText), markers, &results);
+  feedback_->OnSpellcheckResults(kRendererProcessId, base::UTF8ToUTF16(kText),
+                                 markers, &results);
   EXPECT_EQ(hash, results[0].hash);
 }
 
@@ -527,18 +512,15 @@ TEST_F(FeedbackSenderTest, MultipleAddToDictFeedback) {
   static const base::string16 kTextWithDuplicates =
       base::ASCIIToUTF16("Helllo world. Helllo world.");
   for (int i = 0; i < kNumberOfSentences; ++i) {
-    results.push_back(SpellCheckResult(SpellCheckResult::SPELLING,
-                                       kMisspellingStart + i * kSentenceLength,
-                                       kMisspellingLength,
-                                       base::ASCIIToUTF16("Hello")));
+    results.push_back(SpellCheckResult(
+        SpellCheckResult::SPELLING, kMisspellingStart + i * kSentenceLength,
+        kMisspellingLength, base::ASCIIToUTF16("Hello")));
   }
   static const int kNumberOfRenderers = 2;
   int last_renderer_process_id = -1;
   for (int i = 0; i < kNumberOfRenderers; ++i) {
-    feedback_->OnSpellcheckResults(kRendererProcessId + i,
-                                   kTextWithDuplicates,
-                                   std::vector<SpellCheckMarker>(),
-                                   &results);
+    feedback_->OnSpellcheckResults(kRendererProcessId + i, kTextWithDuplicates,
+                                   std::vector<SpellCheckMarker>(), &results);
     last_renderer_process_id = kRendererProcessId + i;
   }
   std::vector<uint32_t> remaining_markers;
@@ -573,20 +555,18 @@ TEST_F(FeedbackSenderTest, AddToDictOnlyPending) {
 // Spellcheck results that are out-of-bounds are not added to feedback.
 TEST_F(FeedbackSenderTest, IgnoreOutOfBounds) {
   std::vector<SpellCheckResult> results;
-  results.push_back(SpellCheckResult(
-      SpellCheckResult::SPELLING, 0, 100, base::UTF8ToUTF16("Hello")));
-  results.push_back(SpellCheckResult(
-      SpellCheckResult::SPELLING, 100, 3, base::UTF8ToUTF16("world")));
-  results.push_back(SpellCheckResult(
-      SpellCheckResult::SPELLING, -1, 3, base::UTF8ToUTF16("how")));
-  results.push_back(SpellCheckResult(
-      SpellCheckResult::SPELLING, 0, 0, base::UTF8ToUTF16("are")));
-  results.push_back(SpellCheckResult(
-      SpellCheckResult::SPELLING, 2, -1, base::UTF8ToUTF16("you")));
-  feedback_->OnSpellcheckResults(kRendererProcessId,
-                                 base::UTF8ToUTF16(kText),
-                                 std::vector<SpellCheckMarker>(),
-                                 &results);
+  results.push_back(SpellCheckResult(SpellCheckResult::SPELLING, 0, 100,
+                                     base::UTF8ToUTF16("Hello")));
+  results.push_back(SpellCheckResult(SpellCheckResult::SPELLING, 100, 3,
+                                     base::UTF8ToUTF16("world")));
+  results.push_back(SpellCheckResult(SpellCheckResult::SPELLING, -1, 3,
+                                     base::UTF8ToUTF16("how")));
+  results.push_back(SpellCheckResult(SpellCheckResult::SPELLING, 0, 0,
+                                     base::UTF8ToUTF16("are")));
+  results.push_back(SpellCheckResult(SpellCheckResult::SPELLING, 2, -1,
+                                     base::UTF8ToUTF16("you")));
+  feedback_->OnSpellcheckResults(kRendererProcessId, base::UTF8ToUTF16(kText),
+                                 std::vector<SpellCheckMarker>(), &results);
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId,
                                       std::vector<uint32_t>());
   EXPECT_FALSE(IsUploadingData());