Respond to PR feedback

dotnet · Nov 7, 2017 · 6c1e33a · 6c1e33a
1 parent 334630c
commit 6c1e33a
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 21 deletions.
diff --git a/src/Compilers/CSharp/Portable/CommandLine/CSharpCommandLineParser.cs b/src/Compilers/CSharp/Portable/CommandLine/CSharpCommandLineParser.cs
@@ -1037,10 +1037,13 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar
 
                         case "pathmap":
                             // "/pathmap:K1=V1,K2=V2..."
-                            unquoted = RemoveQuotesAndSlashes(value);
                             {
+                                unquoted = RemoveQuotesAndSlashes(value);
+
                                 if (unquoted == null)
+                                {
                                     break;
+                                }
 
                                 pathMap = pathMap.Concat(ParsePathMap(unquoted, diagnostics));
                             }

diff --git a/src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs b/src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs
@@ -9236,6 +9236,13 @@ void AssertPdbEmit(TempDirectory dir, string pdbPath, string pePdbPath, params s
                 var pdbPath = Path.Combine(dir.Path, "a.pdb");
                 AssertPdbEmit(dir, pdbPath, @"/a.pdb", $@"/pathmap:{dir.Path}=/");
             }
+
+            // Multi-specified path map with mixed slashes
+            using (var dir = new DisposableDirectory(Temp))
+            {
+                var pdbPath = Path.Combine(dir.Path, "a.pdb");
+                AssertPdbEmit(dir, pdbPath, "/goo/a.pdb", $"/pathmap:{dir.Path}=/goo,{dir.Path}{PathUtilities.DirectorySeparatorChar}=/bar");
+            }
         }
 
         [CompilerTrait(CompilerFeature.Determinism)]

diff --git a/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs b/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs
@@ -2527,25 +2527,44 @@ public void Bug9288_keycontainer()
         [Fact]
         public void PathMapKeepsCrossPlatformRoot()
         {
-            CSharpCommandLineArguments Parse(params string[] args)
+            CSharpCommandLineArguments parse(params string[] args)
             {
                 var parsedArgs = CSharpCommandLineParser.Default.Parse(args, TempRoot.Root, RuntimeEnvironment.GetRuntimeDirectory(), null);
                 parsedArgs.Errors.Verify();
                 return parsedArgs;
             }
-            Assert.Equal(new KeyValuePair<string, string>("C:\\", "/"), Parse("/pathmap:C:\\=/", "a.cs").PathMap[0]);
-            Assert.Equal(new KeyValuePair<string, string>("C:\\", "/temp/"), Parse("/pathmap:C:\\=/temp", "a.cs").PathMap[0]);
-            Assert.Equal(new KeyValuePair<string, string>("C:\\", "/temp/"), Parse("/pathmap:C:\\=/temp/", "a.cs").PathMap[0]);
-            Assert.Equal(new KeyValuePair<string, string>("C:\\temp\\", "/"), Parse("/pathmap:C:\\temp\\=/", "a.cs").PathMap[0]);
-            Assert.Equal(new KeyValuePair<string, string>("C:\\temp\\", "/temp/"), Parse("/pathmap:C:\\temp=/temp", "a.cs").PathMap[0]);
-            Assert.Equal(new KeyValuePair<string, string>("C:\\temp\\", "/temp/"), Parse("/pathmap:C:\\temp\\=/temp/", "a.cs").PathMap[0]);
-
-            Assert.Equal(new KeyValuePair<string, string>("/", "C:\\"), Parse("/pathmap:/=C:\\", "a.cs").PathMap[0]);
-            Assert.Equal(new KeyValuePair<string, string>("/", "C:\\temp\\"), Parse("/pathmap:/=C:\\temp", "a.cs").PathMap[0]);
-            Assert.Equal(new KeyValuePair<string, string>("/", "C:\\temp\\"), Parse("/pathmap:/=C:\\temp\\", "a.cs").PathMap[0]);
-            Assert.Equal(new KeyValuePair<string, string>("/temp/", "C:\\"), Parse("/pathmap:/temp/=C:\\", "a.cs").PathMap[0]);
-            Assert.Equal(new KeyValuePair<string, string>("/temp/", "C:\\temp\\"), Parse("/pathmap:/temp=C:\\temp", "a.cs").PathMap[0]);
-            Assert.Equal(new KeyValuePair<string, string>("/temp/", "C:\\temp\\"), Parse("/pathmap:/temp/=C:\\temp\\", "a.cs").PathMap[0]);
+
+            Assert.Equal(new KeyValuePair<string, string>("C:\\", "/"), parse("/pathmap:C:\\=/", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("C:\\", "/temp/"), parse("/pathmap:C:\\=/temp", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("C:\\", "/temp/"), parse("/pathmap:C:\\=/temp/", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("C:\\temp\\", "/"), parse("/pathmap:C:\\temp\\=/", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("C:\\temp\\", "/temp/"), parse("/pathmap:C:\\temp=/temp", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("C:\\temp\\", "/temp/"), parse("/pathmap:C:\\temp\\=/temp/", "a.cs").PathMap[0]);
+
+            Assert.Equal(new KeyValuePair<string, string>("/", "C:\\"), parse("/pathmap:/=C:\\", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("/", "C:\\temp\\"), parse("/pathmap:/=C:\\temp", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("/", "C:\\temp\\"), parse("/pathmap:/=C:\\temp\\", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("/temp/", "C:\\"), parse("/pathmap:/temp/=C:\\", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("/temp/", "C:\\temp\\"), parse("/pathmap:/temp=C:\\temp", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("/temp/", "C:\\temp\\"), parse("/pathmap:/temp/=C:\\temp\\", "a.cs").PathMap[0]);
+        }
+
+        [Fact]
+        public void PathMapInconsistentSlashes()
+        {
+            CSharpCommandLineArguments parse(params string[] args)
+            {
+                var parsedArgs = CSharpCommandLineParser.Default.Parse(args, TempRoot.Root, RuntimeEnvironment.GetRuntimeDirectory(), null);
+                parsedArgs.Errors.Verify();
+                return parsedArgs;
+            }
+
+            var sep = PathUtilities.DirectorySeparatorChar;
+            Assert.Equal(new KeyValuePair<string, string>("C:\\temp/goo" + sep, "/temp\\goo" + sep), parse("/pathmap:C:\\temp/goo=/temp\\goo", "a.cs").PathMap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("noslash" + sep, "withoutslash" + sep), parse("/pathmap:noslash=withoutslash", "a.cs").PathMap[0]);
+            var doublemap = parse("/pathmap:/temp=/goo,/temp/=/bar", "a.cs").PathMap;
+            Assert.Equal(new KeyValuePair<string, string>("/temp/", "/goo/"), doublemap[0]);
+            Assert.Equal(new KeyValuePair<string, string>("/temp/", "/bar/"), doublemap[1]);
         }
         #endregion
     }

diff --git a/src/Compilers/Core/Portable/CommandLine/CommonCommandLineParser.cs b/src/Compilers/Core/Portable/CommandLine/CommonCommandLineParser.cs
@@ -214,8 +214,8 @@ protected ImmutableArray<KeyValuePair<string, string>> ParsePathMap(string pathM
                 }
                 else
                 {
-                    from = PathUtilities.AddTrailingSeparator(from);
-                    to = PathUtilities.AddTrailingSeparator(to);
+                    from = PathUtilities.EnsureTrailingSeparator(from);
+                    to = PathUtilities.EnsureTrailingSeparator(to);
                     pathMapBuilder.Add(new KeyValuePair<string, string>(from, to));
                 }
             }

diff --git a/src/Compilers/Core/Portable/FileSystem/PathUtilities.cs b/src/Compilers/Core/Portable/FileSystem/PathUtilities.cs
@@ -60,7 +60,7 @@ public static string TrimTrailingSeparators(string s)
         /// <summary>
         /// Ensures a trailing directory separator character
         /// </summary>
-        public static string AddTrailingSeparator(string s)
+        public static string EnsureTrailingSeparator(string s)
         {
             if (s.Length == 0 || IsAnyDirectorySeparator(s[s.Length - 1]))
             {
@@ -80,7 +80,7 @@ public static string AddTrailingSeparator(string s)
             }
             else
             {
-                // If the slashes are not consistent, use the current platform's slash.
+                // If there are no slashes or they are inconsistent, use the current platform's slash.
                 return s + DirectorySeparatorChar;
             }
         }

diff --git a/src/Compilers/Core/Portable/SourceFileResolver.cs b/src/Compilers/Core/Portable/SourceFileResolver.cs
@@ -72,8 +72,8 @@ public SourceFileResolver(
                         throw new ArgumentException(CodeAnalysisResources.NullValueInPathMap, nameof(pathMap));
                     }
 
-                    var normalizedKey = PathUtilities.AddTrailingSeparator(key);
-                    var normalizedValue = PathUtilities.AddTrailingSeparator(value);
+                    var normalizedKey = PathUtilities.EnsureTrailingSeparator(key);
+                    var normalizedValue = PathUtilities.EnsureTrailingSeparator(value);
 
                     pathMapBuilder.Add(new KeyValuePair<string, string>(normalizedKey, normalizedValue));
                 }

diff --git a/src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb b/src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb
@@ -3043,6 +3043,18 @@ End Module
                 Dim pePdbPath = Path.Combine(dir.Path, "a.pdb")
                 assertPdbEmit(dir, "a.pdb", {"/features:pdb-path-determinism"})
             End Using
+
+            ' Unix path map
+            Using dir As New DisposableDirectory(Temp)
+                Dim pdbPath = Path.Combine(dir.Path, "a.pdb")
+                assertPdbEmit(dir, "/a.pdb", {$"/pathmap:{dir.Path}=/"})
+            End Using
+
+            ' Multi-specified path map with mixed slashes
+            Using dir As New DisposableDirectory(Temp)
+                Dim pdbPath = Path.Combine(dir.Path, "a.pdb")
+                assertPdbEmit(dir, "/goo/a.pdb", {$"/pathmap:{dir.Path}=/goo,{dir.Path}{PathUtilities.DirectorySeparatorChar}=/bar"})
+            End Using
         End Sub