TaskAnalyzer: null-conditional access defeats safe-path detection (fi.Directory?.FullName flagged, fi.Directory.FullName not)
- Dominant language
- C#
- Stars
- 5.5k
- Forks
- 1.5k
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 141
Description
`SharedAnalyzerHelpers.IsWrappedSafely` never unwraps `IConditionalAccessOperation`, so any safe path expression reached through `?.` falls through to `return false` and gets flagged.
Found while migrating arcade tasks (dotnet/arcade#17507), where the only workaround was to restructure the code to avoid `?.`.
### Repro
Inside a `[MSBuildMultiThreadableTask]` task, with `fi` a `FileInfo` obtained safely:
```csharp
Directory.CreateDirectory(fi.Directory.FullName); // OK - no diagnostic
Directory.CreateDirectory(fi.Directory?.FullName); // MSBuildTask0003
```
Both expressions have identical path semantics, but only the second is flagged:
> MSBuildTask0003: `Directory.CreateDirectory(...)` may resolve a relative path against the shared working directory
The same gap applies to the other safe forms, e.g. `item?.GetMetadata("FullPath")` and `TaskEnvironment?.GetAbsolutePath(p)`.
### Cause
`IsWrappedSafely` unwraps `IConversionOperation` in a loop (SharedAnalyzerHelpers.cs:125) but has no case for `IConditionalAccessOperation` — there are no references to that operation kind anywhere in `src/TaskAnalyzer`. For `a?.B`, Roslyn produces an `IConditionalAccessOperation` whose `WhenNotNull` holds the actual member access, so none of the existing safe-expression rules are ever reached.
### Suggested fix
Recurse into `WhenNotNull`, next to the existing conversion unwrapping:
```csharp
// Unwrap null-conditional access: `info?.FullName` has the same path
// semantics as `info.FullName`; the null branch yields no path at all.
if (operation is IConditionalAccessOperation conditionalAccess)
{
return IsWrappedSafely(conditionalAccess.WhenNotNull, taskEnvironmentType, absolutePathType, iTaskItemType);
}
```
The receiver inside `WhenNotNull` is an `IConditionalAccessInstanceOperation` placeholder, but the rules that matter here (`FileSystemInfo.FullName`, `ITaskItem.GetMetadata`, `TaskEnvironment.GetAbsolutePath`) match on the member itself and do not inspect the receiver, so the recursion resolves correctly.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in src/TaskAnalyzer/SharedAnalyzerHelpers.cs at IsWrappedSafely and review the existing conversion unwrapping and safe-expression rules. Reproduce the two Directory.CreateDirectory expressions and the other null-conditional forms from the issue, then verify that safe paths reached through IConditionalAccessOperation are no longer diagnosed while unsafe paths remain covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- build-system
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 86/100