Enlighten ResolveCodeAnalysisRuleSet task for multithreaded mode

[jankratochvilcz]

Enlighten ResolveCodeAnalysisRuleSet task for multithreaded mode

Parent: #11834

Context

ResolveCodeAnalysisRuleSet is one of the remaining tasks listed in the migration epic (#11834). It resolves a code-analysis ruleset reference (a simple file name, project-relative path, or full path) against the project directory plus a list of additional search directories, returning the resolved location as the [Output] ResolvedCodeAnalysisRuleSet string.

The task already accepts MSBuildProjectDirectory as an explicit input from Microsoft.Common.CurrentVersion.targets, which is helpful — most cwd-dependence is mediated through that property. However, the task still has cwd-dependent surfaces that matter under multithreaded execution:

ResolveCodeAnalysisRuleSet.Execute
  → GetResolvedRuleSetPath
       → FileSystems.Default.FileExists(Path.Combine(MSBuildProjectDirectory, CodeAnalysisRuleSet))   // simple-name + project-dir branch
       → FileSystems.Default.FileExists(Path.Combine(directory, CodeAnalysisRuleSet))                  // each CodeAnalysisRuleSetDirectories entry
       → FileSystems.Default.FileExists(Path.Combine(MSBuildProjectDirectory, CodeAnalysisRuleSet))   // relative-path branch
       → FileSystems.Default.FileExists(CodeAnalysisRuleSet)                                           // rooted-path branch

Cwd-dependence audit:

• Path.IsPathRooted(...) and Path.GetFileName(...) are pure (no cwd consumption).
• FileSystems.Default.FileExists(absolutePath) is cwd-independent iff the argument is actually absolute.
• FileSystems.Default.FileExists(CodeAnalysisRuleSet) on the rooted branch is gated by Path.IsPathRooted(...) == true, so it's cwd-independent.
• The risky cases are the two Path.Combine(MSBuildProjectDirectory, ...) branches and each Path.Combine(directory, CodeAnalysisRuleSet) in CodeAnalysisRuleSetDirectories. If MSBuildProjectDirectory is empty (the targets don't set it, the user doesn't supply it, or a custom invocation forgets it) or if any entry of CodeAnalysisRuleSetDirectories is relative, the resulting FileExists call silently consumes process Environment.CurrentDirectory.

In legacy execution that's invisible because process cwd happens to match the project directory. Under MT execution, multiple ResolveCodeAnalysisRuleSet instances may be running concurrently in the same process for projects with different directories — there is only one cwd, so any relative resolution is a correctness bug.

The trustworthy source of "what directory is this task associated with?" in MT mode is TaskEnvironment.ProjectDirectory, not the user-supplied MSBuildProjectDirectory property. The cleanest migration is to absolutize all candidate paths through TaskEnvironment before handing them to FileExists.

Approach

Apply the PR #13267 (MSBuild/CallTarget) pattern: absolutize at the task boundary, then run the existing branching logic on absolute strings.

1. Mark ResolveCodeAnalysisRuleSet with [MSBuildMultiThreadableTask] and implement IMultiThreadableTask with public TaskEnvironment TaskEnvironment { get; set; } (no default initializer — matches the built-in style used in Enlighten public versions of intrinsic tasks. #13267; the runtime default is supplied by TaskRouter).
2. In GetResolvedRuleSetPath:
   • Keep the early-exit on string.IsNullOrEmpty(CodeAnalysisRuleSet).
   • Keep the existing classification (CodeAnalysisRuleSet == Path.GetFileName(CodeAnalysisRuleSet) → simple name; Path.IsPathRooted(...) → rooted; else → relative). These are pure string operations and remain valid.
   • Replace each Path.Combine(<base>, CodeAnalysisRuleSet) site with TaskEnvironment.GetAbsolutePath(Path.Combine(<base>, CodeAnalysisRuleSet)) (or, for entries that may themselves be relative, absolutize the base first via TaskEnvironment.GetAbsolutePath(directory) and then Path.Combine). Either form ends up calling FileExists on a guaranteed-absolute string.
   • For the rooted-path FileSystems.Default.FileExists(CodeAnalysisRuleSet) branch, keep as-is (already cwd-independent).
3. Preserve [Output] ResolvedCodeAnalysisRuleSet exact string semantics (Sin 1). Today:
   • simple-name branch (file found in project dir) → returns CodeAnalysisRuleSet (the original input string)
   • simple-name branch (file found in a search dir) → returns Path.Combine(directory, CodeAnalysisRuleSet) (the original combined string, not absolutized)
   • relative-path branch → returns CodeAnalysisRuleSet (the original input)
   • rooted-path branch → returns CodeAnalysisRuleSet (the original input)
   • any branch where the file doesn't exist → returns null
     Do not let an AbsolutePath value or its .Value leak into the assignment to ResolvedCodeAnalysisRuleSet. Keep a separate "original form" string variable for the output, and use absolutized strings only for the FileExists check.
4. Preserve warning message text (Sin 2). Log.LogWarningWithCodeFromResources("Compiler.UnableToFindRuleSet", CodeAnalysisRuleSet) must continue to receive the original input string, not an absolutized form. (Today it's already the original CodeAnalysisRuleSet; just don't accidentally swap it for an absolutized variable during the refactor.)
5. Decide on MSBuildProjectDirectory vs TaskEnvironment.ProjectDirectory. Recommended: continue to honor MSBuildProjectDirectory as the explicit input (preserves backwards compatibility for callers who already set it from targets), but when it is null/empty, fall back to TaskEnvironment.ProjectDirectory rather than skipping the project-directory branches entirely. This is a strict superset of today's behavior — when both are equal (the common case), output is identical; when MSBuildProjectDirectory is empty and TaskEnvironment.ProjectDirectory is meaningful, we now find the file instead of warning MSB3884. If reviewers consider that a breaking-enough widening, gate it behind a ChangeWave; otherwise ship it as a bugfix.

Why no ChangeWave (likely)

For the dominant path — MSBuildProjectDirectory is supplied by the targets and matches the project directory — output is byte-identical to today: same [Output] string, same warning text, same Execute() return value. The semantic change manifests only when (a) running under MT with different per-task project directories, where today's behavior was already wrong, or (b) MSBuildProjectDirectory is empty and we now consult TaskEnvironment.ProjectDirectory (a strict improvement, but reviewers may still want a wave for safety — see Approach §5).

If unforeseen behavioral diffs surface during review, gate the diff behind a new ChangeWave following the precedent set in PR #13069.

Test coverage assessment

Existing direct unit tests (src/Tasks.UnitTests/ResolveCodeAnalysisRuleSet_Tests.cs, 11 cases)

GetResolvedRuleSetPath_FullPath_NonExistent, GetResolvedRuleSetPath_FullPath_Existent, GetResolvedRuleSetPath_SimpleNameAlone_NonExistent, GetResolvedRuleSetPath_SimpleNameAndProjectDirectory_Existent, GetResolvedRuleSetPath_SimpleNameAndProjectDirectory_NonExistent, GetResolvedRuleSetPath_SimpleNameAndDirectories_Existent, GetResolvedRuleSetPath_SimpleNameAndDirectories_NonExistent, GetResolvedRuleSetPath_RelativePath_WithProject_NonExistent, GetResolvedRuleSetPath_RelativePath_WithProject_Existent, GetResolvedRuleSetPath_RelativePath_NoProject, GetResolvedRuleSetPath_EmptyString, GetResolvedRuleSetPath_Null.

Coverage of the four input categories (null/empty, simple name, relative, rooted) crossed with project-dir / search-dir presence is good. Coverage of MT-specific concerns (concurrency, project-relative resolution when cwd ≠ project dir, TaskEnvironment injection) is zero.

Integration tests in this repo

None directly. ResolveCodeAnalysisRuleSet is invoked from Microsoft.Common.CurrentVersion.targets in the ResolveCodeAnalysisRuleSet target, feeding compiler invocations downstream. End-to-end coverage of that flow lives in the C#/VB compiler integration tests in dotnet/sdk and Roslyn, not here. We will not add E2E compile tests in this repo as part of this issue.

Gaps to fill in this PR

• G1 — Concurrency test. Two ResolveCodeAnalysisRuleSet instances with different TaskEnvironment.ProjectDirectory values, each with a ruleset file present only in its own project directory under the same simple name. Assert each task resolves to its own file (i.e., resolution is correctly scoped per-task, not via shared process cwd). (SKILL red-team Phase 4.)
• G2 — Project-relative resolution when cwd ≠ project dir. Set Environment.CurrentDirectory (via TestEnvironment.SetCurrentDirectory) to a directory that does not contain the ruleset, set TaskEnvironment.ProjectDirectory to a directory that does, leave MSBuildProjectDirectory empty, and assert the file is still found. Documents the intentional behavior under MT and the fallback described in Approach §5.
• G3 — Inject TaskEnvironment into existing tests. Set task.TaskEnvironment = TaskEnvironmentHelper.CreateForTest() in every existing test so that the MSBuildProjectDirectory = null / empty branches still go through a defined fallback (cwd-backed TaskEnvironment.Fallback) instead of NRE'ing on the new code path.
• G4 — Baseline parity matrix. Reassert each of the existing semantics on the migrated code:
  • simple name + project dir + file present → [Output] is original simple-name string (not absolutized)
  • simple name + search dir + file present → [Output] is Path.Combine(directory, simpleName) exact string (not absolutized)
  • relative path + project dir + file present → [Output] is original relative-path string (not absolutized)
  • rooted path + file present → [Output] is original rooted string (not absolutized)
  • missing file in any branch → [Output] is null and warning text contains the original CodeAnalysisRuleSet input verbatim (Sin 2)
• G5 — Relative entry in CodeAnalysisRuleSetDirectories. Pass a relative directory in CodeAnalysisRuleSetDirectories. Today this is silently cwd-dependent; under MT it must be absolutized via TaskEnvironment before FileExists. Assert the file is found when present under TaskEnvironment.ProjectDirectory + relativeSearchDir and the resulting [Output] retains the original combined string (preserving the existing behavior that this branch returns the un-absolutized combined path).

Acceptance criteria

• ResolveCodeAnalysisRuleSet decorated [MSBuildMultiThreadableTask] and implements IMultiThreadableTask with a TaskEnvironment property (no default initializer — matches built-in style).
• Every FileSystems.Default.FileExists(...) call in GetResolvedRuleSetPath receives an absolute path (either inherently rooted, or absolutized via TaskEnvironment.GetAbsolutePath).
• The four classification branches (null/empty, simple name, relative, rooted) are preserved; Path.IsPathRooted and Path.GetFileName are unchanged.
• [Output] ResolvedCodeAnalysisRuleSet exact string is byte-identical to current behavior in all five existing-success branches (simple-name in project dir, simple-name in search dir, relative in project dir, rooted, and null when missing). No AbsolutePath leak (Sin 1).
• MSB3884 (Compiler.UnableToFindRuleSet) warning text contains the original CodeAnalysisRuleSet input string verbatim, not an absolutized form (Sin 2).
• All existing 11 ResolveCodeAnalysisRuleSet_Tests pass on net472 and net10.0 with no behavior changes.
• MSBuildProjectDirectory continues to be honored when supplied; falls back to TaskEnvironment.ProjectDirectory when null/empty (or, if reviewers prefer, gated behind a ChangeWave).
• Tests G1, G2, G3, G4, G5 added and pass.
• .\build.cmd -v quiet succeeds; Tasks.UnitTests and Build.UnitTests pass.
• No new compiler warnings (warnings-as-errors in official build).
• No ChangeWave required; if any test required modification beyond G3, document why in the PR and gate the diff behind the next wave.
• multithreaded-task-migration SKILL sign-off checklist walked and passes.

Implementation order

1. Apply attribute + interface to ResolveCodeAnalysisRuleSet. Add TaskEnvironment property.
2. Update GetResolvedRuleSetPath to absolutize each Path.Combine(...) candidate with TaskEnvironment.GetAbsolutePath before calling FileExists. Hold the original (un-absolutized) candidate string in a separate variable; only the [Output] and warning paths see the original.
3. Implement the MSBuildProjectDirectory → TaskEnvironment.ProjectDirectory fallback (or skip if reviewers prefer to keep behavior strictly identical and ChangeWave-gate the widening).
4. Update existing 11 ResolveCodeAnalysisRuleSet_Tests to inject test TaskEnvironment (G3).
5. Add G1 (concurrency), G2 (project-relative under MT), G4 (output-string parity matrix), G5 (relative search-dir entry) tests.
6. Run full Tasks.UnitTests and Build.UnitTests; verify clean.
7. Run repo build with -v quiet to ensure no new warnings.

Risks / open questions

• The fallback from empty MSBuildProjectDirectory to TaskEnvironment.ProjectDirectory (Approach §5) widens the set of files we'll now successfully resolve. In legacy mode the user could see a previously-warning build start succeeding silently. Likely benign (and arguably a bugfix), but reviewers may request ChangeWave gating.
• Compiler integration under MT is not exercised by this repo's tests. Coordination with Roslyn / dotnet/sdk recommended before MT mode ships generally, but not blocking for this PR — the task itself is correct in isolation once migrated.

References

• Parent epic: Migrate Tasks in the MSbuild repo to the new Task type #11834
• Pattern PRs:
  • Enlighten public versions of intrinsic tasks. #13267 — Enlighten public versions of intrinsic tasks (absolutize-at-boundary pattern)
  • Enlighten FindUnderPath and AssignTargetPath tasks #13069 — Enlighten FindUnderPath and AssignTargetPath (ChangeWave usage)
  • Enlighten more tasks that require no change #13045 — Enlighten more tasks that require no change (attribute-only baseline)
  • Enlighten GetFrameworkPath and GetFrameworkSdkPath. #13282 — Enlighten GetFrameworkPath and GetFrameworkSdkPath (locking shared static state)
  • Enlighten RAR task #13319 — Enlighten RAR (heavyweight thread-TaskEnvironment-through-helpers pattern; for reference only)
  • Enlighten FormatUrl task for multithreaded mode #13567 — Enlighten FormatUrl task (sibling task, same pattern as this issue)
• SKILL: .github/skills/multithreaded-task-migration/SKILL.md