Skip to content

Commit

Permalink
Handle changes to ExcludeFromCurrentConfigurationProperty
Browse files Browse the repository at this point in the history
This change requires a good bit of background.

The Solution Explorer builds its tree from the evaluation model of the "active" configuration and only the active configuration, and there can only be one active configuration. Items (source files, resource files, etc.) that aren't part of the active configuration won't be displayed.

In MAUI projects, the items under the "Platforms" folder are meant to be platform-specific (by which we really mean target framework-specific), with each relevant platform getting its own subfolder. Items in the "Android" subfolder should not be included in the "iOS" build, for example.

The most straightforward way to approach this would be to conditionally include the various `<Compile>` items under Platforms based on the target framework. That would work correctly for the purposes of the build, but means that not all of the items would display in the Solution Explorer--only those belonging to whatever platform is associated with that "active" configuration.

Instead, the items are included for *all* target frameworks during MSBuild evaluation, and then the extraneous ones are removed by targets during the build. This ensures they always appear in Solution Explorer and the build still works as expected.

However the language service integration in the .NET Project System utilizes both evaluation and design-time data and has to reconcile the two as best it can. One thing it can't handle on its own are items that are present in evaluation but removed by a target. To help with this, we added the concept of `ExcludeFromCurrentConfiguration` metadata for items; we can use this to tell the language service integration to _pretend_ the item doesn't exist in evaluation even though it does. The MAUI .props and .targets end up defining the items under the Platforms folder in all configurations, but the metadata on those items varies depending on whether or not the item should actually be passed to the Language Service.

It's important to step back at this point and call out that there are a couple layers of workarounds going on here: MAUI includes the items in every configuration to work around the fact that CPS can't build the Solution Explorer tree from multiple target frameworks, and the language service needs `ExcludeFromCurrentConfiguration` metadata to work around the fact that the items are in every configuration.

And now we can finally talk about bug AB#1895917. The above scheme works well enough when the `ExcludeFromCurrentConfiguration` metadata is properly applied to the items. The problem is that the metadata is added by a .targets file (Microsoft.Maui.Controls.SingleProject.targets) that comes from a NuGet package (Microsoft.Maui.Controls.Build.Tasks). If an evaluation occurs before NuGet restore runs and the Language Service integration processes that evaluation, we will see the items without the metadata. This means that we will pass Android-specific source files along as if they were also part of the Mac, iOS, and Windows builds--and do the same for every other platform-specific file. This will lead to spurious errors in the IDE (e.g. the Windows build complaining about the Andoid APIs as those aren't defined). More to the point of this bug, Hot Reload won't work because the IDE will notice the discrepancy between the types it knows about and what is actually in the built assembly and think that it missed some edits.

The fix here is to also look out for changes in the `ExcludeFromCurrentConfiguration` metadata. If a file was previously included and we get a new evaluation saying it is excluded, we need to tell the Language Service to remove it. And if it was previously excluded and now is included, we need to tell the Language Service to add it. This way even if we provided the "wrong" data to the Language Service initially, once NuGet restore has run and we process the next evaluation we can fix everything up.
  • Loading branch information
tmeschter committed Oct 17, 2023
1 parent 72e29cf commit bea583e
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,7 @@ private void ApplyChangesToContext(IWorkspaceProjectContext context, IProjectCha

foreach (string includePath in difference.ChangedItems)
{
UpdateInContextIfPresent(context, includePath, previousMetadata, currentMetadata, isActiveContext, logger);

// TODO: Check for changes in the metadata indicating if we should ignore the file
// in the current configuration.
HandleEvaluationMetadataChange(context, includePath, previousMetadata, currentMetadata, isActiveContext, logger);
}
}

Expand Down Expand Up @@ -223,16 +220,38 @@ private void AddToContextIfNotPresent(IWorkspaceProjectContext context, string i
}
}

private void UpdateInContextIfPresent(IWorkspaceProjectContext context, string includePath, IImmutableDictionary<string, IImmutableDictionary<string, string>> previousMetadata, IImmutableDictionary<string, IImmutableDictionary<string, string>> currentMetadata, bool isActiveContext, IManagedProjectDiagnosticOutputService logger)
/// <remarks>
/// This should only be called for evaluation changes. The items we get from design-time builds represent
/// command line arguments and won't have metadata.
/// </remarks>
private void HandleEvaluationMetadataChange(IWorkspaceProjectContext context, string includePath, IImmutableDictionary<string, IImmutableDictionary<string, string>> previousMetadata, IImmutableDictionary<string, IImmutableDictionary<string, string>> currentMetadata, bool isActiveContext, IManagedProjectDiagnosticOutputService logger)
{
string fullPath = _project.MakeRooted(includePath);

if (_paths.Contains(fullPath))
// A change in ExcludeFromCurrentConfiguration metadata needs to be processed as an add or remove rather
// than an update, so check for that first.
bool previouslyIncluded = IsItemInCurrentConfiguration(includePath, previousMetadata);
bool currentlyIncluded = IsItemInCurrentConfiguration(includePath, currentMetadata);

if (previouslyIncluded && !currentlyIncluded)
{
RemoveFromContextIfPresent(context, includePath, logger);
}
else if (!previouslyIncluded && currentlyIncluded)
{
IImmutableDictionary<string, string> previousItemMetadata = previousMetadata.GetValueOrDefault(includePath, ImmutableStringDictionary<string>.EmptyOrdinal);
IImmutableDictionary<string, string> currentItemMetadata = currentMetadata.GetValueOrDefault(includePath, ImmutableStringDictionary<string>.EmptyOrdinal);
AddToContextIfNotPresent(context, includePath, currentMetadata, isActiveContext, logger);
}
else
{
// No change to ExcludeFromCurrentConfiguration; handle as an update.

string fullPath = _project.MakeRooted(includePath);

UpdateInContext(context, fullPath, previousItemMetadata, currentItemMetadata, isActiveContext, logger);
if (_paths.Contains(fullPath))
{
IImmutableDictionary<string, string> previousItemMetadata = previousMetadata.GetValueOrDefault(includePath, ImmutableStringDictionary<string>.EmptyOrdinal);
IImmutableDictionary<string, string> currentItemMetadata = currentMetadata.GetValueOrDefault(includePath, ImmutableStringDictionary<string>.EmptyOrdinal);

UpdateInContext(context, fullPath, previousItemMetadata, currentItemMetadata, isActiveContext, logger);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void AddEvaluationChanges_CanAddItemWithMetadata()
var difference = IProjectChangeDiffFactory.WithAddedItems("A.cs");
var metadata = MetadataFactory.Create("A.cs", ("Name", "Value"));

ApplyProjectEvaluation(context, handler, 1, difference, metadata);
ApplyProjectEvaluation(context, handler, 1, difference, metadata: metadata);

var result = handler.Files[@"C:\Project\A.cs"];

Expand All @@ -117,7 +117,7 @@ public void AddEvaluationChanges_ItemsWithExclusionMetadataAreIgnored()
.Add("B.cs", ("ExcludeFromCurrentConfiguration", "false"));


ApplyProjectEvaluation(context, handler, 1, difference, metadata);
ApplyProjectEvaluation(context, handler, 1, difference, metadata: metadata);

string[] expectedFiles = new[] { @"C:\Project\B.cs", @"C:\Project\C.cs" };
Assert.Equal(expectedFiles.OrderBy(f => f), handler.FileNames.OrderBy(f => f));
Expand Down Expand Up @@ -311,7 +311,7 @@ public void ApplyProjectEvaluationChanges_WithExistingEvaluationChanges_CanAddCh
var difference = IProjectChangeDiffFactory.WithChangedItems(file);
var metadata = MetadataFactory.Create(file, ("Name", "Value"));

ApplyProjectEvaluation(context, handler, 2, difference, metadata);
ApplyProjectEvaluation(context, handler, 2, difference, metadata: metadata);

var result = handler.Files[@"C:\Project\A.cs"];

Expand Down Expand Up @@ -372,10 +372,56 @@ public void ApplyProjectBuild_WhenOlderEvaluationChangesWithRemovedConflict_Desi
Assert.Single(handler.FileNames, @"C:\Project\Source.cs");
}

private static void ApplyProjectEvaluation(IWorkspaceProjectContext context, AbstractEvaluationCommandLineHandler handler, IComparable version, IProjectChangeDiff difference, IImmutableDictionary<string, IImmutableDictionary<string, string>>? metadata = null)
[Fact]
public void ApplyProjectEvaluation_ChangingExclusionMetadata_IncludesFile()
{
var handler = CreateInstance(@"C:\Project\Project.csproj");
var context = IWorkspaceProjectContextMockFactory.Create();

var metadata = MetadataFactory.Create("Source.cs", ("ExcludeFromCurrentConfiguration", "true"));

ApplyProjectEvaluation(context, handler, version: 0, IProjectChangeDiffFactory.WithAddedItems("Source.cs"), metadata: metadata);

Assert.Empty(handler.FileNames);

var previousMetadata = metadata;
metadata = MetadataFactory.Create("Source.cs", ("ExcludeFromCurrentConfiguration", "false"));

ApplyProjectEvaluation(context, handler, version: 1, IProjectChangeDiffFactory.WithChangedItems("Source.cs"), previousMetadata, metadata);

Assert.Single(handler.FileNames, @"C:\Project\Source.cs");
}

[Fact]
public void ApplyProjectEvaluation_ChangingExclusionMetadata_ExcludesFile()
{
var handler = CreateInstance(@"C:\Project\Project.csproj");
var context = IWorkspaceProjectContextMockFactory.Create();

var metadata = MetadataFactory.Create("Source.cs", ("ExcludeFromCurrentConfiguration", "false"));

ApplyProjectEvaluation(context, handler, version: 0, IProjectChangeDiffFactory.WithAddedItems("Source.cs"), metadata: metadata);

Assert.Single(handler.FileNames, @"C:\Project\Source.cs");

var previousMetadata = metadata;
metadata = MetadataFactory.Create("Source.cs", ("ExcludeFromCurrentConfiguration", "true"));

ApplyProjectEvaluation(context, handler, version: 1, IProjectChangeDiffFactory.WithChangedItems("Source.cs"), previousMetadata, metadata);

Assert.Empty(handler.FileNames);
}

private static void ApplyProjectEvaluation(
IWorkspaceProjectContext context,
AbstractEvaluationCommandLineHandler handler,
IComparable version,
IProjectChangeDiff difference,
IImmutableDictionary<string, IImmutableDictionary<string, string>>? previousMetadata = null,
IImmutableDictionary<string, IImmutableDictionary<string, string>>? metadata = null)
{
metadata ??= ImmutableDictionary<string, IImmutableDictionary<string, string>>.Empty;
var previousMetadata = ImmutableDictionary<string, IImmutableDictionary<string, string>>.Empty;
previousMetadata ??= ImmutableDictionary<string, IImmutableDictionary<string, string>>.Empty;
bool isActiveContext = true;
var logger = IManagedProjectDiagnosticOutputServiceFactory.Create();

Expand Down

0 comments on commit bea583e

Please sign in to comment.