-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Polyfill clean up and source package organization #12977
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
MSBuild defines a polyfill for System.Diagnostics.CodeAnalysis.StringSyntaxAttribute, but it is incorrectly declared in the System. The following changes fix the polyfill and brings it in line with polyfill best practices. - Change #if to compiler StringSyntaxAttribute on `!NET7_0_OR_GREATER` rather than `!NET`. (StringSyntaxAttribute was new in .NET 7.) - Move StringSyntaxAttribute to the System.Diagnostics.CodeAnalysis namespace. - Fix the using directives in each file that consumes StringSyntaxAttribute. - Add a type forward for .NET 7.0+
- Change #if to compile on `!NETCOREAPP3_0_OR_GREATER` instead of `!NET`. (CallerArgumentExpressionAttribute was new in .NET Core 3.0.) - Add a type forward for .NET Core 3.0+
Update links to Compile and Content items from Microsoft.CodeAnalysis.* source packages to different folders. This keeps them separate in the Solution Explorer rather than dumping them altogether in the project root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR performs polyfill cleanup and improves source package organization in the Solution Explorer. The changes correct the namespace for StringSyntaxAttribute, update the CallerArgumentExpressionAttribute polyfill structure, and organize source package files into dedicated folders for better visibility.
- Fixed
StringSyntaxAttributepolyfill to use the correct namespace (System.Diagnostics.CodeAnalysisinstead ofSystem) - Updated
CallerArgumentExpressionAttributepolyfill to follow best practices with type forwarding - Organized source package items into separate
SourcePackages\*folders in Solution Explorer
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Framework/Polyfills/StringSyntaxAttribute.cs | Corrected namespace from System to System.Diagnostics.CodeAnalysis and added type forwarder for .NET 7+ |
| src/Framework/Polyfills/CallerArgumentExpressionAttribute.cs | Added type forwarder and restructured conditional compilation |
| src/Tasks/Warning.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Tasks/Exec.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Tasks/Error.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Shared/TaskLoggingHelper.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/TargetSkippedEventArgs.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/ProjectImportedEventArgs.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/LazyFormattedBuildEventArgs.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/ExtendedBuildWarningEventArgs.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/ExtendedBuildErrorEventArgs.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/CustomBuildEventArgs.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/CriticalBuildMessageEventArgs.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/BuildWarningEventArgs.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/BuildMessageEventArgs.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/BuildErrorEventArgs.cs | Removed unnecessary #if NET conditional around using System.Diagnostics.CodeAnalysis |
| src/Framework/Microsoft.Build.Framework.csproj | Added Link metadata to organize Microsoft.CodeAnalysis.Contracts source package files into SourcePackages\Contracts folder |
| src/Build/Microsoft.Build.csproj | Added Link metadata to organize Microsoft.CodeAnalysis.Collections and Microsoft.CodeAnalysis.PooledObjects source package files into SourcePackages\Collections and SourcePackages\PooledObjects folders respectively |
This pull request represents a bit of clean up and should have no product impact.
a342950: I noticed that the
StringSyntaxAttributepolyfill is declared in the wrong namespace. This attribute is only for IDE tooling support and probably doesn't actually work in non-.NET, since it was declared in theSystemnamespace rather thanSystem.Diagnostics.CodeAnalysis. I've fixed the polyfill and updated all of the files that had used conditional compilation gymnastics to deal with the incorrect namespace.df61bee: Small changes to the
CallerArgumentExpressionAttributepolyfill to match best practices.68b8bf2: Update links to the Compile and Content items in the
Microsoft.CodeAnalysis.*source packages to display the items in separate folders rather than dumping them into the project root. This separates them in the Solution Explorer.