Adding more tests to network traffic annotation auditor.

The following tests are added to network traffic annotation auditor and its
unittest:
ReservedUniqueIDs are correctly checked.
Unique ids do not have invalid characters.
Functions that should be annotated and are not annotated are reported.

Bug: 656607
Bug: 690323
Change-Id: I98271fef6032ce58d4b94d85f9d72dd75c9aeff4
Reviewed-on: https://chromium-review.googlesource.com/571707
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487400}

tools/traffic_annotation/auditor/tests/gn_list_negative.txt

+//:All
+//:gn_all
+//chrome/test:browser_tests
+//chrome/test/media_router:media_router_tests
+//remoting:remoting_all
+//remoting:remoting_perftests
+//remoting:remoting_unittests
+//remoting:test_support
+//remoting/host:all
+//remoting/host:all_tests
+//remoting/host:host
+

tools/traffic_annotation/auditor/tests/gn_list_positive.txt

+//:All
+//:blink_tests
+//:chromium_builder_asan
+//:chromium_builder_perf
+//:gn_all
+//:run_webkit_tests
+//:webkit_layout_tests
+//:webkit_layout_tests_exparchive
+//apps:apps
+//apps:test_support
+//apps/ui/views:views
+//chrome:assert_no_deps
+//chrome:browser_dependencies
+//chrome:child_dependencies
+//chrome:chrome
+//chrome:chrome_initial
+//chrome/app:test_support
+//chrome/browser:browser
+//chrome/browser:test_support
+//chrome/browser/apps:apps
+//chrome/browser/devtools:devtools
+//chrome/browser/extensions:extensions
+//chrome/browser/media/router:router
+//chrome/browser/media/router:test_support
+//chrome/browser/media/router/discovery:discovery
+//chrome/browser/safe_browsing:safe_browsing
+

tools/traffic_annotation/auditor/traffic_annotation_auditor.cc

@@ -7,6 +7,7 @@
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/process/launch.h"
+#include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
@@ -160,10 +161,20 @@ std::string AuditorResult::ToText() const {
       error_text.pop_back();
       error_text.pop_back();
       error_text += ".";
-
       return error_text;
     }
 
+    case AuditorResult::ResultType::ERROR_UNIQUE_ID_INVALID_CHARACTER:
+      DCHECK(details_.size());
+      return base::StringPrintf(
+          "Unique id '%s' in '%s:%i' contains an invalid character.",
+          details_[0].c_str(), file_path_.c_str(), line_);
+
+    case AuditorResult::ResultType::ERROR_MISSING_ANNOTATION:
+      DCHECK(details_.size());
+      return base::StringPrintf("Function '%s' in '%s:%i' requires annotation.",
+                                details_[0].c_str(), file_path_.c_str(), line_);
+
     default:
       return std::string();
   }
@@ -520,7 +531,7 @@ void TrafficAnnotationAuditor::CheckDuplicateHashes() {
     AnnotationInstance& instance = extracted_annotations_[index];
 
     // If unique id's hash code is similar to a reserved id, add an error.
-    if (reserved_ids.find(instance.unique_id_hash_code) != reserved_ids.end()) {
+    if (base::ContainsKey(reserved_ids, instance.unique_id_hash_code)) {
       errors_.push_back(AuditorResult(
           AuditorResult::ResultType::ERROR_RESERVED_UNIQUE_ID_HASH_CODE,
           instance.proto.unique_id(), instance.proto.source().file(),
@@ -529,7 +540,7 @@ void TrafficAnnotationAuditor::CheckDuplicateHashes() {
     }
 
     // Find unique ids with similar hash codes.
-    if (unique_ids.find(instance.unique_id_hash_code) == unique_ids.end()) {
+    if (!base::ContainsKey(unique_ids, instance.unique_id_hash_code)) {
       std::vector<unsigned> empty_list;
       unique_ids.insert(
           std::make_pair(instance.unique_id_hash_code, empty_list));
@@ -554,6 +565,84 @@ void TrafficAnnotationAuditor::CheckDuplicateHashes() {
   }
 }
 
+void TrafficAnnotationAuditor::CheckUniqueIDsFormat() {
+  for (const AnnotationInstance& instance : extracted_annotations_) {
+    if (!base::ContainsOnlyChars(base::ToLowerASCII(instance.proto.unique_id()),
+                                 "0123456789_abcdefghijklmnopqrstuvwxyz")) {
+      errors_.push_back(AuditorResult(
+          AuditorResult::ResultType::ERROR_UNIQUE_ID_INVALID_CHARACTER,
+          instance.proto.unique_id(), instance.proto.source().file(),
+          instance.proto.source().line()));
+    }
+  }
+}
+
+void TrafficAnnotationAuditor::CheckAllRequiredFunctionsAreAnnotated() {
+  for (const CallInstance& call : extracted_calls_) {
+    if (!call.is_annotated && !CheckIfCallCanBeUnannotated(call)) {
+      errors_.push_back(
+          AuditorResult(AuditorResult::ResultType::ERROR_MISSING_ANNOTATION,
+                        call.function_name, call.file_path, call.line_number));
+    }
+  }
+}
+
+bool TrafficAnnotationAuditor::CheckIfCallCanBeUnannotated(
+    const CallInstance& call) {
+  // At this stage we do not enforce annotation on native network requests,
+  // hence all calls except those to 'net::URLRequestContext::CreateRequest' and
+  // 'net::URLFetcher::Create' are ignored.
+  if (call.function_name != "net::URLFetcher::Create" &&
+      call.function_name != "net::URLRequestContext::CreateRequest") {
+    return true;
+  }
+
+  // Is in whitelist?
+  if (IsWhitelisted(call.file_path, AuditorException::ExceptionType::MISSING))
+    return true;
+
+  // Already checked?
+  if (base::ContainsKey(checked_dependencies_, call.file_path))
+    return checked_dependencies_[call.file_path];
+
+  std::string gn_output;
+  if (gn_file_for_test_.empty()) {
+    // Check if the file including this function is part of Chrome build.
+    const base::CommandLine::CharType* args[] = {FILE_PATH_LITERAL("gn"),
+                                                 FILE_PATH_LITERAL("refs"),
+                                                 FILE_PATH_LITERAL("--all")};
+
+    base::CommandLine cmdline(3, args);
+    cmdline.AppendArgPath(build_path_);
+    cmdline.AppendArg(call.file_path);
+
+    base::FilePath original_path;
+    base::GetCurrentDirectory(&original_path);
+    base::SetCurrentDirectory(source_path_);
+
+    if (!base::GetAppOutput(cmdline, &gn_output)) {
+      LOG(ERROR) << "Could not run gn to get dependencies.";
+      gn_output.clear();
+    }
+
+    base::SetCurrentDirectory(original_path);
+  } else {
+    if (!base::ReadFileToString(gn_file_for_test_, &gn_output)) {
+      LOG(ERROR) << "Could not load mock gn output file from "
+                 << gn_file_for_test_.MaybeAsASCII().c_str();
+      gn_output.clear();
+    }
+  }
+
+  checked_dependencies_[call.file_path] =
+      gn_output.length() &&
+      gn_output.find("//chrome:chrome") == std::string::npos;
+
+  return checked_dependencies_[call.file_path];
+}
+
 void TrafficAnnotationAuditor::RunAllChecks() {
   CheckDuplicateHashes();
+  CheckUniqueIDsFormat();
+  CheckAllRequiredFunctionsAreAnnotated();
 }
\ No newline at end of file

tools/traffic_annotation/auditor/traffic_annotation_auditor.h

@@ -46,10 +46,15 @@ class AuditorResult {
     ERROR_MISSING,        // A function is called without annotation.
     ERROR_NO_ANNOTATION,  // A function is called with NO_ANNOTATION tag.
     ERROR_SYNTAX,         // Annotation syntax is not right.
-    ERROR_RESERVED_UNIQUE_ID_HASH_CODE,  // A unique id has a hash code similar
-                                         // to a reserved word.
-    ERROR_DUPLICATE_UNIQUE_ID_HASH_CODE  // Two unique ids have similar hash
-                                         // codes.
+    ERROR_RESERVED_UNIQUE_ID_HASH_CODE,   // A unique id has a hash code similar
+                                          // to a reserved word.
+    ERROR_DUPLICATE_UNIQUE_ID_HASH_CODE,  // Two unique ids have similar hash
+                                          // codes.
+    ERROR_UNIQUE_ID_INVALID_CHARACTER,    // A unique id contanins a characer
+                                          // which is not alphanumeric or
+                                          // underline.
+    ERROR_MISSING_ANNOTATION  // A function that requires annotation is not
+                              // annotated.
   };
 
   static const int kNoCodeLineSpecified;
@@ -208,6 +213,16 @@ class TrafficAnnotationAuditor {
   // Checks to see if any unique id or its hash code is duplicated.
   void CheckDuplicateHashes();
 
+  // Checks to see if unique ids only include alphanumeric characters and
+  // underline.
+  void CheckUniqueIDsFormat();
+
+  // Checks to see if all functions that need annotations have one.
+  void CheckAllRequiredFunctionsAreAnnotated();
+
+  // Checks if a call instance can stay not annotated.
+  bool CheckIfCallCanBeUnannotated(const CallInstance& call);
+
   // Preforms all checks on extracted annotations and calls, and adds the
   // results to |errors_|.
   void RunAllChecks();
@@ -233,6 +248,10 @@ class TrafficAnnotationAuditor {
     extracted_annotations_ = annotations;
   }
 
+  void SetExtractedCallsForTest(const std::vector<CallInstance>& calls) {
+    extracted_calls_ = calls;
+  }
+
   const std::vector<CallInstance>& extracted_calls() const {
     return extracted_calls_;
   }
@@ -241,6 +260,14 @@ class TrafficAnnotationAuditor {
 
   void ClearErrorsForTest() { errors_.clear(); }
 
+  void ClearCheckedDependenciesForTest() { checked_dependencies_.clear(); }
+
+  // Sets the path to a file that would be used to mock the output of
+  // 'gn refs --all [build directory] [file path]' in tests.
+  void SetGnFileForTest(const base::FilePath& file_path) {
+    gn_file_for_test_ = file_path;
+  }
+
  private:
   const base::FilePath source_path_;
   const base::FilePath build_path_;
@@ -252,6 +279,9 @@ class TrafficAnnotationAuditor {
 
   std::vector<std::string> ignore_list_[static_cast<int>(
       AuditorException::ExceptionType::EXCEPTION_TYPE_LAST)];
+
+  base::FilePath gn_file_for_test_;
+  std::map<std::string, bool> checked_dependencies_;
 };
 
 #endif  // TOOLS_TRAFFIC_ANNOTATION_AUDITOR_TRAFFIC_ANNOTATION_AUDITOR_H_
\ No newline at end of file

tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc

@@ -199,12 +199,12 @@ int main(int argc, char* argv[]) {
   }
 
   // Dump Errors and Warnings to stdout.
-  // TODO(rhalavati@): The outputs are now limited to syntax errors. Will be
-  // expanded when repository is full compatible.
   const std::vector<AuditorResult>& errors = auditor.errors();
   for (const auto& error : errors) {
-    if (error.type() == AuditorResult::ResultType::ERROR_SYNTAX)
-      printf("Error: %s\n", error.ToText().c_str());
+    printf("%s: %s\n",
+           error.type() == AuditorResult::ResultType::ERROR_SYNTAX ? "Error"
+                                                                   : "Warning",
+           error.ToText().c_str());
   }
 
   return 0;

tools/traffic_annotation/auditor/traffic_annotation_file_filter.cc

@@ -61,7 +61,7 @@ void TrafficAnnotationFileFilter::GetFilesFromGit(
   base::SetCurrentDirectory(source_path);
 
   std::string git_list;
-  if (git_mock_file_for_testing_.empty()) {
+  if (git_file_for_test_.empty()) {
     const base::CommandLine::CharType* args[] =
 #if defined(OS_WIN)
         {FILE_PATH_LITERAL("git.bat"), FILE_PATH_LITERAL("ls-files")};
@@ -76,9 +76,9 @@ void TrafficAnnotationFileFilter::GetFilesFromGit(
       git_list.clear();
     }
   } else {
-    if (!base::ReadFileToString(git_mock_file_for_testing_, &git_list)) {
+    if (!base::ReadFileToString(git_file_for_test_, &git_list)) {
       LOG(ERROR) << "Could not load mock git list file from "
-                 << git_mock_file_for_testing_.MaybeAsASCII().c_str();
+                 << git_file_for_test_.MaybeAsASCII().c_str();
       git_list.clear();
     }
   }

tools/traffic_annotation/auditor/traffic_annotation_file_filter.h

@@ -39,15 +39,15 @@ class TrafficAnnotationFileFilter {
 
   // Sets the path to a file that would be used to mock the output of
   // 'git ls-files' in tests.
-  void SetGitMockFileForTesting(const base::FilePath& file_path) {
-    git_mock_file_for_testing_ = file_path;
+  void SetGitFileForTest(const base::FilePath& file_path) {
+    git_file_for_test_ = file_path;
   }
 
   const std::vector<std::string>& git_files() { return git_files_; }
 
  private:
   std::vector<std::string> git_files_;
-  base::FilePath git_mock_file_for_testing_;
+  base::FilePath git_file_for_test_;
 };
 
 #endif  // TRAFFIC_ANNOTATION_FILE_FILTER_H_
\ No newline at end of file