diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 44482aa..8482f26 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,3 +1,3 @@ # These owners will be the default owners for everything in the repo and # will be requested for review when someone opens a pull request. -* @cake-contrib/team-bbt @x-jokay \ No newline at end of file +* @cake-contrib/team-cake-issues \ No newline at end of file diff --git a/docs/features.md b/docs/features.md index c6cff68..702e4ce 100644 --- a/docs/features.md +++ b/docs/features.md @@ -7,7 +7,7 @@ The [Cake.Issues.MsBuild addin] provides the following features. # Basic features -* Reads warnings from MSBuild log files. +* Reads errors and warnings from MSBuild log files. * Provides URLs for all code analysis (`CA*`) and StyleCop (`SA*`) warnings. * Support for custom URL resolving using the [MsBuildAddRuleUrlResolver] alias. @@ -35,8 +35,8 @@ The [Cake.Issues.MsBuild addin] provides the following features. | | `IIssue.MessageText` | | | | `IIssue.MessageHtml` | | | | `IIssue.MessageMarkdown` | | -| | `IIssue.Priority` | Always [IssuePriority.Warning] | -| | `IIssue.PriorityName` | Always `Warning` | +| | `IIssue.Priority` | | +| | `IIssue.PriorityName` | | | | `IIssue.Rule` | | | | `IIssue.RuleUrl` | For code analysis (`CA*`) and StyleCop (`SA*`) warnings. Support for additional rules can be added through a custom [MsBuildAddRuleUrlResolver] | diff --git a/nuspec/nuget/Cake.Issues.MsBuild.nuspec b/nuspec/nuget/Cake.Issues.MsBuild.nuspec index 24f8dbc..25940d5 100644 --- a/nuspec/nuget/Cake.Issues.MsBuild.nuspec +++ b/nuspec/nuget/Cake.Issues.MsBuild.nuspec @@ -24,7 +24,7 @@ See the Project Site for an overview of the whole ecosystem of addins for workin Copyright © BBT Software AG and contributors Cake Script Cake-Issues Cake-IssueProvider CodeAnalysis Linting MsBuild - https://github.com/cake-contrib/Cake.Issues.MsBuild/releases/tag/0.9.0 + https://github.com/cake-contrib/Cake.Issues.MsBuild/releases/tag/0.9.1 diff --git a/recipe.cake b/recipe.cake index 8de517f..2bac329 100644 --- a/recipe.cake +++ b/recipe.cake @@ -11,6 +11,7 @@ BuildParameters.SetParameters( repositoryName: "Cake.Issues.MsBuild", appVeyorAccountName: "cakecontrib", shouldGenerateDocumentation: false, + shouldPublishMyGet: false, shouldRunCodecov: false, shouldRunGitVersion: true); diff --git a/src/Cake.Issues.MsBuild.Tests/Cake.Issues.MsBuild.Tests.csproj b/src/Cake.Issues.MsBuild.Tests/Cake.Issues.MsBuild.Tests.csproj index 118a089..4b7f17e 100644 --- a/src/Cake.Issues.MsBuild.Tests/Cake.Issues.MsBuild.Tests.csproj +++ b/src/Cake.Issues.MsBuild.Tests/Cake.Issues.MsBuild.Tests.csproj @@ -15,32 +15,16 @@ - - - - - - - - - - + - - - - - - - - - + + - + @@ -48,15 +32,11 @@ - + - - - - diff --git a/src/Cake.Issues.MsBuild.Tests/LogFileFormat/XmlFileLoggerLogFileFormatTests.cs b/src/Cake.Issues.MsBuild.Tests/LogFileFormat/XmlFileLoggerLogFileFormatTests.cs index e5892b8..92577a4 100644 --- a/src/Cake.Issues.MsBuild.Tests/LogFileFormat/XmlFileLoggerLogFileFormatTests.cs +++ b/src/Cake.Issues.MsBuild.Tests/LogFileFormat/XmlFileLoggerLogFileFormatTests.cs @@ -402,6 +402,62 @@ public void Should_Read_Issue_With_Absolute_FileName_And_Without_Task() } } } + + [Fact] + public void Should_Read_Errors() + { + // Given + var fixture = new MsBuildIssuesProviderFixture("IssueWithError.xml"); + + // When + var issues = fixture.ReadIssues().ToList(); + + // Then + issues.Count.ShouldBe(1); + IssueChecker.Check( + issues[0], + IssueBuilder.NewIssue( + @"'ConfigurationManager.GetSortedConfigFiles(String)': not all code paths return a value", + "Cake.Issues.MsBuild.MsBuildIssuesProvider", + "MSBuild") + .InProjectOfName(string.Empty) + .InFile(@"src\Cake.Issues.MsBuild.Tests\MsBuildIssuesProviderTests.cs", 1311) + .OfRule("CS0161", null) + .WithPriority(IssuePriority.Error)); + } + + [Fact] + public void Should_Read_Both_Warnings_And_Errors() + { + // Given + var fixture = new MsBuildIssuesProviderFixture("IssueWithBothWarningAndErrors.xml"); + + // When + var issues = fixture.ReadIssues().ToList(); + + // Then + issues.Count.ShouldBe(2); + IssueChecker.Check( + issues[0], + IssueBuilder.NewIssue( + @"Microsoft.Usage : 'ConfigurationManager.GetSortedConfigFiles(String)' creates an exception of type 'ApplicationException', an exception type that is not sufficiently specific and should never be raised by user code. If this exception instance might be thrown, use a different exception type.", + "Cake.Issues.MsBuild.MsBuildIssuesProvider", + "MSBuild") + .InProjectOfName(string.Empty) + .InFile(@"src\Cake.Issues.MsBuild.Tests\MsBuildIssuesProviderTests.cs", 1311) + .OfRule("CA2201", new Uri("https://www.google.com/search?q=\"CA2201:\"+site:docs.microsoft.com")) + .WithPriority(IssuePriority.Warning)); + IssueChecker.Check( + issues[1], + IssueBuilder.NewIssue( + @"'ConfigurationManager.GetSortedConfigFiles(String)': not all code paths return a value", + "Cake.Issues.MsBuild.MsBuildIssuesProvider", + "MSBuild") + .InProjectOfName(string.Empty) + .InFile(@"src\Cake.Issues.MsBuild.Tests\MsBuildIssuesProviderTests.cs", 1311) + .OfRule("CS0161", null) + .WithPriority(IssuePriority.Error)); + } } } } diff --git a/src/Cake.Issues.MsBuild.Tests/Testfiles/XmlFileLoggerLogFileFormat/IssueWithBothWarningAndErrors.xml b/src/Cake.Issues.MsBuild.Tests/Testfiles/XmlFileLoggerLogFileFormat/IssueWithBothWarningAndErrors.xml new file mode 100644 index 0000000..06ae933 --- /dev/null +++ b/src/Cake.Issues.MsBuild.Tests/Testfiles/XmlFileLoggerLogFileFormat/IssueWithBothWarningAndErrors.xml @@ -0,0 +1,5 @@ + + + + + \ No newline at end of file diff --git a/src/Cake.Issues.MsBuild.Tests/Testfiles/XmlFileLoggerLogFileFormat/IssueWithError.xml b/src/Cake.Issues.MsBuild.Tests/Testfiles/XmlFileLoggerLogFileFormat/IssueWithError.xml new file mode 100644 index 0000000..610acdc --- /dev/null +++ b/src/Cake.Issues.MsBuild.Tests/Testfiles/XmlFileLoggerLogFileFormat/IssueWithError.xml @@ -0,0 +1,4 @@ + + + + \ No newline at end of file diff --git a/src/Cake.Issues.MsBuild/Cake.Issues.MsBuild.csproj b/src/Cake.Issues.MsBuild/Cake.Issues.MsBuild.csproj index 8aa395a..1281692 100644 --- a/src/Cake.Issues.MsBuild/Cake.Issues.MsBuild.csproj +++ b/src/Cake.Issues.MsBuild/Cake.Issues.MsBuild.csproj @@ -27,7 +27,7 @@ - + diff --git a/src/Cake.Issues.MsBuild/LogFileFormat/BinaryLogFileFormat.cs b/src/Cake.Issues.MsBuild/LogFileFormat/BinaryLogFileFormat.cs index bad304e..72c323c 100644 --- a/src/Cake.Issues.MsBuild/LogFileFormat/BinaryLogFileFormat.cs +++ b/src/Cake.Issues.MsBuild/LogFileFormat/BinaryLogFileFormat.cs @@ -43,168 +43,220 @@ public override IEnumerable ReadIssues( { var buildEventArgs = record.Args; - if (buildEventArgs is BuildWarningEventArgs buildWarning) + IIssue issue = null; + if (buildEventArgs is BuildErrorEventArgs buildError) { - // Ignore warnings without a message. - if (string.IsNullOrWhiteSpace(buildWarning.Message)) - { - continue; - } - - var projectFileRelativePath = this.GetProject(buildWarning, repositorySettings); - - // Read affected file from the warning. - if (!this.TryGetFile(buildWarning, repositorySettings, out string fileName)) - { - continue; - } - - var line = GetLine(buildWarning); - var endLine = GetEndLine(buildWarning); - var column = GetColumn(buildWarning); - var endColumn = GetEndColumn(buildWarning); - var rule = buildWarning.Code; - - // Determine rule URL. - Uri ruleUrl = null; - if (!string.IsNullOrWhiteSpace(rule)) - { - ruleUrl = MsBuildRuleUrlResolver.Instance.ResolveRuleUrl(rule); - } - - // Build issue. - result.Add( - IssueBuilder - .NewIssue(buildWarning.Message, issueProvider) - .WithPriority(IssuePriority.Warning) - .InProject(projectFileRelativePath, System.IO.Path.GetFileNameWithoutExtension(projectFileRelativePath)) - .InFile(fileName, line, endLine, column, endColumn) - .OfRule(rule, ruleUrl) - .Create()); + issue = this.GetIssue(buildError, issueProvider, repositorySettings); } + else if (buildEventArgs is BuildWarningEventArgs buildWarning) + { + issue = this.GetIssue(buildWarning, issueProvider, repositorySettings); + } + + if (issue == null) + { + continue; + } + + result.Add(issue); } return result; } /// - /// Reads the affected line from a warning logged in a MsBuild log. + /// Returns the column based on the value from a MsBuild log. /// - /// Warning element from MsBuild log. - /// Line number or null if warning is not related to a file. - private static int? GetLine(BuildWarningEventArgs warning) + /// Raw value from MsBuild log. + /// Column number or null if warning or error is not related to a file. + private static int? GetColumn(int column) { - var line = warning.LineNumber; - - // Convert negative line numbers or line number 0 to null - if (line <= 0) + // Convert negative column numbers or column number 0 to null + if (column <= 0) { return null; } - return line; + return column; } /// - /// Reads the end of line range from a warning logged in a MsBuild log. + /// Returns the line based on the value from a MsBuild log. /// - /// Warning element from MsBuild log. - /// End of line range or null if warning is not related to a file. - private static int? GetEndLine(BuildWarningEventArgs warning) + /// Raw value from MsBuild log. + /// Line number or null if warning or error is not related to a file. + private static int? GetLine(int line) { - var endLine = warning.EndLineNumber; - // Convert negative line numbers or line number 0 to null - if (endLine <= 0) + if (line <= 0) { return null; } - return endLine; + return line; } /// - /// Reads the affected column from a warning logged in a MsBuild log. + /// Returns an issue for a build error. /// - /// Warning element from MsBuild log. - /// Column number or null if warning is not related to a file. - private static int? GetColumn(BuildWarningEventArgs warning) + /// Error for which the issue should be returned. + /// Issue provider instance. + /// Repository settings to use. + /// Issue instance or null, if the could not be parsed. + private IIssue GetIssue( + BuildErrorEventArgs buildError, + MsBuildIssuesProvider issueProvider, + IRepositorySettings repositorySettings) { - var column = warning.ColumnNumber; - - // Convert negative column numbers or column number 0 to null - if (column <= 0) - { - return null; - } + return + this.GetIssue( + IssuePriority.Error, + buildError.Message, + buildError.ProjectFile, + buildError.File, + buildError.LineNumber, + buildError.EndLineNumber, + buildError.ColumnNumber, + buildError.EndColumnNumber, + buildError.Code, + issueProvider, + repositorySettings); + } - return column; + /// + /// Returns an issue for a build warning. + /// + /// Warning for which the issue should be returned. + /// Issue provider instance. + /// Repository settings to use. + /// Issue instance or null, if the could not be parsed. + private IIssue GetIssue( + BuildWarningEventArgs buildWarning, + MsBuildIssuesProvider issueProvider, + IRepositorySettings repositorySettings) + { + return + this.GetIssue( + IssuePriority.Warning, + buildWarning.Message, + buildWarning.ProjectFile, + buildWarning.File, + buildWarning.LineNumber, + buildWarning.EndLineNumber, + buildWarning.ColumnNumber, + buildWarning.EndColumnNumber, + buildWarning.Code, + issueProvider, + repositorySettings); } /// - /// Reads the end of column range from a warning logged in a MsBuild log. + /// Returns an issue for values from an MsBuild log. /// - /// Warning element from MsBuild log. - /// End of column range or null if warning is not related to a file. - private static int? GetEndColumn(BuildWarningEventArgs warning) + /// Priority of the issue. + /// Raw value from the MsBuild log containing the message. + /// Raw value from the MsBuild log containing the project file. + /// Raw value from the MsBuild log containing the file. + /// Raw value from the MsBuild log containing the line number. + /// Raw value from the MsBuild log containing the end of the line range. + /// Raw value from the MsBuild log containing the column. + /// Raw value from the MsBuild log containing the end of the column range. + /// Raw value from the MsBuild log containing the rule. + /// Issue provider instance. + /// Repository settings to use. + /// Issue instance or null, if the values could not be parsed. + private IIssue GetIssue( + IssuePriority priority, + string message, + string projectFile, + string file, + int lineNumber, + int endLineNumber, + int columnNumber, + int endColumnNumber, + string code, + MsBuildIssuesProvider issueProvider, + IRepositorySettings repositorySettings) { - var endColumn = warning.EndColumnNumber; + // Ignore warnings or errors without a message. + if (string.IsNullOrWhiteSpace(message)) + { + return null; + } - // Convert negative column numbers or column number 0 to null - if (endColumn <= 0) + var projectFileRelativePath = this.GetProject(projectFile, repositorySettings); + + // Read affected file from the warning or error. + var (result, fileName) = this.TryGetFile(file, projectFile, repositorySettings); + if (!result) { return null; } - return endColumn; + var line = GetLine(lineNumber); + var endLine = GetLine(endLineNumber); + var column = GetColumn(columnNumber); + var endColumn = GetColumn(endColumnNumber); + var rule = code; + + // Determine rule URL. + Uri ruleUrl = null; + if (!string.IsNullOrWhiteSpace(rule)) + { + ruleUrl = MsBuildRuleUrlResolver.Instance.ResolveRuleUrl(rule); + } + + // Build issue. + return + IssueBuilder + .NewIssue(message, issueProvider) + .WithPriority(priority) + .InProject(projectFileRelativePath, System.IO.Path.GetFileNameWithoutExtension(projectFileRelativePath)) + .InFile(fileName, line, endLine, column, endColumn) + .OfRule(rule, ruleUrl) + .Create(); } /// - /// Determines the project for a warning logged in a MsBuild log. + /// Determines the project from a value in a MsBuild log. /// - /// Warning element from the MsBuild log. + /// Raw value from the MsBuild log. /// Repository settings to use. /// Relative path to the project. private string GetProject( - BuildWarningEventArgs warning, + string project, IRepositorySettings repositorySettings) { - var project = warning.ProjectFile; - // Validate project path and make relative to repository root. return this.ValidateFilePath(project, repositorySettings).FilePath; } /// - /// Reads the affected file path from a warning logged in a MsBuild log. + /// Reads the affected file path from a value in a MsBuild log. /// - /// Warning element from MsBuild log. + /// Raw value for file path from MsBuild log. + /// Raw value for project path from the MsBuild log. /// Repository settings to use. - /// Returns the full path to the affected file. - /// True if the file path could be parsed. - private bool TryGetFile( - BuildWarningEventArgs warning, - IRepositorySettings repositorySettings, - out string fileName) + /// True if the file path could be parsed and the full path to the affected file. + private (bool successful, string fileName) TryGetFile( + string fileName, + string project, + IRepositorySettings repositorySettings) { - fileName = warning.File; - if (string.IsNullOrWhiteSpace(fileName)) { - return true; + return (true, fileName); } // If not absolute path, combine with file path from project file. if (!fileName.IsFullPath()) { - var projectDirectory = System.IO.Path.GetDirectoryName(warning.ProjectFile); + var projectDirectory = System.IO.Path.GetDirectoryName(project); fileName = System.IO.Path.Combine(projectDirectory, fileName); } // Validate file path and make relative to repository root. - bool result; - (result, fileName) = this.ValidateFilePath(fileName, repositorySettings); - return result; + return this.ValidateFilePath(fileName, repositorySettings); } } } diff --git a/src/Cake.Issues.MsBuild/LogFileFormat/XmlFileLoggerLogFileFormat.cs b/src/Cake.Issues.MsBuild/LogFileFormat/XmlFileLoggerLogFileFormat.cs index 798103b..cd0a808 100644 --- a/src/Cake.Issues.MsBuild/LogFileFormat/XmlFileLoggerLogFileFormat.cs +++ b/src/Cake.Issues.MsBuild/LogFileFormat/XmlFileLoggerLogFileFormat.cs @@ -44,41 +44,44 @@ public override IEnumerable ReadIssues( var filtered = string.Concat(raw.Where(c => !char.IsControl(c))); var logDocument = XDocument.Parse(filtered); - // Loop through all warning tags. - foreach (var warning in logDocument.Descendants("warning")) + // Loop through all warning and error tags. + var elements = new List(logDocument.Descendants("warning")); + elements.AddRange(logDocument.Descendants("error")); + + foreach (var element in elements) { - // Ignore warnings without a message. - if (string.IsNullOrWhiteSpace(warning.Value)) + // Ignore warnings or errors without a message. + if (string.IsNullOrWhiteSpace(element.Value)) { continue; } - // Read affected project from the warning. - if (!this.TryGetProject(warning, repositorySettings, out string projectFileRelativePath)) + // Read affected project from the warning or error. + if (!this.TryGetProject(element, repositorySettings, out string projectFileRelativePath)) { continue; } - // Read affected file from the warning. - if (!this.TryGetFile(warning, repositorySettings, out string fileName)) + // Read affected file from the warning or error. + if (!this.TryGetFile(element, repositorySettings, out string fileName)) { continue; } - // Read affected line from the warning. - if (!TryGetLine(warning, out var line)) + // Read affected line from the warning or error. + if (!TryGetLine(element, out var line)) { continue; } - // Read affected column from the warning. - if (!TryGetColumn(warning, out var column)) + // Read affected column from the warning or error. + if (!TryGetColumn(element, out var column)) { continue; } - // Read rule code from the warning. - if (!TryGetRule(warning, out string rule)) + // Read rule code from the warning or error. + if (!TryGetRule(element, out string rule)) { continue; } @@ -90,11 +93,13 @@ public override IEnumerable ReadIssues( ruleUrl = MsBuildRuleUrlResolver.Instance.ResolveRuleUrl(rule); } + var priority = element.Name.LocalName == "error" ? IssuePriority.Error : IssuePriority.Warning; + // Build issue. result.Add( IssueBuilder - .NewIssue(warning.Value, issueProvider) - .WithPriority(IssuePriority.Warning) + .NewIssue(element.Value, issueProvider) + .WithPriority(priority) .InProject(projectFileRelativePath, System.IO.Path.GetFileNameWithoutExtension(projectFileRelativePath)) .InFile(fileName, line, column) .OfRule(rule, ruleUrl) @@ -105,16 +110,16 @@ public override IEnumerable ReadIssues( } /// - /// Reads the affected line from a warning logged in a MsBuild log. + /// Reads the affected line from a warning or error logged in a MsBuild log. /// - /// Warning element from MsBuild log. + /// Warning or error element from MsBuild log. /// Returns line. /// True if the line could be parsed. - private static bool TryGetLine(XElement warning, out int? line) + private static bool TryGetLine(XElement element, out int? line) { line = null; - var lineAttr = warning.Attribute("line"); + var lineAttr = element.Attribute("line"); var lineValue = lineAttr?.Value; if (string.IsNullOrWhiteSpace(lineValue)) @@ -134,16 +139,16 @@ private static bool TryGetLine(XElement warning, out int? line) } /// - /// Reads the affected column from a warning logged in a MsBuild log. + /// Reads the affected column from a warning or error logged in a MsBuild log. /// - /// Warning element from MsBuild log. + /// Warning or error element from MsBuild log. /// Returns column. /// True if the column could be parsed. - private static bool TryGetColumn(XElement warning, out int? column) + private static bool TryGetColumn(XElement element, out int? column) { column = null; - var columnAttr = warning.Attribute("column"); + var columnAttr = element.Attribute("column"); var columnValue = columnAttr?.Value; if (string.IsNullOrWhiteSpace(columnValue)) @@ -163,14 +168,14 @@ private static bool TryGetColumn(XElement warning, out int? column) } /// - /// Reads the rule code from a warning logged in a MsBuild log. + /// Reads the rule code from a warning or error logged in a MsBuild log. /// - /// Warning element from MsBuild log. + /// Warning or error element from MsBuild log. /// Returns the code of the rule. /// True if the rule code could be parsed. - private static bool TryGetRule(XElement warning, out string rule) + private static bool TryGetRule(XElement error, out string rule) { - var codeAttr = warning.Attribute("code"); + var codeAttr = error.Attribute("code"); if (codeAttr == null) { rule = null; @@ -182,20 +187,20 @@ private static bool TryGetRule(XElement warning, out string rule) } /// - /// Determines the project for a warning logged in a MsBuild log. + /// Determines the project for a warning or error logged in a MsBuild log. /// - /// Warning element from MsBuild log. + /// Warning or error element from MsBuild log. /// Repository settings to use. /// Returns project. /// True if the project could be parsed. private bool TryGetProject( - XElement warning, + XElement element, IRepositorySettings repositorySettings, out string project) { project = string.Empty; - var projectNode = warning.Ancestors("project").FirstOrDefault(); + var projectNode = element.Ancestors("project").FirstOrDefault(); if (projectNode == null) { return true; @@ -220,20 +225,20 @@ private bool TryGetProject( } /// - /// Reads the affected file path from a warning logged in a MsBuild log. + /// Reads the affected file path from a warning or error logged in a MsBuild log. /// - /// Warning element from MsBuild log. + /// Warning or error element from MsBuild log. /// Repository settings to use. /// Returns the full path to the affected file. /// True if the file path could be parsed. private bool TryGetFile( - XElement warning, + XElement element, IRepositorySettings repositorySettings, out string fileName) { fileName = string.Empty; - var fileAttr = warning.Attribute("file"); + var fileAttr = element.Attribute("file"); if (fileAttr == null) { return true; @@ -248,7 +253,7 @@ private bool TryGetFile( // If not absolute path, combine with file path from compile task. if (!fileName.IsFullPath()) { - var parentFileAttr = warning.Parent?.Attribute("file"); + var parentFileAttr = element.Parent?.Attribute("file"); if (parentFileAttr != null) { var compileTaskDirectory = System.IO.Path.GetDirectoryName(parentFileAttr.Value);