Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
217 changes: 217 additions & 0 deletions src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1179,6 +1179,223 @@ public override bool Execute()
diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute);
}

// ═══════════════════════════════════════════════════════════════════════
// MSBuildTask0003: Path consumers on types that are not obviously file APIs.
// These read the file system through a path string on an unrelated type, so
// they are just as unsafe as File.*/Directory.* under multithreaded execution.
// ═══════════════════════════════════════════════════════════════════════

[Fact]
public async Task AssemblyNameGetAssemblyName_WithStringPath_ProducesDiagnostic()
{
var diags = await GetDiagnosticsAsync("""
using System.Reflection;
public class MyTask : Microsoft.Build.Utilities.Task
{
public string AssemblyPath { get; set; } = "";
public override bool Execute()
{
var name = AssemblyName.GetAssemblyName(AssemblyPath);
return true;
}
}
""");

diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute);
}

[Fact]
public async Task AssemblyNameGetAssemblyName_WithGetAbsolutePath_NoDiagnostic()
{
var diags = await GetDiagnosticsAsync("""
using System.Reflection;
using Microsoft.Build.Framework;
public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask
{
public TaskEnvironment TaskEnvironment { get; set; }
public string AssemblyPath { get; set; } = "";
public override bool Execute()
{
var name = AssemblyName.GetAssemblyName(TaskEnvironment.GetAbsolutePath(AssemblyPath));
return true;
}
}
""");

diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute);
}

[Fact]
public async Task NewAssemblyName_WithDisplayName_NoDiagnostic()
{
// AssemblyName(string assemblyName) takes a display name, not a path.
var diags = await GetDiagnosticsAsync("""
using System.Reflection;
public class MyTask : Microsoft.Build.Utilities.Task
{
public override bool Execute()
{
var name = new AssemblyName("MyAssembly, Version=1.0.0.0");
return true;
}
}
""");

diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute);
}

[Fact]
public async Task XmlDocumentLoad_WithStringPath_ProducesDiagnostic()
{
var diags = await GetDiagnosticsAsync("""
using System.Xml;
public class MyTask : Microsoft.Build.Utilities.Task
{
public override bool Execute()
{
var doc = new XmlDocument();
doc.Load("input.xml");
doc.Save("output.xml");
return true;
}
}
""");

diags.Count(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ShouldBe(2);
}

[Fact]
public async Task XmlDocumentLoad_WithStreamOverload_NoDiagnostic()
{
// Stream overloads carry no path string, so they must not be flagged.
var diags = await GetDiagnosticsAsync("""
using System.IO;
using System.Xml;
public class MyTask : Microsoft.Build.Utilities.Task
{
public override bool Execute()
{
var doc = new XmlDocument();
Stream stream = Stream.Null;
doc.Load(stream);
doc.Save(stream);
return true;
}
}
""");

diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute);
}

[Fact]
public async Task XmlDocumentNonPathMembers_NoDiagnostic()
{
// CreateElement/SelectNodes/LoadXml take names, XPath expressions and XML text —
// none of them are paths, so adding XmlDocument must not flag them.
var diags = await GetDiagnosticsAsync("""
using System.Xml;
public class MyTask : Microsoft.Build.Utilities.Task
{
public override bool Execute()
{
var doc = new XmlDocument();
doc.LoadXml("<root />");
var element = doc.CreateElement("child", "http://example.com/ns");
doc.DocumentElement!.AppendChild(element);
doc.SelectNodes("//child");
return true;
}
}
""");

diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute);
}

[Fact]
public async Task X509CertificateLoaderLoadFromFile_WithStringPath_ProducesDiagnostic()
{
var diags = await GetDiagnosticsAsync("""
using System.Security.Cryptography.X509Certificates;
public class MyTask : Microsoft.Build.Utilities.Task
{
public string CertificatePath { get; set; } = "";
public override bool Execute()
{
var certificate = X509CertificateLoader.LoadCertificateFromFile(CertificatePath);
return true;
}
}
""");

diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute);
}

[Fact]
public async Task X509CertificateLoaderLoadFromBytes_NoDiagnostic()
{
// The byte[] overload has no path parameter; 'password' must not be treated as one.
var diags = await GetDiagnosticsAsync("""
using System.Security.Cryptography.X509Certificates;
public class MyTask : Microsoft.Build.Utilities.Task
{
public override bool Execute()
{
var certificate = X509CertificateLoader.LoadPkcs12(new byte[0], "secret");
return true;
}
}
""");

diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute);
}

[Fact]
public async Task ZipFileExtensionsExtractToFile_WithStringPath_ProducesDiagnostic()
{
// ZipFileExtensions members are extension methods; the reduced call form must still be seen.
var diags = await GetDiagnosticsAsync("""
using System.IO.Compression;
using Microsoft.Build.Framework;
public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask
{
public TaskEnvironment TaskEnvironment { get; set; }
public string DestinationPath { get; set; } = "";
public override bool Execute()
{
using var archive = ZipFile.OpenRead(TaskEnvironment.GetAbsolutePath("archive.zip"));
foreach (var entry in archive.Entries)
{
entry.ExtractToFile(DestinationPath);
}
return true;
}
}
""");

var pathDiags = diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ToArray();
pathDiags.Length.ShouldBe(1);
pathDiags[0].GetMessage().ShouldContain("ExtractToFile");
}

[Fact]
public async Task XPathDocumentConstructor_WithStringPath_ProducesDiagnostic()
{
var diags = await GetDiagnosticsAsync("""
using System.Xml.XPath;
public class MyTask : Microsoft.Build.Utilities.Task
{
public string DocumentPath { get; set; } = "";
public override bool Execute()
{
var doc = new XPathDocument(DocumentPath);
return true;
}
}
""");

diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute);
}

// ═══════════════════════════════════════════════════════════════════════
// Iteration 9-13: New APIs and features
// ═══════════════════════════════════════════════════════════════════════
Expand Down
38 changes: 38 additions & 0 deletions src/TaskAnalyzer.Tests/MultiThreadableTaskCodeFixProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -249,4 +249,42 @@ public override bool Execute()
Diag(DiagnosticIds.FilePathRequiresAbsolute).WithLocation(0)
.WithArguments("new FileInfo(...)", "wrap path argument with TaskEnvironment.GetAbsolutePath()")).RunAsync();
}

[Fact]
public async Task Fix_ZipFileExtensionsStaticForm_WrapsPathArgumentNotReceiver()
{
// The static form of an extension method puts a non-path argument first. The fix must
// skip it and wrap the string path parameter, otherwise it produces code that does not compile.
await CreateFixTest(
testCode: """
using System.IO.Compression;
using Microsoft.Build.Framework;
public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask
{
public TaskEnvironment TaskEnvironment { get; set; }
public override bool Execute()
{
ZipArchive archive = null!;
{|#0:ZipFileExtensions.CreateEntryFromFile(archive, "input.txt", "entry.txt")|};
return true;
}
}
""",
fixedCode: """
using System.IO.Compression;
using Microsoft.Build.Framework;
public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask
{
public TaskEnvironment TaskEnvironment { get; set; }
public override bool Execute()
{
ZipArchive archive = null!;
ZipFileExtensions.CreateEntryFromFile(archive, TaskEnvironment.GetAbsolutePath("input.txt"), "entry.txt");
return true;
}
}
""",
Diag(DiagnosticIds.FilePathRequiresAbsolute).WithLocation(0)
.WithArguments("ZipFileExtensions.CreateEntryFromFile(ZipArchive, string, string)", "wrap path argument with TaskEnvironment.GetAbsolutePath()")).RunAsync();
}
}
1 change: 1 addition & 0 deletions src/TaskAnalyzer.Tests/TestHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ private static MetadataReference[] CreateCoreReferences()
typeof(System.Xml.XmlReader).Assembly, // System.Xml.ReaderWriter
typeof(System.IO.Compression.ZipFile).Assembly, // System.IO.Compression.ZipFile
typeof(System.IO.Compression.ZipArchive).Assembly, // System.IO.Compression
typeof(System.Security.Cryptography.X509Certificates.X509Certificate2).Assembly, // System.Security.Cryptography
};

var locations = assemblies
Expand Down
54 changes: 41 additions & 13 deletions src/TaskAnalyzer/MultiThreadableTaskCodeFixProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.Build.TaskAuthoring.Analyzer
{
Expand All @@ -38,13 +39,15 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
return;
}

var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);

foreach (var diagnostic in context.Diagnostics)
{
var node = root.FindNode(diagnostic.Location.SourceSpan);

if (diagnostic.Id == DiagnosticIds.FilePathRequiresAbsolute)
{
RegisterFilePathFix(context, node, diagnostic);
RegisterFilePathFix(context, semanticModel, node, diagnostic);
}
else if (diagnostic.Id == DiagnosticIds.TaskEnvironmentRequired)
{
Expand All @@ -53,7 +56,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
}
}

private static void RegisterFilePathFix(CodeFixContext context, SyntaxNode node, Diagnostic diagnostic)
private static void RegisterFilePathFix(CodeFixContext context, SemanticModel? semanticModel, SyntaxNode node, Diagnostic diagnostic)
{
// Find the invocation or object creation expression
var invocation = FindContainingCall(node);
Expand All @@ -75,17 +78,7 @@ private static void RegisterFilePathFix(CodeFixContext context, SyntaxNode node,
return;
}

// Find the first argument that is NOT already wrapped with TaskEnvironment.GetAbsolutePath()
ArgumentSyntax? targetArg = null;
foreach (var arg in argumentList.Arguments)
{
if (!IsAlreadyWrapped(arg.Expression))
{
targetArg = arg;
break;
}
}

var targetArg = FindPathArgumentToWrap(argumentList, semanticModel, context.CancellationToken);
if (targetArg is null)
{
return;
Expand All @@ -99,6 +92,41 @@ private static void RegisterFilePathFix(CodeFixContext context, SyntaxNode node,
diagnostic);
}

/// <summary>
/// Finds the first argument that is NOT already wrapped with TaskEnvironment.GetAbsolutePath()
/// and that binds to a string parameter whose name identifies it as a path — the same test the
/// analyzer applies when it reports the diagnostic. Skipping non-path parameters keeps the fix off
/// arguments that cannot be rooted, such as the <c>ZipArchive</c> receiver of the static form of
/// <c>ZipFileExtensions.CreateEntryFromFile(archive, sourceFileName, entryName)</c>.
/// </summary>
private static ArgumentSyntax? FindPathArgumentToWrap(
ArgumentListSyntax argumentList, SemanticModel? semanticModel, CancellationToken cancellationToken)
{
foreach (var arg in argumentList.Arguments)
{
if (IsAlreadyWrapped(arg.Expression))
{
continue;
}

// Without a semantic model the parameter cannot be inspected; fall back to the first
// unwrapped argument, which is the path for the overwhelming majority of path APIs.
if (semanticModel is null)
{
return arg;
}

if (semanticModel.GetOperation(arg, cancellationToken) is IArgumentOperation { Parameter: { } parameter } &&
parameter.Type.SpecialType == SpecialType.System_String &&
SharedAnalyzerHelpers.IsPathParameterName(parameter.Name))
{
return arg;
}
}

return null;
}

/// <summary>
/// Checks whether an argument expression is already wrapped in TaskEnvironment.GetAbsolutePath().
/// </summary>
Expand Down
Loading