MSBuild project file anti-patterns: unquoted conditions, missing PrivateAssets, TargetFramework in .props
- Dominant language
- C#
- Stars
- 5.5k
- Forks
- 1.5k
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 141
Description
## Summary
An automated scan of the MSBuild project files (`.csproj`, `.props`, `.targets`) identified several anti-patterns that could lead to subtle build issues. The findings are organized by severity.
---
## 🔴 High Severity
### AP-02: Unquoted Condition Expression
**File:** `src/Directory.Build.props`, line 98
```xml
```
Both sides of the `==` comparison should be single-quoted. If `$(IsInnerBuild)` is empty or contains special characters, this condition can parse incorrectly.
**Fix:**
```xml
```
---
### AP-07: Missing `PrivateAssets="all"` on Analyzer Package
**File:** `eng/dependabot/Directory.Packages.props`, line 55
```xml
```
Analyzer packages should use `PrivateAssets="all"` to prevent them from flowing as transitive dependencies to consumers of published NuGet packages. The `StyleCop.Analyzers` entry on the next line already has this — likely an oversight.
**Fix:**
```xml
```
---
### AP-21: Property Conditioned on `TargetFramework` in `.props` Files
**Files:**
- `src/Directory.Build.props` — lines 69, 75, 85
- `eng/BootStrapMsBuild.props` — lines 13, 17
```xml
```
`$(TargetFramework)` is **not reliably set** during `.props` evaluation for single-targeting projects. It is only available as a global property for inner builds of multi-targeting (`TargetFrameworks`) projects. The repo has ~30 single-targeting projects (Package projects, test assets, utilities) where these conditions silently evaluate incorrectly:
- Line 69 condition is always `False` (even for `net472` single-target projects)
- Line 75 condition is always `True` (even for `net472` single-target projects)
**Recommended fix:** Move these `PropertyGroup` blocks to `src/Directory.Build.targets` or `src/Directory.BeforeCommon.targets` (which already contains similar TargetFramework-based conditions).
> **Note:** `ItemGroup` and `Target` conditions on `$(TargetFramework)` in `.props` are safe — only `PropertyGroup`/property conditions are affected.
---
## 🟡 Medium Severity
### AP-11: Custom Target Missing `Inputs`/`Outputs`
**File:** `src/MSBuild/MSBuild.csproj`, line 200
```xml
```
This file-copying target runs on every build. Adding `Inputs`/`Outputs` would allow MSBuild to skip it when files are up-to-date, improving incremental build performance.
### AP-16: PowerShell Exec for JSON Manipulation
**File:** `eng/Tools.props`, line 12
A complex PowerShell one-liner is used for JSON file manipulation in the `UpdateOptProfMetadataJson` target. This is not cross-platform and opaque to MSBuild logging. Low priority since it only runs in specific CI scenarios.
---
## ✅ Clean Areas
The scan found **no issues** for 14 other anti-pattern categories including: no hardcoded paths (AP-03), no legacy `packages.config` (AP-06), proper Central Package Management (AP-09), no side effects during evaluation (AP-19), and platform-guarded Exec commands (AP-20).
---
## Context
- **Scan scope:** 72 `.csproj`, 32 `.props`, 34 `.targets`, 6 `.proj` files
- **Excluded:** Test assets in `src/BuildCheck.UnitTests/TestAssets/` and `src/Build.UnitTests/TestAssets/` (intentionally non-standard for testing)
- **Anti-pattern catalog:** Based on the [MSBuild anti-patterns reference](https://learn.microsoft.com/en-us/visualstudio/msbuild/build-process-overview) covering AP-01 through AP-21
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.