Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Unify code paths that update ordinary/additional/analyzerconfig documents #74441

Merged
merged 1 commit into from
Jul 23, 2024
Merged

Unify code paths that update ordinary/additional/analyzerconfig documents #74441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 46 additions & 27 deletions src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,14 @@ public ProjectState(LanguageServices languageServices, ProjectInfo projectInfo,
_lazyContentHashToDocumentId = AsyncLazy.Create(static (self, cancellationToken) => self.ComputeContentHashToDocumentIdAsync(cancellationToken), arg: this);
}

public TextDocumentStates<TDocumentState> GetDocumentStates<TDocumentState>()
where TDocumentState : TextDocumentState
=> (TextDocumentStates<TDocumentState>)(object)(
typeof(TDocumentState) == typeof(DocumentState) ? DocumentStates :
typeof(TDocumentState) == typeof(AdditionalDocumentState) ? AdditionalDocumentStates :
typeof(TDocumentState) == typeof(AnalyzerConfigDocumentState) ? AnalyzerConfigDocumentStates :
throw ExceptionUtilities.UnexpectedValue(typeof(TDocumentState)));

private async Task<Dictionary<ImmutableArray<byte>, DocumentId>> ComputeContentHashToDocumentIdAsync(CancellationToken cancellationToken)
{
var result = new Dictionary<ImmutableArray<byte>, DocumentId>(ImmutableArrayComparer<byte>.Instance);
Expand Down Expand Up @@ -276,6 +284,14 @@ internal DocumentState CreateDocument(DocumentInfo documentInfo, ParseOptions? p
return doc;
}

internal TDocumentState CreateDocument<TDocumentState>(DocumentInfo documentInfo)
where TDocumentState : TextDocumentState
=> (TDocumentState)(object)(
typeof(TDocumentState) == typeof(DocumentState) ? CreateDocument(documentInfo, ParseOptions, new LoadTextOptions(ChecksumAlgorithm)) :
typeof(TDocumentState) == typeof(AdditionalDocumentState) ? new AdditionalDocumentState(LanguageServices.SolutionServices, documentInfo, new LoadTextOptions(ChecksumAlgorithm)) :
typeof(TDocumentState) == typeof(AnalyzerConfigDocumentState) ? new AnalyzerConfigDocumentState(LanguageServices.SolutionServices, documentInfo, new LoadTextOptions(ChecksumAlgorithm)) :
throw ExceptionUtilities.UnexpectedValue(typeof(TDocumentState)));

public AnalyzerOptions AnalyzerOptions
=> _lazyAnalyzerOptions ??= new AnalyzerOptions(
additionalFiles: AdditionalDocumentStates.SelectAsArray(static documentState => documentState.AdditionalText),
Expand Down Expand Up @@ -828,26 +844,25 @@ public ProjectState RemoveAllNormalDocuments()
}

public ProjectState UpdateDocument(DocumentState newDocument, bool contentChanged)
=> UpdateDocuments([newDocument], contentChanged);
=> UpdateDocuments([DocumentStates.GetRequiredState(newDocument.Id)], [newDocument], contentChanged);

public ProjectState UpdateDocuments(ImmutableArray<DocumentState> newDocuments, bool contentChanged)
public ProjectState UpdateDocuments(ImmutableArray<DocumentState> oldDocuments, ImmutableArray<DocumentState> newDocuments, bool contentChanged)
{
var oldDocuments = newDocuments.SelectAsArray(d => DocumentStates.GetRequiredState(d.Id));
if (oldDocuments.SequenceEqual(newDocuments))
return this;

// Must not be empty as we would have otherwise bailed out in the check above.
Contract.ThrowIfTrue(newDocuments.IsEmpty);
Contract.ThrowIfTrue(oldDocuments.IsEmpty);
Copy link
Contributor

Choose a reason for hiding this comment

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

Contract.ThrowIfTrue(oldDocuments.IsEmpty);

Just wanted to make sure this is a new requirement of the callers

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I follow. The contract has been there before.

Copy link
Contributor

Choose a reason for hiding this comment

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

But it would have returned before hitting the contract failure previously, right?

Contract.ThrowIfFalse(oldDocuments.Length == newDocuments.Length);
Debug.Assert(!oldDocuments.SequenceEqual(newDocuments));

var newDocumentStates = DocumentStates.SetStates(newDocuments);

// When computing the latest dependent version, we just need to know how
GetLatestDependentVersions(
newDocumentStates, AdditionalDocumentStates,
newDocumentStates,
AdditionalDocumentStates,
oldDocuments.CastArray<TextDocumentState>(),
newDocuments.CastArray<TextDocumentState>(),
contentChanged,
out var dependentDocumentVersion, out var dependentSemanticVersion);
out var dependentDocumentVersion,
out var dependentSemanticVersion);

return With(
documentStates: newDocumentStates,
Expand All @@ -856,34 +871,38 @@ public ProjectState UpdateDocuments(ImmutableArray<DocumentState> newDocuments,
}

public ProjectState UpdateAdditionalDocument(AdditionalDocumentState newDocument, bool contentChanged)
=> UpdateAdditionalDocuments([AdditionalDocumentStates.GetRequiredState(newDocument.Id)], [newDocument], contentChanged);

public ProjectState UpdateAdditionalDocuments(ImmutableArray<AdditionalDocumentState> oldDocuments, ImmutableArray<AdditionalDocumentState> newDocuments, bool contentChanged)
Copy link
Member Author

Choose a reason for hiding this comment

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

Follow the same pattern as UpdateDocuments

{
var oldDocument = AdditionalDocumentStates.GetRequiredState(newDocument.Id);
if (oldDocument == newDocument)
{
return this;
}
Contract.ThrowIfTrue(oldDocuments.IsEmpty);
Contract.ThrowIfFalse(oldDocuments.Length == newDocuments.Length);
Debug.Assert(!oldDocuments.SequenceEqual(newDocuments));

var newDocumentStates = AdditionalDocumentStates.SetStates(newDocuments);

var newDocumentStates = AdditionalDocumentStates.SetState(newDocument);
GetLatestDependentVersions(
DocumentStates, newDocumentStates, [oldDocument], [newDocument], contentChanged,
out var dependentDocumentVersion, out var dependentSemanticVersion);
DocumentStates,
newDocumentStates,
oldDocuments.CastArray<TextDocumentState>(),
newDocuments.CastArray<TextDocumentState>(),
contentChanged,
out var dependentDocumentVersion,
out var dependentSemanticVersion);

return this.With(
return With(
additionalDocumentStates: newDocumentStates,
latestDocumentVersion: dependentDocumentVersion,
latestDocumentTopLevelChangeVersion: dependentSemanticVersion);
}

public ProjectState UpdateAnalyzerConfigDocument(AnalyzerConfigDocumentState newDocument)
{
var oldDocument = AnalyzerConfigDocumentStates.GetRequiredState(newDocument.Id);
if (oldDocument == newDocument)
{
return this;
}

var newDocumentStates = AnalyzerConfigDocumentStates.SetState(newDocument);
=> UpdateAnalyzerConfigDocuments([AnalyzerConfigDocumentStates.GetRequiredState(newDocument.Id)], [newDocument]);

public ProjectState UpdateAnalyzerConfigDocuments(ImmutableArray<AnalyzerConfigDocumentState> oldDocuments, ImmutableArray<AnalyzerConfigDocumentState> newDocuments)
{
Debug.Assert(!oldDocuments.SequenceEqual(newDocuments));
tmat marked this conversation as resolved.
Show resolved Hide resolved
var newDocumentStates = AnalyzerConfigDocumentStates.SetStates(newDocuments);
return CreateNewStateForChangedAnalyzerConfig(newDocumentStates, _analyzerConfigOptionsCache.FallbackOptions);
}

Expand Down
12 changes: 6 additions & 6 deletions src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,7 @@ public Solution AddDocument(DocumentInfo documentInfo)
/// </summary>
/// <returns>A new <see cref="Solution"/> with the documents added.</returns>
public Solution AddDocuments(ImmutableArray<DocumentInfo> documentInfos)
=> WithCompilationState(_compilationState.AddDocuments(documentInfos));
=> WithCompilationState(_compilationState.AddDocumentsToMultipleProjects<DocumentState>(documentInfos));

/// <summary>
/// Creates a new solution instance with the corresponding project updated to include a new
Expand Down Expand Up @@ -1021,7 +1021,7 @@ public Solution AddAdditionalDocument(DocumentInfo documentInfo)
=> AddAdditionalDocuments([documentInfo]);

public Solution AddAdditionalDocuments(ImmutableArray<DocumentInfo> documentInfos)
=> WithCompilationState(_compilationState.AddAdditionalDocuments(documentInfos));
=> WithCompilationState(_compilationState.AddDocumentsToMultipleProjects<AdditionalDocumentState>(documentInfos));

/// <summary>
/// Creates a new solution instance with the corresponding project updated to include a new
Expand Down Expand Up @@ -1073,7 +1073,7 @@ private ProjectState GetRequiredProjectState(ProjectId projectId)
/// Creates a new Solution instance that contains a new compiler configuration document like a .editorconfig file.
/// </summary>
public Solution AddAnalyzerConfigDocuments(ImmutableArray<DocumentInfo> documentInfos)
=> WithCompilationState(_compilationState.AddAnalyzerConfigDocuments(documentInfos));
=> WithCompilationState(_compilationState.AddDocumentsToMultipleProjects<AnalyzerConfigDocumentState>(documentInfos));

/// <summary>
/// Creates a new solution instance that no longer includes the specified document.
Expand All @@ -1094,7 +1094,7 @@ public Solution RemoveDocuments(ImmutableArray<DocumentId> documentIds)
}

private Solution RemoveDocumentsImpl(ImmutableArray<DocumentId> documentIds)
=> WithCompilationState(_compilationState.RemoveDocuments(documentIds));
=> WithCompilationState(_compilationState.RemoveDocumentsFromMultipleProjects<DocumentState>(documentIds));

/// <summary>
/// Creates a new solution instance that no longer includes the specified additional document.
Expand All @@ -1115,7 +1115,7 @@ public Solution RemoveAdditionalDocuments(ImmutableArray<DocumentId> documentIds
}

private Solution RemoveAdditionalDocumentsImpl(ImmutableArray<DocumentId> documentIds)
=> WithCompilationState(_compilationState.RemoveAdditionalDocuments(documentIds));
=> WithCompilationState(_compilationState.RemoveDocumentsFromMultipleProjects<AdditionalDocumentState>(documentIds));

/// <summary>
/// Creates a new solution instance that no longer includes the specified <see cref="AnalyzerConfigDocument"/>.
Expand All @@ -1136,7 +1136,7 @@ public Solution RemoveAnalyzerConfigDocuments(ImmutableArray<DocumentId> documen
}

private Solution RemoveAnalyzerConfigDocumentsImpl(ImmutableArray<DocumentId> documentIds)
=> WithCompilationState(_compilationState.RemoveAnalyzerConfigDocuments(documentIds));
=> WithCompilationState(_compilationState.RemoveDocumentsFromMultipleProjects<AnalyzerConfigDocumentState>(documentIds));

/// <summary>
/// Creates a new solution instance with the document specified updated to have the new name.
Expand Down
69 changes: 43 additions & 26 deletions ...es/Core/Portable/Workspace/Solution/SolutionCompilationState.TranslationAction_Actions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,16 @@ private abstract partial class TranslationAction
internal sealed class TouchDocumentsAction(
ProjectState oldProjectState,
ProjectState newProjectState,
ImmutableArray<DocumentState> oldStates,
ImmutableArray<DocumentState> newStates) : TranslationAction(oldProjectState, newProjectState)
{
private readonly ImmutableArray<DocumentState> _oldStates = newStates.SelectAsArray(s => oldProjectState.DocumentStates.GetRequiredState(s.Id));
private readonly ImmutableArray<DocumentState> _oldStates = oldStates;
private readonly ImmutableArray<DocumentState> _newStates = newStates;

public override async Task<Compilation> TransformCompilationAsync(Compilation oldCompilation, CancellationToken cancellationToken)
{
var finalCompilation = oldCompilation;
for (int i = 0, n = _newStates.Length; i < n; i++)
for (var i = 0; i < _newStates.Length; i++)
{
cancellationToken.ThrowIfCancellationRequested();
var newState = _newStates[i];
Expand Down Expand Up @@ -57,22 +58,22 @@ public override GeneratorDriver TransformGeneratorDriver(GeneratorDriver generat
{
// As we're merging ourselves with the prior touch action, we want to keep the old project state
// that we are translating from.
return new TouchDocumentsAction(priorAction.OldProjectState, this.NewProjectState, _newStates);
return new TouchDocumentsAction(priorAction.OldProjectState, NewProjectState, priorTouchAction._oldStates, _newStates);
}

return null;
}
}

internal sealed class TouchAdditionalDocumentAction(
internal sealed class TouchAdditionalDocumentsAction(
Copy link
Member Author

Choose a reason for hiding this comment

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

Allow multiple documents to be changed at the same time, like TouchDocumentsAction.

ProjectState oldProjectState,
ProjectState newProjectState,
AdditionalDocumentState oldState,
AdditionalDocumentState newState)
ImmutableArray<AdditionalDocumentState> oldStates,
ImmutableArray<AdditionalDocumentState> newStates)
: TranslationAction(oldProjectState, newProjectState)
{
private readonly AdditionalDocumentState _oldState = oldState;
private readonly AdditionalDocumentState _newState = newState;
private readonly ImmutableArray<AdditionalDocumentState> _oldStates = oldStates;
private readonly ImmutableArray<AdditionalDocumentState> _newStates = newStates;

// Changing an additional document doesn't change the compilation directly, so we can "apply" the
// translation (which is a no-op). Since we use a 'false' here to mean that it's not worth keeping the
Expand All @@ -84,24 +85,48 @@ public override Task<Compilation> TransformCompilationAsync(Compilation oldCompi

public override TranslationAction? TryMergeWithPrior(TranslationAction priorAction)
{
if (priorAction is TouchAdditionalDocumentAction priorTouchAction &&
priorTouchAction._newState == _oldState)
if (priorAction is TouchAdditionalDocumentsAction priorTouchAction &&
priorTouchAction._newStates.SequenceEqual(_oldStates))
{
// As we're merging ourselves with the prior touch action, we want to keep the old project state
// that we are translating from.
return new TouchAdditionalDocumentAction(priorAction.OldProjectState, this.NewProjectState, priorTouchAction._oldState, _newState);
return new TouchAdditionalDocumentsAction(priorAction.OldProjectState, NewProjectState, priorTouchAction._oldStates, _newStates);
}

return null;
}

public override GeneratorDriver TransformGeneratorDriver(GeneratorDriver generatorDriver)
{
var oldText = _oldState.AdditionalText;
var newText = _newState.AdditionalText;
for (var i = 0; i < _newStates.Length; i++)
{
generatorDriver = generatorDriver.ReplaceAdditionalText(_oldStates[i].AdditionalText, _newStates[i].AdditionalText);
}

return generatorDriver;
}
}

return generatorDriver.ReplaceAdditionalText(oldText, newText);
internal sealed class TouchAnalyzerConfigDocumentsAction(
Copy link
Member Author

Choose a reason for hiding this comment

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

Separate this action from ProjectCompilationOptionsAction.

ProjectState oldProjectState,
ProjectState newProjectState)
: TranslationAction(oldProjectState, newProjectState)
{
/// <summary>
/// Updating editorconfig document updates <see cref="CompilationOptions.SyntaxTreeOptionsProvider"/>.
/// </summary>
public override Task<Compilation> TransformCompilationAsync(Compilation oldCompilation, CancellationToken cancellationToken)
{
RoslynDebug.AssertNotNull(this.NewProjectState.CompilationOptions);
return Task.FromResult(oldCompilation.WithOptions(this.NewProjectState.CompilationOptions));
}

// Updating the analyzer config optons doesn't require us to reparse trees, so we can use this to update
// compilations with stale generated trees.
public override bool CanUpdateCompilationWithStaleGeneratedTreesIfGeneratorsGiveSameOutput => true;

public override GeneratorDriver TransformGeneratorDriver(GeneratorDriver generatorDriver)
=> generatorDriver.WithUpdatedAnalyzerConfigOptions(NewProjectState.AnalyzerOptions.AnalyzerConfigOptionsProvider);
}

internal sealed class RemoveDocumentsAction(
Expand Down Expand Up @@ -210,8 +235,7 @@ public override GeneratorDriver TransformGeneratorDriver(GeneratorDriver generat

internal sealed class ProjectCompilationOptionsAction(
ProjectState oldProjectState,
ProjectState newProjectState,
bool isAnalyzerConfigChange)
ProjectState newProjectState)
: TranslationAction(oldProjectState, newProjectState)
{
public override Task<Compilation> TransformCompilationAsync(Compilation oldCompilation, CancellationToken cancellationToken)
Expand All @@ -226,16 +250,9 @@ public override Task<Compilation> TransformCompilationAsync(Compilation oldCompi

public override GeneratorDriver TransformGeneratorDriver(GeneratorDriver generatorDriver)
{
if (isAnalyzerConfigChange)
{
return generatorDriver.WithUpdatedAnalyzerConfigOptions(this.NewProjectState.AnalyzerOptions.AnalyzerConfigOptionsProvider);
}
else
{
// Changing any other option is fine and the driver can be reused. The driver
// will get the new compilation once we pass it to it.
return generatorDriver;
}
// Changing any other option is fine and the driver can be reused. The driver
// will get the new compilation once we pass it to it.
return generatorDriver;
}
}

Expand Down
Loading
Loading