TaskAnalyzer: no diagnostic when a path property is resolved at one use site but used raw at another

[ViktorHofer]

Issue Description

There is no diagnostic for inconsistent path resolution: a task property that is correctly wrapped in TaskEnvironment.GetAbsolutePath() at one use site, but consumed raw at a sibling site in the same method.

This is distinct from the "Insufficient path-state tracking" item under MSBuildTask0003 in #14772. That item is about the state of a resolved value (canonical / qualified / command name). This is about the unresolved value continuing to be used alongside the resolved one — typically in a length offset or a prefix comparison, where no path API is involved at all, so no existing rule looks at it.

It is the single most common bug class we hit in the Arcade migration: 10 real defects across dotnet/arcade#17381, every one of them introduced by following the analyzer's own MSBuildTask0003 guidance on only the call site it flagged.

Steps to Reproduce

[MSBuildMultiThreadableTask]
public sealed class PartialResolutionTask : Task, IMultiThreadableTask
{
    public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;

    [Required]
    public string Root { get; set; }

    public override bool Execute()
    {
        foreach (string f in Directory.GetFiles(TaskEnvironment.GetAbsolutePath(Root), "*", SearchOption.AllDirectories))
        {
            // Root is still relative here, but f is absolute.
            string rel = f.Substring(Root.Length + 1);

            if (!f.StartsWith(Root, StringComparison.Ordinal))
            {
                Log.LogMessage(rel);
            }
        }

        return true;
    }
}

Expected Behavior

A diagnostic on the Root.Length and StartsWith(Root) uses, noting that Root is resolved elsewhere in the same scope and that mixing resolved and unresolved forms of the same value produces incorrect results.

Actual Behavior

No diagnostic. The only thing reported is MSBuildTask0006 on the property itself:

Repro.cs(80,53): warning MSBuildTask0006: Consider changing task property 'Root' from 'string' to 'AbsolutePath' instead of converting inside the task body

MSBuildTask0006 is a good hint here, but it is advisory, is not a correctness statement, and does not point at the two defective expressions.

Why This Matters More Than It Looks

Every one of these produces a silently wrong result rather than an exception:

• f.Substring(Root.Length + 1) — f is absolute, Root is relative, so the offset is short by the length of the working directory prefix. The result is a mangled relative path, not a crash.
• f.StartsWith(Root) — always false, so a filter silently matches nothing (or everything, when negated).

Because there is no exception, these survive unit tests that use absolute inputs, and only manifest when the task is invoked with a relative property value.

A concrete example from the migration, WritePackageUsageData.cs, had five use sites of RootDir. MSBuildTask0003 flagged the Directory.GetFiles call. The other four — two Path.Combine, one Substring offset, one StartsWith — were invisible to the analyzer, and the last of them was missed by human code review as well.

Suggested Fix

Within a method body, when a symbol (task property, parameter, or local) flows into TaskEnvironment.GetAbsolutePath() at least once, report the other reads of that same symbol that flow into:

• string.Length used as an index or offset against a value derived from the resolved form,
• StartsWith / EndsWith / IndexOf / Equals / Replace against such a value,
• Path.Combine / Path.GetDirectoryName / any path-shaped API,

with a message along the lines of "'Root' is resolved with TaskEnvironment.GetAbsolutePath elsewhere in this method but is used unresolved here; the two forms are not interchangeable."

This would also make the MSBuildTask0006 suggestion actionable, since converting the property to AbsolutePath is precisely the fix that removes the whole class.

We wrote a crude standalone checker for exactly this pattern while auditing the migration and it had a very high signal-to-noise ratio — it found one true positive that 24 inline review comments had missed, with no false positives across 136 tasks. Happy to share it if useful.

Versions & Configurations

• Microsoft.Build.TaskAuthoring.Analyzer 18.11.0-1.26420.118
• Microsoft.Build.Utilities.Core 18.8.2
• msbuild_task_analyzer.scope = multithreadable_only

Related

• Roslyn analyzers for MSBuild tasks #14772, Multithreading: docs and analyzer should reflect that [MSBuildMultiThreadableTask] is the routing signal, not IMultiThreadableTask #14779, TaskAnalyzer: MSBuildTask0003 code fix produces non-compiling code for nested calls and static methods #14780
• dotnet/arcade#17378, dotnet/arcade#17381

[ViktorHofer]

Two real instances from a ~150-task migration in dotnet/arcade and the dotnet/dotnet VMR, in case concrete cases help with rule design. Both passed the analyzer cleanly.

1. CheckForPoison.cs (dotnet/dotnet eng/tools) — the resolved and unresolved uses are three lines apart, on the same variable:

AssemblyName asm = AssemblyName.GetAssemblyName(fileToCheck);                       // raw
...
if (... IsAssemblyFromSbrp(TaskEnvironment.GetAbsolutePath(fileToCheck)))           // resolved
else if (IsAssemblyPoisoned(TaskEnvironment.GetAbsolutePath(fileToCheck)))          // resolved

The consequence is worse than a bad path. That first call is only an "is this an assembly?" probe, and the enclosing catch treats any exception as "not an assembly" — so an unresolved path throws FileNotFoundException, gets swallowed, and both poison checks are skipped. Green build, silently weakened leak-detection gate.

2. GetAssemblyFullName (dotnet/arcade) — same shape, AssemblyName.GetAssemblyName on a raw input while neighbouring code resolves.

Both also depend on #14791 (AssemblyName.GetAssemblyName is not a monitored API), so neither would be reported today even with this issue fixed. They are still good test cases for the rule proposed here, since the inconsistency is the signal that does not require knowing the sink.

One design note from triaging ~80 candidate findings by hand: a "resolved elsewhere" heuristic has real false-positive potential, because a task legitimately mixes resolved paths with strings that are not paths at all. What made these two unambiguous was that the same local variable was passed resolved at one site and raw at another. Scoping the rule to per-variable inconsistency, rather than per-property, would keep it precise.