From 01e53f4161996ce73408117d012f24c2c1737e58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Thu, 29 Aug 2024 17:52:55 +0200 Subject: [PATCH] Pass location information to EventArgs (#10545) --- src/Build/BuildCheck/API/BuildCheckResult.cs | 2 +- .../BuildCheck/BuildCheckEventArgs.cs | 13 +++-- src/Framework/BuildCheck/IBuildCheckResult.cs | 11 ++-- src/Framework/BuildErrorEventArgs.cs | 22 ++++++++ src/Framework/BuildWarningEventArgs.cs | 14 +++++ .../Microsoft.Build.Framework.csproj | 3 + src/Shared/IElementLocation.cs | 56 +------------------ src/Shared/IMSBuildElementLocation.cs | 56 +++++++++++++++++++ 8 files changed, 111 insertions(+), 66 deletions(-) create mode 100644 src/Shared/IMSBuildElementLocation.cs diff --git a/src/Build/BuildCheck/API/BuildCheckResult.cs b/src/Build/BuildCheck/API/BuildCheckResult.cs index fe8a234e12d..6471d717056 100644 --- a/src/Build/BuildCheck/API/BuildCheckResult.cs +++ b/src/Build/BuildCheck/API/BuildCheckResult.cs @@ -49,7 +49,7 @@ internal BuildEventArgs ToEventArgs(CheckResultSeverity severity) // Here we will provide different link for built-in rules and custom rules - once we have the base classes differentiated. public string FormatMessage() => - _message ??= $"{(Equals(Location ?? ElementLocation.EmptyLocation, ElementLocation.EmptyLocation) ? string.Empty : (Location!.LocationString + ": "))}https://aka.ms/buildcheck/codes#{CheckRule.Id} - {string.Format(CheckRule.MessageFormat, MessageArgs)}"; + _message ??= $"https://aka.ms/buildcheck/codes#{CheckRule.Id} - {string.Format(CheckRule.MessageFormat, MessageArgs)}"; private string? _message; } diff --git a/src/Framework/BuildCheck/BuildCheckEventArgs.cs b/src/Framework/BuildCheck/BuildCheckEventArgs.cs index 5a2d1ad5043..b490195a034 100644 --- a/src/Framework/BuildCheck/BuildCheckEventArgs.cs +++ b/src/Framework/BuildCheck/BuildCheckEventArgs.cs @@ -107,11 +107,11 @@ internal override void CreateFromStream(BinaryReader reader, int version) internal sealed class BuildCheckResultWarning : BuildWarningEventArgs { public BuildCheckResultWarning(IBuildCheckResult result, string code) - : base(subcategory: null, code: code, file: null, lineNumber: 0, columnNumber: 0, endLineNumber: 0, endColumnNumber: 0, message: result.FormatMessage(), helpKeyword: null, senderName: null) => + : base(code: code, file: result.Location.File, lineNumber: result.Location.Line, columnNumber: result.Location.Column, message: result.FormatMessage()) => RawMessage = result.FormatMessage(); internal BuildCheckResultWarning(string formattedMessage, string code) - : base(subcategory: null, code: code, file: null, lineNumber: 0, columnNumber: 0, endLineNumber: 0, endColumnNumber: 0, message: formattedMessage, helpKeyword: null, senderName: null) => + : base(code: code, file: null, lineNumber: 0, columnNumber: 0, message: formattedMessage) => RawMessage = formattedMessage; internal BuildCheckResultWarning() { } @@ -134,11 +134,11 @@ internal override void CreateFromStream(BinaryReader reader, int version) internal sealed class BuildCheckResultError : BuildErrorEventArgs { public BuildCheckResultError(IBuildCheckResult result, string code) - : base(subcategory: null, code: code, file: null, lineNumber: 0, columnNumber: 0, endLineNumber: 0, endColumnNumber: 0, message: result.FormatMessage(), helpKeyword: null, senderName: null) + : base(code: code, file: result.Location.File, lineNumber: result.Location.Line, columnNumber: result.Location.Column, message: result.FormatMessage()) => RawMessage = result.FormatMessage(); internal BuildCheckResultError(string formattedMessage, string code) - : base(subcategory: null, code: code, file: null, lineNumber: 0, columnNumber: 0, endLineNumber: 0, endColumnNumber: 0, message: formattedMessage, helpKeyword: null, senderName: null) + : base(code: code, file: null, lineNumber: 0, columnNumber: 0, message: formattedMessage) => RawMessage = formattedMessage; internal BuildCheckResultError() { } @@ -160,7 +160,10 @@ internal override void CreateFromStream(BinaryReader reader, int version) internal sealed class BuildCheckResultMessage : BuildMessageEventArgs { - public BuildCheckResultMessage(IBuildCheckResult result) => RawMessage = result.FormatMessage(); + public BuildCheckResultMessage(IBuildCheckResult result) + : base(message: result.FormatMessage(), file: result.Location.File, lineNumber: result.Location.Line, columnNumber: result.Location.Column, MessageImportance.High) + => RawMessage = result.FormatMessage(); + internal BuildCheckResultMessage(string formattedMessage) => RawMessage = formattedMessage; diff --git a/src/Framework/BuildCheck/IBuildCheckResult.cs b/src/Framework/BuildCheck/IBuildCheckResult.cs index 1d471e6c9bc..4f7043b771b 100644 --- a/src/Framework/BuildCheck/IBuildCheckResult.cs +++ b/src/Framework/BuildCheck/IBuildCheckResult.cs @@ -1,11 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; +using Microsoft.Build.Shared; namespace Microsoft.Build.Experimental.BuildCheck; @@ -18,6 +14,11 @@ internal interface IBuildCheckResult /// Optional location of the finding (in near future we might need to support multiple locations). /// string LocationString { get; } + + /// + /// Location of the finding. + /// + IMSBuildElementLocation Location { get; } string[] MessageArgs { get; } string MessageFormat { get; } diff --git a/src/Framework/BuildErrorEventArgs.cs b/src/Framework/BuildErrorEventArgs.cs index 543f71e1ec3..1b793121678 100644 --- a/src/Framework/BuildErrorEventArgs.cs +++ b/src/Framework/BuildErrorEventArgs.cs @@ -201,6 +201,28 @@ public BuildErrorEventArgs( this.helpLink = helpLink; } + /// + /// This constructor allows event data without ends to be initialized. + /// + /// event code + /// file associated with the event + /// line number (0 if not applicable) + /// column number (0 if not applicable) + /// text message + protected BuildErrorEventArgs( + string code, + string message, + string file, + int lineNumber, + int columnNumber) + : base(message, helpKeyword: null, senderName: null) + { + this.code = code; + this.file = file; + this.lineNumber = lineNumber; + this.columnNumber = columnNumber; + } + /// /// Default constructor /// diff --git a/src/Framework/BuildWarningEventArgs.cs b/src/Framework/BuildWarningEventArgs.cs index 8e2e522147b..7cd5ec1f09d 100644 --- a/src/Framework/BuildWarningEventArgs.cs +++ b/src/Framework/BuildWarningEventArgs.cs @@ -163,6 +163,20 @@ public BuildWarningEventArgs( this.helpLink = helpLink; } + /// + /// This constructor allows event data without ends to be initialized. + /// + /// event code + /// file associated with the event + /// line number (0 if not applicable) + /// column number (0 if not applicable) + /// text message + public BuildWarningEventArgs(string code, string file, int lineNumber, int columnNumber, string message) + : this(subcategory: null, code: code, file: file, lineNumber: lineNumber, columnNumber: columnNumber, endLineNumber: 0, endColumnNumber: 0, message: message, helpKeyword: null, senderName: null) + { + // do nothing + } + private string subcategory; private string code; private string file; diff --git a/src/Framework/Microsoft.Build.Framework.csproj b/src/Framework/Microsoft.Build.Framework.csproj index 787ba544b89..736cccac2f1 100644 --- a/src/Framework/Microsoft.Build.Framework.csproj +++ b/src/Framework/Microsoft.Build.Framework.csproj @@ -39,6 +39,9 @@ Shared\BinaryWriterExtensions.cs + + Shared\IMSBuildElementLocation.cs + diff --git a/src/Shared/IElementLocation.cs b/src/Shared/IElementLocation.cs index 8ea57b003f3..4824e758d86 100644 --- a/src/Shared/IElementLocation.cs +++ b/src/Shared/IElementLocation.cs @@ -2,65 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.Build.BackEnd; +using Microsoft.Build.Framework; #nullable disable namespace Microsoft.Build.Shared { internal interface IElementLocation : IMSBuildElementLocation, ITranslatable { } - - /// - /// Represents the location information for error reporting purposes. This is normally used to - /// associate a run-time error with the original XML. - /// This is not used for arbitrary errors from tasks, which store location in a BuildXXXXEventArgs. - /// All implementations should be IMMUTABLE. - /// Any editing of the project XML through the MSBuild API's will invalidate locations in that XML until the XML is reloaded. - /// - /// - /// This is currently internal - but it is prepared to be made public once it will be needed by other public BuildCheck OM - /// (e.g. by property read/write OM). - /// - public interface IMSBuildElementLocation - { - /// - /// The file from which this particular element originated. It may - /// differ from the ProjectFile if, for instance, it was part of - /// an import or originated in a targets file. - /// Should always have a value. - /// If not known, returns empty string. - /// - string File - { - get; - } - - /// - /// The line number where this element exists in its file. - /// The first line is numbered 1. - /// Zero indicates "unknown location". - /// - int Line - { - get; - } - - /// - /// The column number where this element exists in its file. - /// The first column is numbered 1. - /// Zero indicates "unknown location". - /// - int Column - { - get; - } - - /// - /// The location in a form suitable for replacement - /// into a message. - /// - string LocationString - { - get; - } - } } diff --git a/src/Shared/IMSBuildElementLocation.cs b/src/Shared/IMSBuildElementLocation.cs new file mode 100644 index 00000000000..bd329f0580b --- /dev/null +++ b/src/Shared/IMSBuildElementLocation.cs @@ -0,0 +1,56 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Build.Shared +{ + /// + /// Represents the location information for error reporting purposes. This is normally used to + /// associate a run-time error with the original XML. + /// This is not used for arbitrary errors from tasks, which store location in a BuildXXXXEventArgs. + /// All implementations should be IMMUTABLE. + /// Any editing of the project XML through the MSBuild API's will invalidate locations in that XML until the XML is reloaded. + /// + public interface IMSBuildElementLocation + { + /// + /// The file from which this particular element originated. It may + /// differ from the ProjectFile if, for instance, it was part of + /// an import or originated in a targets file. + /// Should always have a value. + /// If not known, returns empty string. + /// + string File + { + get; + } + + /// + /// The line number where this element exists in its file. + /// The first line is numbered 1. + /// Zero indicates "unknown location". + /// + int Line + { + get; + } + + /// + /// The column number where this element exists in its file. + /// The first column is numbered 1. + /// Zero indicates "unknown location". + /// + int Column + { + get; + } + + /// + /// The location in a form suitable for replacement + /// into a message. + /// + string LocationString + { + get; + } + } +}