Add an AddExtension() method in FilePath

This provides an AddExtension() method in FilePath, which will append an
extension to the FilePath. This provides clearer, more deliberate functionality
than AppendASCII(), and allows us to add extensions without using
ReplaceExtension, which will not work on files which already have extension (or
appear to, e.g. temp files).

BUG=NONE
TEST=FilePathTest.AddExtension (added), previous FilePathTests.


Review URL: http://codereview.chromium.org/10067002

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

3 files changed, 73 insertions, 22 deletions

diff --git a/base/file_path.cc b/base/file_path.cc
index 3b1fca4..b930898 100644
--- a/base/file_path.cc
+++ b/base/file_path.cc
@@ -159,6 +159,17 @@ StringType::size_type ExtensionSeparatorPosition(const StringType& path) {
   return last_dot;
 }
 
+// Returns true if path is "", ".", or "..".
+bool IsEmptyOrSpecialCase(const StringType& path) {
+  // Special cases "", ".", and ".."
+  if (path.empty() || path == FilePath::kCurrentDirectory ||
+      path == FilePath::kParentDirectory) {
+    return true;
+  }
+
+  return false;
+}
+
 }  // namespace
 
 FilePath::FilePath() {
@@ -375,19 +386,9 @@ FilePath FilePath::InsertBeforeExtension(const StringType& suffix) const {
   if (suffix.empty())
     return FilePath(path_);
 
-  if (path_.empty())
+  if (IsEmptyOrSpecialCase(BaseName().value()))
     return FilePath();
 
-  StringType base = BaseName().value();
-  if (base.empty())
-    return FilePath();
-  if (*(base.end() - 1) == kExtensionSeparator) {
-    // Special case "." and ".."
-    if (base == kCurrentDirectory || base == kParentDirectory) {
-      return FilePath();
-    }
-  }
-
   StringType ext = Extension();
   StringType ret = RemoveExtension().value();
   ret.append(suffix);
@@ -405,19 +406,26 @@ FilePath FilePath::InsertBeforeExtensionASCII(const base::StringPiece& suffix)
 #endif
 }
 
-FilePath FilePath::ReplaceExtension(const StringType& extension) const {
-  if (path_.empty())
+FilePath FilePath::AddExtension(const StringType& extension) const {
+  if (IsEmptyOrSpecialCase(BaseName().value()))
     return FilePath();
 
-  StringType base = BaseName().value();
-  if (base.empty())
-    return FilePath();
-  if (*(base.end() - 1) == kExtensionSeparator) {
-    // Special case "." and ".."
-    if (base == kCurrentDirectory || base == kParentDirectory) {
-      return FilePath();
-    }
+  // If the new extension is "" or ".", then just return the current FilePath.
+  if (extension.empty() || extension == StringType(1, kExtensionSeparator))
+    return *this;
+
+  StringType str = path_;
+  if (extension[0] != kExtensionSeparator &&
+      *(str.end() - 1) != kExtensionSeparator) {
+    str.append(1, kExtensionSeparator);
   }
+  str.append(extension);
+  return FilePath(str);
+}
+
+FilePath FilePath::ReplaceExtension(const StringType& extension) const {
+  if (IsEmptyOrSpecialCase(BaseName().value()))
+    return FilePath();
 
   FilePath no_ext = RemoveExtension();
   // If the new extension is "" or ".", then just remove the current extension.
diff --git a/base/file_path.h b/base/file_path.h
index 4d763eb..b90056b 100644
--- a/base/file_path.h
+++ b/base/file_path.h
@@ -242,8 +242,13 @@ class BASE_EXPORT FilePath {
   FilePath InsertBeforeExtensionASCII(
       const base::StringPiece& suffix) const WARN_UNUSED_RESULT;
 
+  // Adds |extension| to |file_name|. Returns the current FilePath if
+  // |extension| is empty. Returns "" if BaseName() == "." or "..".
+  FilePath AddExtension(
+      const StringType& extension) const WARN_UNUSED_RESULT;
+
   // Replaces the extension of |file_name| with |extension|.  If |file_name|
-  // does not have an extension, them |extension| is added.  If |extension| is
+  // does not have an extension, then |extension| is added.  If |extension| is
   // empty, then the extension is removed from |file_name|.
   // Returns "" if BaseName() == "." or "..".
   FilePath ReplaceExtension(
diff --git a/base/file_path_unittest.cc b/base/file_path_unittest.cc
index 71bff29..194ee49 100644
--- a/base/file_path_unittest.cc
+++ b/base/file_path_unittest.cc
@@ -896,6 +896,44 @@ TEST_F(FilePathTest, ReplaceExtension) {
   }
 }
 
+TEST_F(FilePathTest, AddExtension) {
+  const struct BinaryTestData cases[] = {
+    { { FPL(""),              FPL("") },      FPL("") },
+    { { FPL(""),              FPL("txt") },   FPL("") },
+    { { FPL("."),             FPL("txt") },   FPL("") },
+    { { FPL(".."),            FPL("txt") },   FPL("") },
+    { { FPL("."),             FPL("") },      FPL("") },
+    { { FPL("foo.dll"),       FPL("txt") },   FPL("foo.dll.txt") },
+    { { FPL("./foo.dll"),     FPL("txt") },   FPL("./foo.dll.txt") },
+    { { FPL("foo..dll"),      FPL("txt") },   FPL("foo..dll.txt") },
+    { { FPL("foo.dll"),       FPL(".txt") },  FPL("foo.dll.txt") },
+    { { FPL("foo"),           FPL("txt") },   FPL("foo.txt") },
+    { { FPL("foo."),          FPL("txt") },   FPL("foo.txt") },
+    { { FPL("foo.."),         FPL("txt") },   FPL("foo..txt") },
+    { { FPL("foo"),           FPL(".txt") },  FPL("foo.txt") },
+    { { FPL("foo.baz.dll"),   FPL("txt") },   FPL("foo.baz.dll.txt") },
+    { { FPL("foo.baz.dll"),   FPL(".txt") },  FPL("foo.baz.dll.txt") },
+    { { FPL("foo.dll"),       FPL("") },      FPL("foo.dll") },
+    { { FPL("foo.dll"),       FPL(".") },     FPL("foo.dll") },
+    { { FPL("foo"),           FPL("") },      FPL("foo") },
+    { { FPL("foo"),           FPL(".") },     FPL("foo") },
+    { { FPL("foo.baz.dll"),   FPL("") },      FPL("foo.baz.dll") },
+    { { FPL("foo.baz.dll"),   FPL(".") },     FPL("foo.baz.dll") },
+#if defined(FILE_PATH_USES_WIN_SEPARATORS)
+    { { FPL("C:\\foo.bar\\foo"),    FPL("baz") }, FPL("C:\\foo.bar\\foo.baz") },
+    { { FPL("C:\\foo.bar\\..\\\\"), FPL("baz") }, FPL("") },
+#endif
+    { { FPL("/foo.bar/foo"),        FPL("baz") }, FPL("/foo.bar/foo.baz") },
+    { { FPL("/foo.bar/..////"),     FPL("baz") }, FPL("") },
+  };
+  for (unsigned int i = 0; i < arraysize(cases); ++i) {
+    FilePath path(cases[i].inputs[0]);
+    FilePath added = path.AddExtension(cases[i].inputs[1]);
+    EXPECT_EQ(cases[i].expected, added.value()) << "i: " << i <<
+        ", path: " << path.value() << ", add: " << cases[i].inputs[1];
+  }
+}
+
 TEST_F(FilePathTest, MatchesExtension) {
   const struct BinaryBooleanTestData cases[] = {
     { { FPL("foo"),                     FPL("") },                    true},