Cheer up spell-checking code

chrome/browser/spellchecker/feedback_sender_unittest.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // Unit tests for |FeedbackSender| object.
 
 #include "chrome/browser/spellchecker/feedback_sender.h"
 
 #include <stddef.h>
 #include <stdint.h>
@@ skipping 23 matching lines @@
 const char kCountry[] = "USA";
 const char kLanguage[] = "en";
 const char kText[] = "Helllo world.";
 const int kMisspellingLength = 6;
 const int kMisspellingStart = 0;
 const int kRendererProcessId = 0;
 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;
   }
   return number_of_occurrences;
 }
 
 }  // namespace
 
 // A test fixture to help keep tests simple.
 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();
   }
 
   ~FeedbackSenderTest() override {}
 
  protected:
   // Appends the "--enable-spelling-service-feedback" switch to the
   // command-line.
   void AppendCommandLineSwitch() {
     // The command-line switch is temporary.
     // 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();
   }
 
   // Enables the "SpellingServiceFeedback.Enabled" field trial.
   void EnableFieldTrial() {
     // The field trial is temporary.
     // TODO(rouslan): Remove the field trial. http://crbug.com/247726
     field_trial_list_.reset(
         new base::FieldTrialList(new metrics::SHA1EntropyProvider("foo")));
     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;
   }
 
   void ExpireSession() {
     feedback_->session_start_ =
         base::Time::Now() -
         base::TimeDelta::FromHours(chrome::spellcheck_common::kSessionHours);
   }
 
   bool UploadDataContains(const std::string& data) const {
     const net::TestURLFetcher* fetcher =
         fetchers_.GetFetcherByID(kUrlFetcherId);
     return fetcher && CountOccurences(fetcher->upload_data(), data) > 0;
   }
 
   bool UploadDataContains(const std::string& data,
                           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
   // does not open network connections.
   bool IsUploadingData() const {
     return !!fetchers_.GetFetcherByID(kUrlFetcherId);
   }
 
-  void ClearUploadData() {
-    fetchers_.RemoveFetcherFromMap(kUrlFetcherId);
-  }
+  void ClearUploadData() { fetchers_.RemoveFetcherFromMap(kUrlFetcherId); }
 
   std::string GetUploadData() const {
     const net::TestURLFetcher* fetcher =
         fetchers_.GetFetcherByID(kUrlFetcherId);
     return fetcher ? fetcher->upload_data() : std::string();
   }
 
   scoped_ptr<spellcheck::FeedbackSender> feedback_;
 
  private:
@@ skipping 34 matching lines @@
 
 // Send NO_ACTION feedback message if the marker has been removed from the
 // document.
 TEST_F(FeedbackSenderTest, NoActionFeedback) {
   AddPendingFeedback();
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId,
                                       std::vector<uint32_t>());
   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
 // to the custom dictionary.
 TEST_F(FeedbackSenderTest, AddToDictFeedback) {
   uint32_t hash = AddPendingFeedback();
   feedback_->AddedToDictionary(hash);
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId,
                                       std::vector<uint32_t>());
   EXPECT_TRUE(UploadDataContains("\"actionType\":\"ADD_TO_DICT\""));
@@ skipping 39 matching lines @@
                                       std::vector<uint32_t>());
   EXPECT_TRUE(UploadDataContains("\"actionType\":\"MANUALLY_CORRECTED\""));
   EXPECT_TRUE(UploadDataContains("\"actionTargetValue\":\"" +
                                  kManualCorrection + "\""));
 }
 
 // Send feedback messages in batch.
 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));
 }
 
 // Send a series of PENDING feedback messages and one final NO_ACTION feedback
 // message with the same hash identifier for a single misspelling.
 TEST_F(FeedbackSenderTest, SameHashFeedback) {
   uint32_t hash = AddPendingFeedback();
   std::vector<uint32_t> remaining_markers(1, hash);
@@ skipping 20 matching lines @@
   EXPECT_FALSE(IsUploadingData());
 }
 
 // When a session expires:
 // 1) Pending feedback is finalized and sent to the server in the last message
 //    batch in the session.
 // 2) No feedback is sent until a spellcheck request happens.
 // 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);
 
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId, remaining_markers);
   EXPECT_FALSE(UploadDataContains("\"actionType\":\"NO_ACTION\""));
   EXPECT_TRUE(UploadDataContains("\"actionType\":\"PENDING\""));
   std::string original_hash_string =
       base::StringPrintf("\"suggestionId\":\"%u\"", original_hash);
   EXPECT_TRUE(UploadDataContains(original_hash_string));
   ClearUploadData();
 
   ExpireSession();
 
   // Last message batch in the current session has only finalized messages.
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId, remaining_markers);
   EXPECT_TRUE(UploadDataContains("\"actionType\":\"NO_ACTION\""));
   EXPECT_FALSE(UploadDataContains("\"actionType\":\"PENDING\""));
   EXPECT_TRUE(UploadDataContains(original_hash_string));
   ClearUploadData();
 
   // The next session starts on the next spellchecker request. Until then,
   // there's no more feedback sent.
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId, remaining_markers);
   EXPECT_FALSE(IsUploadingData());
 
   // The first spellcheck request after session expiration creates different
   // 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;
 
   // The first feedback message batch in session |i + 1| has the new document
   // marker hash identifiers.
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId, remaining_markers);
   EXPECT_FALSE(UploadDataContains("\"actionType\":\"NO_ACTION\""));
   EXPECT_TRUE(UploadDataContains("\"actionType\":\"PENDING\""));
   EXPECT_FALSE(UploadDataContains(original_hash_string));
@@ skipping 49 matching lines @@
 
 // The field names and types should correspond to the API.
 TEST_F(FeedbackSenderTest, FeedbackAPI) {
   AddPendingFeedback();
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId,
                                       std::vector<uint32_t>());
   std::string actual_data = GetUploadData();
   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");
   static const std::string expected_data =
       "{\"apiVersion\":\"v2\","
       "\"method\":\"spelling.feedback\","
       "\"params\":"
       "{\"clientName\":\"Chrome\","
       "\"originCountry\":\"USA\","
       "\"key\":\"TestDummyKey\","
@@ skipping 60 matching lines @@
                                       std::vector<uint32_t>());
 
   EXPECT_FALSE(UploadDataContains("\"apiVersion\":\"v2\""));
   EXPECT_TRUE(UploadDataContains("\"apiVersion\":\"v2-internal\""));
 }
 
 // Duplicate spellcheck results should be matched to the existing markers.
 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);
 }
 
 // Adding a word to dictionary should trigger ADD_TO_DICT feedback for every
 // occurrence of that word.
 TEST_F(FeedbackSenderTest, MultipleAddToDictFeedback) {
   std::vector<SpellCheckResult> results;
   static const int kSentenceLength = 14;
   static const int kNumberOfSentences = 2;
   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;
   for (size_t i = 0; i < results.size(); ++i)
     remaining_markers.push_back(results[i].hash);
   feedback_->OnReceiveDocumentMarkers(last_renderer_process_id,
                                       remaining_markers);
   EXPECT_TRUE(UploadDataContains("PENDING", 2));
   EXPECT_FALSE(UploadDataContains("ADD_TO_DICT"));
 
@@ skipping 14 matching lines @@
   feedback_->AddedToDictionary(add_to_dict_hash);
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId,
                                       std::vector<uint32_t>());
   EXPECT_TRUE(UploadDataContains("SELECT", 1));
   EXPECT_TRUE(UploadDataContains("ADD_TO_DICT", 2));
 }
 
 // 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());
 }
 
 // FeedbackSender does not collect and upload feedback when instructed to stop.
 TEST_F(FeedbackSenderTest, CanStopFeedbackCollection) {
   feedback_->StopFeedbackCollection();
   AddPendingFeedback();
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId,
@@ skipping 36 matching lines @@
                         "the chance to work hard at work worth doing."),
       std::vector<SpellCheckMarker>(), &results);
   feedback_->OnReceiveDocumentMarkers(kRendererProcessId,
                                       std::vector<uint32_t>());
   EXPECT_TRUE(
       UploadDataContains(",\"originalText\":\"and away teh best prize\","));
   EXPECT_TRUE(UploadDataContains(",\"misspelledStart\":9,"));
 }
 
 }  // namespace spellcheck