diff --git a/src/Microsoft.DotNet.Cli.Sln.Internal/LocalizableStrings.cs b/src/Microsoft.DotNet.Cli.Sln.Internal/LocalizableStrings.cs index 24e940b70..3e696ecde 100644 --- a/src/Microsoft.DotNet.Cli.Sln.Internal/LocalizableStrings.cs +++ b/src/Microsoft.DotNet.Cli.Sln.Internal/LocalizableStrings.cs @@ -2,6 +2,14 @@ { internal class LocalizableStrings { + // {0} is the line number + // {1} is the error message details + public const string ErrorMessageFormatString = "Invalid format in line {0}: {1}"; + + public const string ProjectParsingErrorFormatString = "Project section is missing '{0}' when parsing the line starting at position {1}"; + + public const string InvalidPropertySetFormatString = "Property set is missing '{0}'"; + public const string GlobalSectionMoreThanOnceError = "Global section specified more than once"; public const string GlobalSectionNotClosedError = "Global section not closed"; diff --git a/src/Microsoft.DotNet.Cli.Sln.Internal/SlnFile.cs b/src/Microsoft.DotNet.Cli.Sln.Internal/SlnFile.cs index 51034eabb..7eda0d91e 100644 --- a/src/Microsoft.DotNet.Cli.Sln.Internal/SlnFile.cs +++ b/src/Microsoft.DotNet.Cli.Sln.Internal/SlnFile.cs @@ -120,6 +120,8 @@ namespace Microsoft.DotNet.Cli.Sln.Internal private void Read(TextReader reader) { + const string HeaderPrefix = "Microsoft Visual Studio Solution File, Format Version "; + string line; int curLineNum = 0; bool globalFound = false; @@ -129,14 +131,16 @@ namespace Microsoft.DotNet.Cli.Sln.Internal { curLineNum++; line = line.Trim(); - if (line.StartsWith("Microsoft Visual Studio Solution File", StringComparison.Ordinal)) + if (line.StartsWith(HeaderPrefix, StringComparison.Ordinal)) { - int i = line.LastIndexOf(' '); - if (i == -1) + if (line.Length <= HeaderPrefix.Length) { - throw new InvalidSolutionFormatException(curLineNum); + throw new InvalidSolutionFormatException( + curLineNum, + LocalizableStrings.FileHeaderMissingError); } - FormatVersion = line.Substring(i + 1); + + FormatVersion = line.Substring(HeaderPrefix.Length); _prefixBlankLines = curLineNum - 1; } if (line.StartsWith("# ", StringComparison.Ordinal)) @@ -157,7 +161,9 @@ namespace Microsoft.DotNet.Cli.Sln.Internal { if (globalFound) { - throw new InvalidSolutionFormatException(curLineNum, LocalizableStrings.GlobalSectionMoreThanOnceError); + throw new InvalidSolutionFormatException( + curLineNum, + LocalizableStrings.GlobalSectionMoreThanOnceError); } globalFound = true; while ((line = reader.ReadLine()) != null) @@ -181,7 +187,9 @@ namespace Microsoft.DotNet.Cli.Sln.Internal } if (line == null) { - throw new InvalidSolutionFormatException(curLineNum, LocalizableStrings.GlobalSectionNotClosedError); + throw new InvalidSolutionFormatException( + curLineNum, + LocalizableStrings.GlobalSectionNotClosedError); } } else if (line.IndexOf('=') != -1) @@ -191,7 +199,9 @@ namespace Microsoft.DotNet.Cli.Sln.Internal } if (FormatVersion == null) { - throw new InvalidSolutionFormatException(curLineNum, LocalizableStrings.FileHeaderMissingError); + throw new InvalidSolutionFormatException( + curLineNum, + LocalizableStrings.FileHeaderMissingError); } } @@ -331,15 +341,20 @@ namespace Microsoft.DotNet.Cli.Sln.Internal } } - throw new InvalidSolutionFormatException(curLineNum, LocalizableStrings.ProjectSectionNotClosedError); + throw new InvalidSolutionFormatException( + curLineNum, + LocalizableStrings.ProjectSectionNotClosedError); } private void FindNext(int ln, string line, ref int i, char c) { + var inputIndex = i; i = line.IndexOf(c, i); if (i == -1) { - throw new InvalidSolutionFormatException(ln); + throw new InvalidSolutionFormatException( + ln, + string.Format(LocalizableStrings.ProjectParsingErrorFormatString, c, inputIndex)); } } @@ -481,7 +496,9 @@ namespace Microsoft.DotNet.Cli.Sln.Internal { return SlnSectionType.PostProcess; } - throw new InvalidSolutionFormatException(curLineNum, String.Format(LocalizableStrings.InvalidSectionTypeError, s)); + throw new InvalidSolutionFormatException( + curLineNum, + String.Format(LocalizableStrings.InvalidSectionTypeError, s)); } private string FromSectionType(bool isProjectSection, SlnSectionType type) @@ -502,13 +519,17 @@ namespace Microsoft.DotNet.Cli.Sln.Internal int k = line.IndexOf('('); if (k == -1) { - throw new InvalidSolutionFormatException(curLineNum, LocalizableStrings.SectionIdMissingError); + throw new InvalidSolutionFormatException( + curLineNum, + LocalizableStrings.SectionIdMissingError); } var tag = line.Substring(0, k).Trim(); var k2 = line.IndexOf(')', k); if (k2 == -1) { - throw new InvalidSolutionFormatException(curLineNum); + throw new InvalidSolutionFormatException( + curLineNum, + LocalizableStrings.SectionIdMissingError); } Id = line.Substring(k + 1, k2 - k - 1); @@ -531,7 +552,9 @@ namespace Microsoft.DotNet.Cli.Sln.Internal } if (line == null) { - throw new InvalidSolutionFormatException(curLineNum, LocalizableStrings.ClosingSectionTagNotFoundError); + throw new InvalidSolutionFormatException( + curLineNum, + LocalizableStrings.ClosingSectionTagNotFoundError); } } @@ -550,7 +573,9 @@ namespace Microsoft.DotNet.Cli.Sln.Internal var i = line.IndexOf('.'); if (i == -1) { - throw new InvalidSolutionFormatException(_baseIndex + n); + throw new InvalidSolutionFormatException( + _baseIndex + n, + string.Format(LocalizableStrings.InvalidPropertySetFormatString, '.')); } var id = line.Substring(0, i); if (curSet == null || id != curSet.Id) @@ -1141,12 +1166,8 @@ namespace Microsoft.DotNet.Cli.Sln.Internal public class InvalidSolutionFormatException : Exception { - public InvalidSolutionFormatException(int line) : base("Invalid format in line " + line) - { - } - - public InvalidSolutionFormatException(int line, string msg) - : base("Invalid format in line " + line + ": " + msg) + public InvalidSolutionFormatException(int line, string details) + : base(string.Format(LocalizableStrings.ErrorMessageFormatString, line, details)) { } } diff --git a/src/dotnet/CommonLocalizableStrings.cs b/src/dotnet/CommonLocalizableStrings.cs index f64bf0fb6..135029829 100644 --- a/src/dotnet/CommonLocalizableStrings.cs +++ b/src/dotnet/CommonLocalizableStrings.cs @@ -105,7 +105,7 @@ namespace Microsoft.DotNet.Tools public const string CouldNotFindSolutionIn = "Specified solution file {0} does not exist, or there is no solution file in the directory."; public const string CouldNotFindSolutionOrDirectory = "Could not find solution or directory `{0}`."; public const string MoreThanOneSolutionInDirectory = "Found more than one solution file in {0}. Please specify which one to use."; - public const string InvalidSolution = "Invalid solution `{0}`."; + public const string InvalidSolutionFormatString = "Invalid solution `{0}`. {1}"; // {0} is the solution path, {1} is already localized details on the failure public const string SolutionDoesNotExist = "Specified solution file {0} does not exist, or there is no solution file in the directory."; /// add p2p diff --git a/src/dotnet/SlnFileFactory.cs b/src/dotnet/SlnFileFactory.cs index 0ae26f317..e824bfd67 100644 --- a/src/dotnet/SlnFileFactory.cs +++ b/src/dotnet/SlnFileFactory.cs @@ -30,9 +30,12 @@ namespace Microsoft.DotNet.Tools.Common { slnFile = SlnFile.Read(solutionPath); } - catch + catch (InvalidSolutionFormatException e) { - throw new GracefulException(CommonLocalizableStrings.InvalidSolution, solutionPath); + throw new GracefulException( + CommonLocalizableStrings.InvalidSolutionFormatString, + solutionPath, + e.Message); } return slnFile; } diff --git a/test/Microsoft.DotNet.Cli.Sln.Internal.Tests/Microsoft.DotNet.Cli.Sln.Internal.Tests.cs b/test/Microsoft.DotNet.Cli.Sln.Internal.Tests/Microsoft.DotNet.Cli.Sln.Internal.Tests.cs index b5f2c5a13..1fb96775e 100644 --- a/test/Microsoft.DotNet.Cli.Sln.Internal.Tests/Microsoft.DotNet.Cli.Sln.Internal.Tests.cs +++ b/test/Microsoft.DotNet.Cli.Sln.Internal.Tests/Microsoft.DotNet.Cli.Sln.Internal.Tests.cs @@ -276,11 +276,13 @@ EndGlobal .Should().Be(SolutionModified); } - [Fact] - public void WhenGivenAnSolutionWithMissingHeaderItThrows() + [Theory] + [InlineData("Invalid Solution")] + [InlineData("Microsoft Visual Studio Solution File, Format Version ")] + public void WhenGivenASolutionWithMissingHeaderItThrows(string fileContents) { var tmpFile = Temp.CreateFile(); - tmpFile.WriteAllText("Invalid Solution"); + tmpFile.WriteAllText(fileContents); Action action = () => { @@ -290,5 +292,180 @@ EndGlobal action.ShouldThrow() .WithMessage("Invalid format in line 1: File header is missing"); } + + [Fact] + public void WhenGivenASolutionWithMultipleGlobalSectionsItThrows() + { + const string SolutionFile = @" +Microsoft Visual Studio Solution File, Format Version 12.00 +Global +EndGlobal +Global +EndGlobal +"; + var tmpFile = Temp.CreateFile(); + tmpFile.WriteAllText(SolutionFile); + + Action action = () => + { + SlnFile.Read(tmpFile.Path); + }; + + action.ShouldThrow() + .WithMessage("Invalid format in line 5: Global section specified more than once"); + } + + [Fact] + public void WhenGivenASolutionWithGlobalSectionNotClosedItThrows() + { + const string SolutionFile = @" +Microsoft Visual Studio Solution File, Format Version 12.00 +Global +"; + var tmpFile = Temp.CreateFile(); + tmpFile.WriteAllText(SolutionFile); + + Action action = () => + { + SlnFile.Read(tmpFile.Path); + }; + + action.ShouldThrow() + .WithMessage("Invalid format in line 3: Global section not closed"); + } + + [Fact] + public void WhenGivenASolutionWithProjectSectionNotClosedItThrows() + { + const string SolutionFile = @" +Microsoft Visual Studio Solution File, Format Version 12.00 +Project(""{9A19103F-16F7-4668-BE54-9A1E7A4F7556}"") = ""App"", ""App\App.csproj"", ""{7072A694-548F-4CAE-A58F-12D257D5F486}"" +"; + var tmpFile = Temp.CreateFile(); + tmpFile.WriteAllText(SolutionFile); + + Action action = () => + { + SlnFile.Read(tmpFile.Path); + }; + + action.ShouldThrow() + .WithMessage("Invalid format in line 3: Project section not closed"); + } + + [Fact] + public void WhenGivenASolutionWithInvalidProjectSectionItThrows() + { + const string SolutionFile = @" +Microsoft Visual Studio Solution File, Format Version 12.00 +Project""{9A19103F-16F7-4668-BE54-9A1E7A4F7556}"") = ""App"", ""App\App.csproj"", ""{7072A694-548F-4CAE-A58F-12D257D5F486}"" +EndProject +"; + + var tmpFile = Temp.CreateFile(); + tmpFile.WriteAllText(SolutionFile); + + Action action = () => + { + SlnFile.Read(tmpFile.Path); + }; + + action.ShouldThrow() + .WithMessage("Invalid format in line 3: Project section is missing '(' when parsing the line starting at position 0"); + } + + [Fact] + public void WhenGivenASolutionWithInvalidSectionTypeItThrows() + { + const string SolutionFile = @" +Microsoft Visual Studio Solution File, Format Version 12.00 +Global + GlobalSection(SolutionConfigurationPlatforms) = thisIsUnknown + EndGlobalSection +EndGlobal +"; + var tmpFile = Temp.CreateFile(); + tmpFile.WriteAllText(SolutionFile); + + Action action = () => + { + SlnFile.Read(tmpFile.Path); + }; + + action.ShouldThrow() + .WithMessage("Invalid format in line 4: Invalid section type: thisIsUnknown"); + } + + [Fact] + public void WhenGivenASolutionWithMissingSectionIdTypeItThrows() + { + const string SolutionFile = @" +Microsoft Visual Studio Solution File, Format Version 12.00 +Global + GlobalSection = preSolution + EndGlobalSection +EndGlobal +"; + var tmpFile = Temp.CreateFile(); + tmpFile.WriteAllText(SolutionFile); + + Action action = () => + { + SlnFile.Read(tmpFile.Path); + }; + + action.ShouldThrow() + .WithMessage("Invalid format in line 4: Section id missing"); + } + + [Fact] + public void WhenGivenASolutionWithSectionNotClosedItThrows() + { + const string SolutionFile = @" +Microsoft Visual Studio Solution File, Format Version 12.00 +Global + GlobalSection(SolutionConfigurationPlatforms) = preSolution +EndGlobal +"; + var tmpFile = Temp.CreateFile(); + tmpFile.WriteAllText(SolutionFile); + + Action action = () => + { + SlnFile.Read(tmpFile.Path); + }; + + action.ShouldThrow() + .WithMessage("Invalid format in line 6: Closing section tag not found"); + } + + [Fact] + public void WhenGivenASolutionWithInvalidPropertySetItThrows() + { + const string SolutionFile = @" +Microsoft Visual Studio Solution File, Format Version 12.00 +Project(""{7072A694-548F-4CAE-A58F-12D257D5F486}"") = ""AppModified"", ""AppModified\AppModified.csproj"", ""{9A19103F-16F7-4668-BE54-9A1E7A4F7556}"" +EndProject +Global + GlobalSection(ProjectConfigurationPlatforms) = postSolution + {7072A694-548F-4CAE-A58F-12D257D5F486} Debug|Any CPU ActiveCfg = Debug|Any CPU + EndGlobalSection +EndGlobal +"; + var tmpFile = Temp.CreateFile(); + tmpFile.WriteAllText(SolutionFile); + + Action action = () => + { + var slnFile = SlnFile.Read(tmpFile.Path); + if (slnFile.ProjectConfigurationsSection.Count == 0) + { + // Need to force loading of nested property sets + } + }; + + action.ShouldThrow() + .WithMessage("Invalid format in line 7: Property set is missing '.'"); + } } } diff --git a/test/dotnet-add-proj.Tests/GivenDotnetAddProj.cs b/test/dotnet-add-proj.Tests/GivenDotnetAddProj.cs index 64f9a726c..7ea309909 100644 --- a/test/dotnet-add-proj.Tests/GivenDotnetAddProj.cs +++ b/test/dotnet-add-proj.Tests/GivenDotnetAddProj.cs @@ -90,7 +90,7 @@ Additional Arguments: .WithWorkingDirectory(projectDirectory) .ExecuteWithCapturedOutput($"add InvalidSolution.sln project {projectToAdd}"); cmd.Should().Fail(); - cmd.StdErr.Should().Be("Invalid solution `InvalidSolution.sln`."); + cmd.StdErr.Should().Be("Invalid solution `InvalidSolution.sln`. Invalid format in line 1: File header is missing"); cmd.StdOut.Should().BeVisuallyEquivalentTo(HelpText); } @@ -110,7 +110,7 @@ Additional Arguments: .WithWorkingDirectory(projectDirectory) .ExecuteWithCapturedOutput($"add project {projectToAdd}"); cmd.Should().Fail(); - cmd.StdErr.Should().Be($"Invalid solution `{solutionPath}`."); + cmd.StdErr.Should().Be($"Invalid solution `{solutionPath}`. Invalid format in line 1: File header is missing"); cmd.StdOut.Should().BeVisuallyEquivalentTo(HelpText); } diff --git a/test/dotnet-list-proj.Tests/GivenDotnetListProj.cs b/test/dotnet-list-proj.Tests/GivenDotnetListProj.cs index 5def520b6..877450467 100644 --- a/test/dotnet-list-proj.Tests/GivenDotnetListProj.cs +++ b/test/dotnet-list-proj.Tests/GivenDotnetListProj.cs @@ -83,7 +83,7 @@ Options: .WithWorkingDirectory(projectDirectory) .ExecuteWithCapturedOutput("list InvalidSolution.sln projects"); cmd.Should().Fail(); - cmd.StdErr.Should().Be("Invalid solution `InvalidSolution.sln`."); + cmd.StdErr.Should().Be("Invalid solution `InvalidSolution.sln`. Invalid format in line 1: File header is missing"); cmd.StdOut.Should().BeVisuallyEquivalentTo(HelpText); } @@ -102,7 +102,7 @@ Options: .WithWorkingDirectory(projectDirectory) .ExecuteWithCapturedOutput("list projects"); cmd.Should().Fail(); - cmd.StdErr.Should().Be($"Invalid solution `{solutionFullPath}`."); + cmd.StdErr.Should().Be($"Invalid solution `{solutionFullPath}`. Invalid format in line 1: File header is missing"); cmd.StdOut.Should().BeVisuallyEquivalentTo(HelpText); } diff --git a/test/dotnet-remove-proj.Tests/GivenDotnetRemoveProj.cs b/test/dotnet-remove-proj.Tests/GivenDotnetRemoveProj.cs index 5462eb6d1..c9da242d2 100644 --- a/test/dotnet-remove-proj.Tests/GivenDotnetRemoveProj.cs +++ b/test/dotnet-remove-proj.Tests/GivenDotnetRemoveProj.cs @@ -88,7 +88,7 @@ Additional Arguments: .WithWorkingDirectory(projectDirectory) .ExecuteWithCapturedOutput($"remove InvalidSolution.sln project {projectToRemove}"); cmd.Should().Fail(); - cmd.StdErr.Should().Be("Invalid solution `InvalidSolution.sln`."); + cmd.StdErr.Should().Be("Invalid solution `InvalidSolution.sln`. Invalid format in line 1: File header is missing"); cmd.StdOut.Should().BeVisuallyEquivalentTo(HelpText); } @@ -108,7 +108,7 @@ Additional Arguments: .WithWorkingDirectory(projectDirectory) .ExecuteWithCapturedOutput($"remove project {projectToRemove}"); cmd.Should().Fail(); - cmd.StdErr.Should().Be($"Invalid solution `{solutionPath}`."); + cmd.StdErr.Should().Be($"Invalid solution `{solutionPath}`. Invalid format in line 1: File header is missing"); cmd.StdOut.Should().BeVisuallyEquivalentTo(HelpText); }