Skip to content

Commit

Permalink
Pass location information to EventArgs (#10545)
Browse files Browse the repository at this point in the history
  • Loading branch information
JanProvaznik authored Aug 29, 2024
1 parent b82694a commit 01e53f4
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 66 deletions.
2 changes: 1 addition & 1 deletion src/Build/BuildCheck/API/BuildCheckResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
13 changes: 8 additions & 5 deletions src/Framework/BuildCheck/BuildCheckEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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() { }
Expand All @@ -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() { }
Expand All @@ -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;

Expand Down
11 changes: 6 additions & 5 deletions src/Framework/BuildCheck/IBuildCheckResult.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -18,6 +14,11 @@ internal interface IBuildCheckResult
/// Optional location of the finding (in near future we might need to support multiple locations).
/// </summary>
string LocationString { get; }

/// <summary>
/// Location of the finding.
/// </summary>
IMSBuildElementLocation Location { get; }
string[] MessageArgs { get; }
string MessageFormat { get; }

Expand Down
22 changes: 22 additions & 0 deletions src/Framework/BuildErrorEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,28 @@ public BuildErrorEventArgs(
this.helpLink = helpLink;
}

/// <summary>
/// This constructor allows event data without ends to be initialized.
/// </summary>
/// <param name="code">event code</param>
/// <param name="file">file associated with the event</param>
/// <param name="lineNumber">line number (0 if not applicable)</param>
/// <param name="columnNumber">column number (0 if not applicable)</param>
/// <param name="message">text message</param>
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;
}

/// <summary>
/// Default constructor
/// </summary>
Expand Down
14 changes: 14 additions & 0 deletions src/Framework/BuildWarningEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,20 @@ public BuildWarningEventArgs(
this.helpLink = helpLink;
}

/// <summary>
/// This constructor allows event data without ends to be initialized.
/// </summary>
/// <param name="code">event code</param>
/// <param name="file">file associated with the event</param>
/// <param name="lineNumber">line number (0 if not applicable)</param>
/// <param name="columnNumber">column number (0 if not applicable)</param>
/// <param name="message">text message</param>
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;
Expand Down
3 changes: 3 additions & 0 deletions src/Framework/Microsoft.Build.Framework.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
<Compile Include="..\Shared\BinaryWriterExtensions.cs">
<Link>Shared\BinaryWriterExtensions.cs</Link>
</Compile>
<Compile Include="..\Shared\IMSBuildElementLocation.cs">
<Link>Shared\IMSBuildElementLocation.cs</Link>
</Compile>
</ItemGroup>

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETFramework'">
Expand Down
56 changes: 1 addition & 55 deletions src/Shared/IElementLocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 { }

/// <summary>
/// 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.
/// </summary>
/// <remarks>
/// 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).
/// </remarks>
public interface IMSBuildElementLocation
{
/// <summary>
/// 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.
/// </summary>
string File
{
get;
}

/// <summary>
/// The line number where this element exists in its file.
/// The first line is numbered 1.
/// Zero indicates "unknown location".
/// </summary>
int Line
{
get;
}

/// <summary>
/// The column number where this element exists in its file.
/// The first column is numbered 1.
/// Zero indicates "unknown location".
/// </summary>
int Column
{
get;
}

/// <summary>
/// The location in a form suitable for replacement
/// into a message.
/// </summary>
string LocationString
{
get;
}
}
}
56 changes: 56 additions & 0 deletions src/Shared/IMSBuildElementLocation.cs
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// 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.
/// </summary>
public interface IMSBuildElementLocation
{
/// <summary>
/// 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.
/// </summary>
string File
{
get;
}

/// <summary>
/// The line number where this element exists in its file.
/// The first line is numbered 1.
/// Zero indicates "unknown location".
/// </summary>
int Line
{
get;
}

/// <summary>
/// The column number where this element exists in its file.
/// The first column is numbered 1.
/// Zero indicates "unknown location".
/// </summary>
int Column
{
get;
}

/// <summary>
/// The location in a form suitable for replacement
/// into a message.
/// </summary>
string LocationString
{
get;
}
}
}

0 comments on commit 01e53f4

Please sign in to comment.