Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 23, 2025

This PR adds a new PropertyExists() condition function to MSBuild that allows checking whether a property is actually defined, rather than relying on the current behavior where undefined properties are coerced to empty strings.

Problem

Currently in MSBuild, there's no way to distinguish between:

  1. A property that is explicitly set to an empty value: <Property></Property>
  2. A property that is completely undefined

Both scenarios result in an empty string when the property is referenced, making it impossible to implement proper coalescing logic where an explicitly empty property should override another property's value.

Solution

Added a new PropertyExists('PropertyName') function that:

  • Returns true if the property exists in the project (even if it has an empty value)
  • Returns false if the property is completely undefined
  • Supports property name expansion: PropertyExists('$(SomePropertyName)')

Usage Examples

<!-- Before: Can't distinguish between empty and undefined -->
<PropertyGroup>
  <Result Condition="'$(SomeProperty)' == ''">Property is empty OR undefined</Result>
</PropertyGroup>

<!-- After: Can distinguish properly -->
<PropertyGroup>
  <Result Condition="PropertyExists('SomeProperty') and '$(SomeProperty)' == ''">Property exists but is empty</Result>
  <Result Condition="!PropertyExists('SomeProperty')">Property is truly undefined</Result>
</PropertyGroup>

<!-- Real-world coalescing scenario -->
<PropertyGroup>
  <_UserSpecified Condition="PropertyExists('ToolPackageRuntimeIdentifiers')">$(ToolPackageRuntimeIdentifiers)</_UserSpecified>
  <_UserSpecified Condition="!PropertyExists('ToolPackageRuntimeIdentifiers')">$(RuntimeIdentifiers)</_UserSpecified>
</PropertyGroup>

Implementation

  • Extended IConditionEvaluationState interface with PropertyExists(string propertyName) method
  • Added implementation in ConditionEvaluationState that delegates to Expander.PropertyExists()
  • Added PropertyExists() method to Expander class that checks if property is non-null
  • Added PropertyExists function case to FunctionCallExpressionNode.BoolEvaluate()
  • Comprehensive unit tests covering normal usage, expansion, error cases, and edge cases

Testing

  • All existing tests continue to pass (no regressions)
  • New unit tests verify correct behavior for existing, empty, and undefined properties
  • Manual testing confirms the function works with built-in properties, environment variables, and user-defined properties
  • Backward compatibility maintained with existing Exists() and HasTrailingSlash() functions

Fixes #12404.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copy link
Contributor

Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo.

@Copilot Copilot AI changed the title [WIP] Add an MSBuild Property Function to check if a property actually exists (vs relying on coercion to empty string) Add PropertyExists MSBuild condition function to distinguish between undefined and empty properties Aug 23, 2025
@Copilot Copilot AI requested a review from baronfel August 23, 2025 15:17
Copilot finished work on behalf of baronfel August 23, 2025 15:17
@baronfel baronfel marked this pull request as ready for review August 23, 2025 16:52
@Copilot Copilot AI review requested due to automatic review settings August 23, 2025 16:52
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

/// </summary>
internal bool PropertyExists(string propertyName)
{
P property = _properties.GetProperty(propertyName, 0, propertyName.Length - 1);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the IPropertyProvider<T>.GetProperty(string name) method, instead of doing the length calculations here?

T GetProperty(string name);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main motivation for this change would be to remove the propertyName.Length - 1 expression that looks like an off-by-one error, even though it isn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an MSBuild Property Function to check if a property actually exists (vs relying on coercion to empty string)
3 participants