Skip to content

Commit

Permalink
Merge pull request #9606 from drewnoakes/lang-svc-version-regression
Browse files Browse the repository at this point in the history 
Fix NFE about version regression on configuration switch
  • Loading branch information
@drewnoakes
drewnoakes authored Dec 3, 2024
2 parents 079fefc + 3174149 commit 5983ea9
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 16 deletions.
48 changes: 44 additions & 4 deletions ...soft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/Workspace.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ void ProcessProjectEvaluationHandlers()
// for example from Debug to Release.
ConfiguredProject configuredProject = update.Value.ConfiguredProject;

IComparable version = GetConfiguredProjectVersion(update);
IComparable version = GetVersion(update);

foreach (IProjectEvaluationHandler evaluationHandler in _updateHandlers.EvaluationHandlers)
{
Expand Down Expand Up @@ -448,7 +448,7 @@ void InvokeCommandLineUpdateHandlers()

Assumes.Present(parser);

IComparable version = GetConfiguredProjectVersion(update);
IComparable version = GetVersion(update);

string baseDirectory = _unconfiguredProject.GetProjectDirectory();

Expand Down Expand Up @@ -543,9 +543,9 @@ void UpdateProgressRegistration()
}
}

private static IComparable GetConfiguredProjectVersion(IProjectValueVersions update)
private static IComparable GetVersion(IProjectValueVersions update)
{
return update.DataSourceVersions[ProjectDataSources.ConfiguredProjectVersion];
return new CompositeVersion(update.DataSourceVersions);
}

public async Task WriteAsync(Func<IWorkspace, Task> action, CancellationToken cancellationToken)
Expand Down Expand Up @@ -599,6 +599,46 @@ internal void Fault(Exception exception)
_contextCreated.TrySetException(exception);
}

/// <summary>
/// Combines both the configured project version and the active configured project version
/// into a single comparable value. In this way, we can ensure we order updates correctly,
/// even when the active configuration changes.
/// </summary>
private sealed class CompositeVersion(IImmutableDictionary<NamedIdentity, IComparable> versions) : IComparable
{
private readonly IComparable _configuredProjectVersion = versions[ProjectDataSources.ConfiguredProjectVersion];
private readonly IComparable _activeConfigurationVersion = versions[ProjectDataSources.ActiveProjectConfiguration];

public int CompareTo(object obj)
{
if (obj is not CompositeVersion other)
{
throw new ArgumentException("Cannot compare to a non-CompositeVersion object.");
}

// The active configuration version will only increase. Every time the configuration
// changes, this value goes up.
//
// We check this *before* checking the configured project version, because when
// switching active configuration, the configured project version can go *down*.
// However, it's important for our processing of evaluation and build data that
// the version is monotonic (only increases).
//
// We achieve monotonicity by combining these two versions.
int c = _activeConfigurationVersion.CompareTo(other._activeConfigurationVersion);

if (c is not 0)
{
// Active configuration differs, so compare on this alone.
return c;
}

// Active configuration is identical. Compare the configured project versions
// for that configuration directly.
return _configuredProjectVersion.CompareTo(other._configuredProjectVersion);
}
}

/// <summary>
/// Maps from our property data snapshot to Roslyn's API for accessing the project's
/// evaluation data.
Expand Down
36 changes: 24 additions & 12 deletions ...io.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/LanguageServices/WorkspaceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public async Task Dispose_DisposesHandlers()
factory: () => Mock.Of<IWorkspaceUpdateHandler>(MockBehavior.Loose),
disposeAction: () => disposeCount++);

var workspace = await CreateInstanceAsync(updateHandlers: new UpdateHandlers(new[] { exportFactory }));
var workspace = await CreateInstanceAsync(updateHandlers: new UpdateHandlers([exportFactory]));

Assert.Equal(0, disposeCount);

Expand Down Expand Up @@ -382,7 +382,7 @@ public async Task EvaluationUpdate_InvokesEvaluationHandlersWhenChangesExist(boo
o => o.Handle(
workspaceProjectContext.Object,
It.IsAny<ProjectConfiguration>(),
1,
It.IsAny<IComparable>(),
It.IsAny<IProjectChangeDescription>(),
It.IsAny<ContextState>(),
It.IsAny<IManagedProjectDiagnosticOutputService>()));
Expand All @@ -391,7 +391,7 @@ public async Task EvaluationUpdate_InvokesEvaluationHandlersWhenChangesExist(boo
var workspace = await CreateInstanceAsync(
evaluationRuleUpdate: evaluationRuleUpdate,
workspaceProjectContext: workspaceProjectContext.Object,
updateHandlers: new UpdateHandlers(new[] { ExportFactoryFactory.Implement(() => updateHandler.Object) }));
updateHandlers: new UpdateHandlers([ExportFactoryFactory.Implement(() => updateHandler.Object)]));

updateHandler.Verify();
}
Expand Down Expand Up @@ -443,7 +443,7 @@ public async Task EvaluationUpdate_InvokesProjectEvaluationHandlersWhenChangesEx
o => o.Handle(
workspaceProjectContext.Object,
It.IsAny<ProjectConfiguration>(),
1,
It.IsAny<IComparable>(),
It.IsAny<IProjectChangeDescription>(),
new ContextState(false, true),
It.IsAny<IManagedProjectDiagnosticOutputService>()));
Expand All @@ -452,7 +452,7 @@ public async Task EvaluationUpdate_InvokesProjectEvaluationHandlersWhenChangesEx
var workspace = await CreateInstanceAsync(
evaluationRuleUpdate: evaluationRuleUpdate,
workspaceProjectContext: workspaceProjectContext.Object,
updateHandlers: new UpdateHandlers(new[] { ExportFactoryFactory.Implement(() => updateHandler.Object) }));
updateHandlers: new UpdateHandlers([ExportFactoryFactory.Implement(() => updateHandler.Object)]));

updateHandler.Verify();
}
Expand Down Expand Up @@ -497,7 +497,7 @@ public async Task EvaluationUpdate_InvokesSourceItemHandlersWhenChangesExist(boo
var workspace = await CreateInstanceAsync(
sourceItemsUpdate: sourceItemsUpdate,
workspaceProjectContext: workspaceProjectContext.Object,
updateHandlers: new UpdateHandlers(new[] { ExportFactoryFactory.Implement(() => updateHandler.Object) }));
updateHandlers: new UpdateHandlers([ExportFactoryFactory.Implement(() => updateHandler.Object)]));

updateHandler.Verify();
}
Expand Down Expand Up @@ -546,7 +546,7 @@ public async Task BuildUpdate_InvokesCommandLineHandlerWhenChangesExist(bool any
commandLineHandler.Setup(
o => o.Handle(
workspaceProjectContext.Object,
1,
It.IsAny<IComparable>(),
It.Is<BuildOptions>(options => options.MetadataReferences.Select(r => r.Reference).SingleOrDefault() == "Added.dll"),
It.Is<BuildOptions>(options => options.MetadataReferences.Select(r => r.Reference).SingleOrDefault() == "Removed.dll"),
new ContextState(false, true),
Expand All @@ -567,7 +567,7 @@ public async Task BuildUpdate_InvokesCommandLineHandlerWhenChangesExist(bool any
buildRuleUpdate: buildRuleUpdate,
commandLineParserServices: commandLineParserServices,
workspaceProjectContext: workspaceProjectContext.Object,
updateHandlers: new UpdateHandlers(new[] { ExportFactoryFactory.Implement(() => updateHandler.Object) }));
updateHandlers: new UpdateHandlers([ExportFactoryFactory.Implement(() => updateHandler.Object)]));

updateHandler.Verify();
}
Expand Down Expand Up @@ -864,7 +864,8 @@ private static async Task ApplyEvaluationAsync(
ConfiguredProject? configuredProject = null,
IProjectSubscriptionUpdate? evaluationRuleUpdate = null,
IProjectSubscriptionUpdate? sourceItemsUpdate = null,
int configuredProjectVersion = 1)
int configuredProjectVersion = 1,
int activeConfigurationVersion = 1)
{
configuredProject ??= ConfiguredProjectFactory.Create();

Expand Down Expand Up @@ -912,14 +913,20 @@ private static async Task ApplyEvaluationAsync(
var update = WorkspaceUpdate.FromEvaluation((configuredProject, projectSnapshot, evaluationRuleUpdate, sourceItemsUpdate));

await workspace.OnWorkspaceUpdateAsync(
IProjectVersionedValueFactory.Create(update, ProjectDataSources.ConfiguredProjectVersion, configuredProjectVersion));
IProjectVersionedValueFactory.Create(
update,
dataSourceVersions: ImmutableDictionary.CreateRange<NamedIdentity, IComparable>([
new(ProjectDataSources.ConfiguredProjectVersion, configuredProjectVersion),
new(ProjectDataSources.ActiveProjectConfiguration, activeConfigurationVersion)
])));
}

private static async Task ApplyBuildAsync(
Workspace workspace,
ConfiguredProject? configuredProject = null,
IProjectSubscriptionUpdate? buildRuleUpdate = null,
int configuredProjectVersion = 1)
int configuredProjectVersion = 1,
int activeConfigurationVersion = 1)
{
configuredProject ??= ConfiguredProjectFactory.Create();

Expand All @@ -944,6 +951,11 @@ private static async Task ApplyBuildAsync(
var update = WorkspaceUpdate.FromBuild((configuredProject, buildRuleUpdate));

await workspace.OnWorkspaceUpdateAsync(
IProjectVersionedValueFactory.Create(update, ProjectDataSources.ConfiguredProjectVersion, configuredProjectVersion));
IProjectVersionedValueFactory.Create(
update,
dataSourceVersions: ImmutableDictionary.CreateRange<NamedIdentity, IComparable>([
new(ProjectDataSources.ConfiguredProjectVersion, configuredProjectVersion),
new(ProjectDataSources.ActiveProjectConfiguration, activeConfigurationVersion)
])));
}
}

0 comments on commit 5983ea9

Please sign in to comment.