Revert 262747 "Improve GN public header file checking"

It failed Win x64 Builder:
http://build.chromium.org/p/chromium.win/builders/Win%20x64%20Builder/builds/17043/steps/compile/logs/stdio

> Improve GN public header file checking
> 
> This makes the header checker and include iterator work from InputFiles (which basically just hold the file buffer) rather than just raw strings. This allows us to reference these files from Err.
> 
> Some extra line/char tracking is now in the include iterator so we can actually quote from and annotate the place in the source file where the bad include is, which looks much nicer than "the file blah included blah". It also makes it possible to write much shorter error messages that still make sense.
> 
> This adds visibility checking as previously documented in the help for "public" and a unit test for that.
> 
> This updates the documentation for "public" which was wrong (it referred to patterns which was not used).
> 
> R=scottmg@chromium.org
> 
> Review URL: https://codereview.chromium.org/229423002

TBR=brettw@chromium.org

Review URL: https://codereview.chromium.org/231293003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@262755 0039d316-1c4b-4281-b951-d872f2087c98

BUILD.gn

@@ -32,6 +32,7 @@ group("root") {
     #"//sdch",
     #"//skia",
     #"//third_party/WebKit/Source/platform",
+    "//third_party/freetype2",
     #"//third_party/icu:icudata",
     #"//third_party/leveldatabase",
     "//third_party/libpng",
@@ -46,10 +47,4 @@ group("root") {
     "//ui/gfx/geometry",
     "//url",
   ]
-
-  if (is_linux) {
-    deps += [
-      "//third_party/freetype2",
-    ]
-  }
 }

third_party/freetype2/BUILD.gn

@@ -2,8 +2,6 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-assert(is_linux, "This file should only be depended on from Linux.")
-
 config("freetype2_config") {
   include_dirs = [ "include", "src/include" ]
 }

tools/gn/c_include_iterator.cc

@@ -5,9 +5,6 @@
 #include "tools/gn/c_include_iterator.h"
 
 #include "base/logging.h"
-#include "base/strings/string_util.h"
-#include "tools/gn/input_file.h"
-#include "tools/gn/location.h"
 
 namespace {
 
@@ -50,34 +47,24 @@ bool ShouldCountTowardNonIncludeLines(const base::StringPiece& line) {
     return false;  // Don't count comments.
   if (StartsWith(line, "#"))
     return false;  // Don't count preprocessor.
-  if (base::ContainsOnlyChars(line, base::kWhitespaceASCII))
-    return false;  // Don't count whitespace lines.
   return true;  // Count everything else.
 }
 
 // Given a line, checks to see if it looks like an include or import and
 // extract the path. The type of include is returned. Returns INCLUDE_NONE on
 // error or if this is not an include line.
-//
-// The 1-based character number on the line that the include was found at
-// will be filled into *begin_char.
 IncludeType ExtractInclude(const base::StringPiece& line,
-                           base::StringPiece* path,
-                           int* begin_char) {
+                           base::StringPiece* path) {
   static const char kInclude[] = "#include";
   static const size_t kIncludeLen = arraysize(kInclude) - 1;  // No null.
   static const char kImport[] = "#import";
   static const size_t kImportLen = arraysize(kImport) - 1;  // No null.
 
-  base::StringPiece trimmed = TrimLeadingWhitespace(line);
-  if (trimmed.empty())
-    return INCLUDE_NONE;
-
   base::StringPiece contents;
-  if (StartsWith(trimmed, base::StringPiece(kInclude, kIncludeLen)))
-    contents = TrimLeadingWhitespace(trimmed.substr(kIncludeLen));
-  else if (StartsWith(trimmed, base::StringPiece(kImport, kImportLen)))
-    contents = TrimLeadingWhitespace(trimmed.substr(kImportLen));
+  if (StartsWith(line, base::StringPiece(kInclude, kIncludeLen)))
+    contents = TrimLeadingWhitespace(line.substr(kIncludeLen));
+  else if (StartsWith(line, base::StringPiece(kImport, kImportLen)))
+    contents = TrimLeadingWhitespace(line.substr(kImportLen));
 
   if (contents.empty())
     return INCLUDE_NONE;
@@ -100,64 +87,54 @@ IncludeType ExtractInclude(const base::StringPiece& line,
     return INCLUDE_NONE;
 
   *path = contents.substr(1, terminator_index - 1);
-  // Note: one based so we do "+ 1".
-  *begin_char = static_cast<int>(path->data() - line.data()) + 1;
   return type;
 }
 
 }  // namespace
 
 const int CIncludeIterator::kMaxNonIncludeLines = 10;
 
-CIncludeIterator::CIncludeIterator(const InputFile* input)
-    : input_file_(input),
-      file_(input->contents()),
+CIncludeIterator::CIncludeIterator(const base::StringPiece& file)
+    : file_(file),
       offset_(0),
-      line_number_(0),
       lines_since_last_include_(0) {
 }
 
 CIncludeIterator::~CIncludeIterator() {
 }
 
-bool CIncludeIterator::GetNextIncludeString(base::StringPiece* out,
-                                            LocationRange* location) {
+bool CIncludeIterator::GetNextIncludeString(base::StringPiece* out) {
   base::StringPiece line;
-  int cur_line_number = 0;
   while (lines_since_last_include_ <= kMaxNonIncludeLines &&
-         GetNextLine(&line, &cur_line_number)) {
+         GetNextLine(&line)) {
+    base::StringPiece trimmed = TrimLeadingWhitespace(line);
+    if (trimmed.empty())
+      continue;  // Just ignore all empty lines.
+
     base::StringPiece include_contents;
-    int begin_char;
-    IncludeType type = ExtractInclude(line, &include_contents, &begin_char);
+    IncludeType type = ExtractInclude(trimmed, &include_contents);
     if (type == INCLUDE_USER) {
       // Only count user includes for now.
       *out = include_contents;
-      *location = LocationRange(
-          Location(input_file_, cur_line_number, begin_char),
-          Location(input_file_, cur_line_number,
-                   begin_char + include_contents.size()));
-
       lines_since_last_include_ = 0;
       return true;
     }
 
-    if (ShouldCountTowardNonIncludeLines(line))
+    if (ShouldCountTowardNonIncludeLines(trimmed))
       lines_since_last_include_++;
   }
   return false;
 }
 
-bool CIncludeIterator::GetNextLine(base::StringPiece* line, int* line_number) {
+bool CIncludeIterator::GetNextLine(base::StringPiece* line) {
   if (offset_ == file_.size())
     return false;
 
   size_t begin = offset_;
   while (offset_ < file_.size() && file_[offset_] != '\n')
     offset_++;
-  line_number_++;
 
   *line = file_.substr(begin, offset_ - begin);
-  *line_number = line_number_;
 
   // If we didn't hit EOF, skip past the newline for the next one.
   if (offset_ < file_.size())

tools/gn/c_include_iterator.h

@@ -8,43 +8,32 @@
 #include "base/basictypes.h"
 #include "base/strings/string_piece.h"
 
-class InputFile;
-class LocationRange;
-
 // Iterates through #includes in C source and header files.
 //
 // This only returns includes we want to check, which is user includes with
 // double-quotes: #include "..."
 class CIncludeIterator {
  public:
-  // The InputFile pointed to must outlive this class.
-  CIncludeIterator(const InputFile* input);
+  // The buffer pointed to must outlive this class.
+  CIncludeIterator(const base::StringPiece& file);
   ~CIncludeIterator();
 
-  // Fills in the string with the contents of the next include, and the
-  // location with where it came from, and returns true, or returns false if
-  // there are no more includes.
-  bool GetNextIncludeString(base::StringPiece* out, LocationRange* location);
+  // Fills in the string with the contents of the next include and returns
+  // true, or returns false if there are no more includes.
+  bool GetNextIncludeString(base::StringPiece* out);
 
   // Maximum numbef of non-includes we'll tolerate before giving up. This does
   // not count comments or preprocessor.
   static const int kMaxNonIncludeLines;
 
  private:
-  // Returns false on EOF, otherwise fills in the given line and the one-based
-  // line number into *line_number.
-  bool GetNextLine(base::StringPiece* line, int* line_number);
-
-  const InputFile* input_file_;
+  // Returns false on EOF, otherwise fills in the given line.
+  bool GetNextLine(base::StringPiece* line);
 
-  // This just points into input_file_.contents() for convenience.
   base::StringPiece file_;
 
-  // 0-based offset into the file.
   size_t offset_;
 
-  int line_number_;  // One-based. Indicates the last line we read.
-
   // Number of lines we've processed since seeing the last include (or the
   // beginning of the file) with some exceptions.
   int lines_since_last_include_;

tools/gn/c_include_iterator_unittest.cc

@@ -4,20 +4,6 @@
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "tools/gn/c_include_iterator.h"
-#include "tools/gn/input_file.h"
-#include "tools/gn/location.h"
-
-namespace {
-
-bool RangeIs(const LocationRange& range,
-             int line, int begin_char, int end_char) {
-  return range.begin().line_number() == line &&
-         range.end().line_number() == line &&
-         range.begin().char_offset() == begin_char &&
-         range.end().char_offset() == end_char;
-}
-
-}  // namespace
 
 TEST(CIncludeIterator, Basic) {
   std::string buffer;
@@ -33,30 +19,18 @@ TEST(CIncludeIterator, Basic) {
   buffer.append("\n");
   buffer.append("void SomeCode() {\n");
 
-  InputFile file(SourceFile("//foo.cc"));
-  file.SetContents(buffer);
-
-  CIncludeIterator iter(&file);
+  CIncludeIterator iter(buffer);
 
   base::StringPiece contents;
-  LocationRange range;
-  EXPECT_TRUE(iter.GetNextIncludeString(&contents, &range));
+  EXPECT_TRUE(iter.GetNextIncludeString(&contents));
   EXPECT_EQ("foo/bar.h", contents);
-  EXPECT_TRUE(RangeIs(range, 3, 11, 20)) << range.begin().Describe(true);
-
-  EXPECT_TRUE(iter.GetNextIncludeString(&contents, &range));
+  EXPECT_TRUE(iter.GetNextIncludeString(&contents));
   EXPECT_EQ("foo/baz.h", contents);
-  EXPECT_TRUE(RangeIs(range, 7, 12, 21)) << range.begin().Describe(true);
-
-  EXPECT_TRUE(iter.GetNextIncludeString(&contents, &range));
+  EXPECT_TRUE(iter.GetNextIncludeString(&contents));
   EXPECT_EQ("la/deda.h", contents);
-  EXPECT_TRUE(RangeIs(range, 8, 11, 20)) << range.begin().Describe(true);
-
-  EXPECT_TRUE(iter.GetNextIncludeString(&contents, &range));
+  EXPECT_TRUE(iter.GetNextIncludeString(&contents));
   EXPECT_EQ("weird_mac_import.h", contents);
-  EXPECT_TRUE(RangeIs(range, 9, 10, 28)) << range.begin().Describe(true);
-
-  EXPECT_FALSE(iter.GetNextIncludeString(&contents, &range));
+  EXPECT_FALSE(iter.GetNextIncludeString(&contents));
 }
 
 // Tests that we don't search for includes indefinitely.
@@ -66,14 +40,10 @@ TEST(CIncludeIterator, GiveUp) {
     buffer.append("x\n");
   buffer.append("#include \"foo/bar.h\"\n");
 
-  InputFile file(SourceFile("//foo.cc"));
-  file.SetContents(buffer);
-
   base::StringPiece contents;
-  LocationRange range;
 
-  CIncludeIterator iter(&file);
-  EXPECT_FALSE(iter.GetNextIncludeString(&contents, &range));
+  CIncludeIterator iter(buffer);
+  EXPECT_FALSE(iter.GetNextIncludeString(&contents));
   EXPECT_TRUE(contents.empty());
 }
 
@@ -88,14 +58,10 @@ TEST(CIncludeIterator, DontGiveUp) {
     buffer.append("#preproc\n");
   buffer.append("#include \"foo/bar.h\"\n");
 
-  InputFile file(SourceFile("//foo.cc"));
-  file.SetContents(buffer);
-
   base::StringPiece contents;
-  LocationRange range;
 
-  CIncludeIterator iter(&file);
-  EXPECT_TRUE(iter.GetNextIncludeString(&contents, &range));
+  CIncludeIterator iter(buffer);
+  EXPECT_TRUE(iter.GetNextIncludeString(&contents));
   EXPECT_EQ("foo/bar.h", contents);
 }
 
@@ -115,16 +81,12 @@ TEST(CIncludeIterator, TolerateNonIncludes) {
     buffer.append("#include \"" + include + "\"\n");
   }
 
-  InputFile file(SourceFile("//foo.cc"));
-  file.SetContents(buffer);
-
   base::StringPiece contents;
-  LocationRange range;
 
-  CIncludeIterator iter(&file);
+  CIncludeIterator iter(buffer);
   for (size_t group = 0; group < kGroupCount; group++) {
-    EXPECT_TRUE(iter.GetNextIncludeString(&contents, &range));
+    EXPECT_TRUE(iter.GetNextIncludeString(&contents));
     EXPECT_EQ(include, contents.as_string());
   }
-  EXPECT_FALSE(iter.GetNextIncludeString(&contents, &range));
+  EXPECT_FALSE(iter.GetNextIncludeString(&contents));
 }