Skip to content

Commit

Permalink
Fix SourceCodeKind setting (#74294)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tmat
tmat committed Jul 17, 2024
1 parent a26c728 commit c556b4a
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 61 deletions.
101 changes: 43 additions & 58 deletions src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ internal partial class DocumentState : TextDocumentState

// properties inherited from the containing project:
public LanguageServices LanguageServices { get; }
private readonly ParseOptions? _options;
public ParseOptions? ParseOptions { get; }

// null if the document doesn't support syntax trees:
private readonly ITreeAndVersionSource? _treeSource;
public ITreeAndVersionSource? TreeSource { get; }

private ImmutableArray<byte> _contentHash;

Expand All @@ -46,11 +46,11 @@ protected DocumentState(
ITreeAndVersionSource? treeSource)
: base(languageServices.SolutionServices, documentServiceProvider, attributes, textSource, loadTextOptions)
{
Contract.ThrowIfFalse(_options is null == _treeSource is null);
Contract.ThrowIfFalse(ParseOptions is null == TreeSource is null);

LanguageServices = languageServices;
_options = options;
_treeSource = treeSource;
ParseOptions = options;
TreeSource = treeSource;
}

public DocumentState(
Expand All @@ -61,19 +61,19 @@ public DocumentState(
: base(languageServices.SolutionServices, info, loadTextOptions)
{
LanguageServices = languageServices;
_options = options;
ParseOptions = options;

// If this is document that doesn't support syntax, then don't even bother holding
// onto any tree source. It will never be used to get a tree, and can only hurt us
// by possibly holding onto data that might cause a slow memory leak.
if (languageServices.GetService<ISyntaxTreeFactoryService>() == null)
{
_treeSource = null;
TreeSource = null;
}
else
{
Contract.ThrowIfNull(options);
_treeSource = CreateLazyFullyParsedTree(
TreeSource = CreateLazyFullyParsedTree(
TextAndVersionSource,
LoadTextOptions,
info.Attributes.SyntaxTreeFilePath,
Expand All @@ -82,20 +82,13 @@ public DocumentState(
}
}

public ITreeAndVersionSource? TreeSource => _treeSource;

[MemberNotNullWhen(true, nameof(_treeSource))]
[MemberNotNullWhen(true, nameof(TreeSource))]
[MemberNotNullWhen(true, nameof(_options))]
[MemberNotNullWhen(true, nameof(ParseOptions))]
internal bool SupportsSyntaxTree
=> _treeSource != null;

public ParseOptions? ParseOptions
=> _options;
=> TreeSource != null;

public SourceCodeKind SourceCodeKind
=> ParseOptions == null ? Attributes.SourceCodeKind : ParseOptions.Kind;
=> ParseOptions?.Kind ?? Attributes.SourceCodeKind;

public bool IsGenerated
=> Attributes.IsGenerated;
Expand Down Expand Up @@ -293,7 +286,7 @@ private static bool TopLevelChanged(SyntaxTree oldTree, SourceText oldText, Synt

public bool HasContentChanged(DocumentState oldState)
{
return oldState._treeSource != _treeSource
return oldState.TreeSource != TreeSource
|| HasTextChanged(oldState, ignoreUnchangeableDocument: false);
}

Expand Down Expand Up @@ -322,33 +315,20 @@ public DocumentState UpdateChecksumAlgorithm(SourceHashAlgorithm checksumAlgorit
TextAndVersionSource,
newLoadTextOptions,
Attributes.SyntaxTreeFilePath,
_options,
ParseOptions,
LanguageServices) : null;

return new DocumentState(
LanguageServices,
Services,
Attributes,
_options,
ParseOptions,
TextAndVersionSource,
newLoadTextOptions,
newTreeSource);
}

public DocumentState UpdateParseOptions(ParseOptions options, bool onlyPreprocessorDirectiveChange)
{
var originalSourceKind = this.SourceCodeKind;

var newState = this.SetParseOptions(options, onlyPreprocessorDirectiveChange);
if (newState.SourceCodeKind != originalSourceKind)
{
newState = newState.UpdateSourceCodeKind(originalSourceKind);
}

return newState;
}

private DocumentState SetParseOptions(ParseOptions options, bool onlyPreprocessorDirectiveChange)
public DocumentState UpdateParseOptionsAndSourceCodeKind(ParseOptions options, bool onlyPreprocessorDirectiveChange)
{
Contract.ThrowIfFalse(SupportsSyntaxTree);

Expand All @@ -360,7 +340,7 @@ private DocumentState SetParseOptions(ParseOptions options, bool onlyPreprocesso
// We only need to care about `#if` directives as those are the only sorts of directives that can affect how
// a tree is parsed.
if (onlyPreprocessorDirectiveChange &&
_treeSource.TryGetValue(out var existingTreeAndVersion))
TreeSource.TryGetValue(out var existingTreeAndVersion))
{
var existingTree = existingTreeAndVersion.Tree;

Expand Down Expand Up @@ -397,12 +377,17 @@ private DocumentState SetParseOptions(ParseOptions options, bool onlyPreprocesso

public DocumentState UpdateSourceCodeKind(SourceCodeKind kind)
{
if (this.ParseOptions == null || kind == this.SourceCodeKind)
if (kind == SourceCodeKind)
{
return this;
}

return this.SetParseOptions(this.ParseOptions.WithKind(kind), onlyPreprocessorDirectiveChange: false);
if (ParseOptions != null)
{
return UpdateParseOptionsAndSourceCodeKind(ParseOptions.WithKind(kind), onlyPreprocessorDirectiveChange: false);
}

return UpdateAttributes(Attributes.With(sourceCodeKind: kind));
}

public DocumentState UpdateName(string name)
Expand All @@ -429,19 +414,19 @@ private DocumentState UpdateAttributes(DocumentInfo.DocumentAttributes newAttrib
TextAndVersionSource,
LoadTextOptions,
newAttributes.SyntaxTreeFilePath,
_options,
ParseOptions,
LanguageServices) : null;
}
else
{
newTreeSource = _treeSource;
newTreeSource = TreeSource;
}

return new DocumentState(
LanguageServices,
Services,
newAttributes,
_options,
ParseOptions,
TextAndVersionSource,
LoadTextOptions,
newTreeSource);
Expand All @@ -466,15 +451,15 @@ protected override TextDocumentState UpdateText(ITextAndVersionSource newTextSou
}
else if (incremental)
{
newTreeSource = CreateLazyIncrementallyParsedTree(_treeSource, newTextSource, LoadTextOptions);
newTreeSource = CreateLazyIncrementallyParsedTree(TreeSource, newTextSource, LoadTextOptions);
}
else
{
newTreeSource = CreateLazyFullyParsedTree(
newTextSource,
LoadTextOptions,
Attributes.SyntaxTreeFilePath,
_options,
ParseOptions,
LanguageServices,
mode); // TODO: understand why the mode is given here. If we're preserving text by identity, why also preserve the tree?
}
Expand All @@ -483,7 +468,7 @@ protected override TextDocumentState UpdateText(ITextAndVersionSource newTextSou
LanguageServices,
Services,
Attributes,
_options,
ParseOptions,
textSource: newTextSource,
LoadTextOptions,
treeSource: newTreeSource);
Expand Down Expand Up @@ -519,14 +504,14 @@ internal DocumentState UpdateTree(SyntaxNode newRoot, PreservationMode mode)

var syntaxTreeFactory = LanguageServices.GetRequiredService<ISyntaxTreeFactoryService>();

Contract.ThrowIfNull(_options);
var (text, treeAndVersion) = CreateTreeWithLazyText(newRoot, newTextVersion, newTreeVersion, encoding, LoadTextOptions.ChecksumAlgorithm, Attributes, _options, syntaxTreeFactory);
Contract.ThrowIfNull(ParseOptions);
var (text, treeAndVersion) = CreateTreeWithLazyText(newRoot, newTextVersion, newTreeVersion, encoding, LoadTextOptions.ChecksumAlgorithm, Attributes, ParseOptions, syntaxTreeFactory);

return new DocumentState(
LanguageServices,
Services,
Attributes,
_options,
ParseOptions,
textSource: text,
LoadTextOptions,
treeSource: SimpleTreeAndVersionSource.Create(treeAndVersion));
Expand Down Expand Up @@ -558,14 +543,14 @@ internal DocumentState UpdateTree(SyntaxNode newRoot, PreservationMode mode)

private VersionStamp GetNewTreeVersionForUpdatedTree(SyntaxNode newRoot, VersionStamp newTextVersion, PreservationMode mode)
{
RoslynDebug.Assert(_treeSource != null);
RoslynDebug.Assert(TreeSource != null);

if (mode != PreservationMode.PreserveIdentity)
{
return newTextVersion;
}

if (!_treeSource.TryGetValue(out var oldTreeAndVersion) || !oldTreeAndVersion!.Tree.TryGetRoot(out var oldRoot))
if (!TreeSource.TryGetValue(out var oldTreeAndVersion) || !oldTreeAndVersion!.Tree.TryGetRoot(out var oldRoot))
{
return newTextVersion;
}
Expand All @@ -590,7 +575,7 @@ private VersionStamp GetNewerVersion()
return textAndVersion!.Version.GetNewerVersion();
}

if (_treeSource != null && _treeSource.TryGetValue(out var treeAndVersion) && treeAndVersion != null)
if (TreeSource != null && TreeSource.TryGetValue(out var treeAndVersion) && treeAndVersion != null)
{
return treeAndVersion.Version.GetNewerVersion();
}
Expand All @@ -601,7 +586,7 @@ private VersionStamp GetNewerVersion()
public bool TryGetSyntaxTree([NotNullWhen(returnValue: true)] out SyntaxTree? syntaxTree)
{
syntaxTree = null;
if (_treeSource != null && _treeSource.TryGetValue(out var treeAndVersion) && treeAndVersion != null)
if (TreeSource != null && TreeSource.TryGetValue(out var treeAndVersion) && treeAndVersion != null)
{
syntaxTree = treeAndVersion.Tree;
BindSyntaxTreeToId(syntaxTree, Id);
Expand All @@ -615,9 +600,9 @@ public bool TryGetSyntaxTree([NotNullWhen(returnValue: true)] out SyntaxTree? sy
public async ValueTask<SyntaxTree> GetSyntaxTreeAsync(CancellationToken cancellationToken)
{
// operation should only be performed on documents that support syntax trees
RoslynDebug.Assert(_treeSource != null);
RoslynDebug.Assert(TreeSource != null);

var treeAndVersion = await _treeSource.GetValueAsync(cancellationToken).ConfigureAwait(false);
var treeAndVersion = await TreeSource.GetValueAsync(cancellationToken).ConfigureAwait(false);

// make sure there is an association between this tree and this doc id before handing it out
BindSyntaxTreeToId(treeAndVersion.Tree, this.Id);
Expand All @@ -627,9 +612,9 @@ public async ValueTask<SyntaxTree> GetSyntaxTreeAsync(CancellationToken cancella
internal SyntaxTree GetSyntaxTree(CancellationToken cancellationToken)
{
// operation should only be performed on documents that support syntax trees
RoslynDebug.Assert(_treeSource != null);
RoslynDebug.Assert(TreeSource != null);

var treeAndVersion = _treeSource.GetValue(cancellationToken);
var treeAndVersion = TreeSource.GetValue(cancellationToken);

// make sure there is an association between this tree and this doc id before handing it out
BindSyntaxTreeToId(treeAndVersion.Tree, this.Id);
Expand All @@ -638,7 +623,7 @@ internal SyntaxTree GetSyntaxTree(CancellationToken cancellationToken)

public bool TryGetTopLevelChangeTextVersion(out VersionStamp version)
{
if (_treeSource != null && _treeSource.TryGetValue(out var treeAndVersion) && treeAndVersion != null)
if (TreeSource != null && TreeSource.TryGetValue(out var treeAndVersion) && treeAndVersion != null)
{
version = treeAndVersion.Version;
return true;
Expand All @@ -652,17 +637,17 @@ public bool TryGetTopLevelChangeTextVersion(out VersionStamp version)

public override async ValueTask<VersionStamp> GetTopLevelChangeTextVersionAsync(CancellationToken cancellationToken)
{
if (_treeSource == null)
if (TreeSource == null)
{
return await GetTextVersionAsync(cancellationToken).ConfigureAwait(false);
}

if (_treeSource.TryGetValue(out var treeAndVersion) && treeAndVersion != null)
if (TreeSource.TryGetValue(out var treeAndVersion) && treeAndVersion != null)
{
return treeAndVersion.Version;
}

treeAndVersion = await _treeSource.GetValueAsync(cancellationToken).ConfigureAwait(false);
treeAndVersion = await TreeSource.GetValueAsync(cancellationToken).ConfigureAwait(false);
return treeAndVersion.Version;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Workspaces/Core/Portable/Workspace/Solution/DocumentState_LinkedFileReuse.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public DocumentState UpdateTextAndTreeContents(
LanguageServices,
Services,
Attributes,
_options,
ParseOptions,
siblingTextSource,
LoadTextOptions,
treeSource: null);
Expand Down
2 changes: 1 addition & 1 deletion src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ public ProjectState WithParseOptions(ParseOptions options)

return With(
projectInfo: ProjectInfo.WithParseOptions(options).WithVersion(Version.GetNewerVersion()),
documentStates: DocumentStates.UpdateStates(static (state, args) => state.UpdateParseOptions(args.options, args.onlyPreprocessorDirectiveChange), (options, onlyPreprocessorDirectiveChange)));
documentStates: DocumentStates.UpdateStates(static (state, args) => state.UpdateParseOptionsAndSourceCodeKind(args.options, args.onlyPreprocessorDirectiveChange), (options, onlyPreprocessorDirectiveChange)));
}

public ProjectState WithFallbackAnalyzerOptions(StructuredAnalyzerConfigOptions options)
Expand Down
42 changes: 41 additions & 1 deletion src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,34 @@ public void WithSourceCodeKind()
Assert.Same(solution, solution.WithDocumentSourceCodeKind(documentId, SourceCodeKind.Regular));

var newSolution1 = solution.WithDocumentSourceCodeKind(documentId, SourceCodeKind.Script);
Assert.Equal(SourceCodeKind.Script, newSolution1.GetDocument(documentId)!.SourceCodeKind);
Assert.Equal(SourceCodeKind.Script, newSolution1.GetRequiredDocument(documentId).SourceCodeKind);

Assert.Throws<ArgumentOutOfRangeException>(() => solution.WithDocumentSourceCodeKind(documentId, (SourceCodeKind)(-1)));

Assert.Throws<ArgumentNullException>(() => solution.WithDocumentSourceCodeKind(null!, SourceCodeKind.Script));
Assert.Throws<InvalidOperationException>(() => solution.WithDocumentSourceCodeKind(s_unrelatedDocumentId, SourceCodeKind.Script));
}

[Fact]
public void WithSourceCodeKind_ParseOptions()
{
using var workspace = CreateWorkspaceWithProjectAndDocuments();

var solution = workspace.CurrentSolution;
var documentId = solution.Projects.Single().DocumentIds.Single();
var projectId = documentId.ProjectId;

solution = solution.WithProjectParseOptions(projectId, CSharpParseOptions.Default.WithKind(SourceCodeKind.Script));

var document1 = solution.GetRequiredDocument(documentId);
Assert.Equal(SourceCodeKind.Script, document1.DocumentState.ParseOptions?.Kind);
Assert.Equal(SourceCodeKind.Script, document1.SourceCodeKind);

var document2 = document1.WithSourceCodeKind(SourceCodeKind.Regular);
Assert.Equal(SourceCodeKind.Regular, document2.DocumentState.ParseOptions?.Kind);
Assert.Equal(SourceCodeKind.Regular, document2.SourceCodeKind);
}

[Fact, Obsolete("Testing obsolete API")]
public void WithSourceCodeKind_Obsolete()
{
Expand Down Expand Up @@ -4367,6 +4387,26 @@ public void NoCompilationProjectsHaveNullSyntaxTreesAndSemanticModels()
Assert.Null(document.GetSemanticModelAsync().Result);
}

[Fact]
public void NoCompilation_SourceCodeKind()
{
using var workspace = CreateWorkspace([typeof(NoCompilationLanguageService)]);
var projectId = ProjectId.CreateNewId();
var documentId = DocumentId.CreateNewId(projectId);

var solution = workspace.CurrentSolution
.AddProject(projectId, "Test", "Test.dll", NoCompilationConstants.LanguageName)
.AddDocument(DocumentInfo.Create(documentId, "Test", sourceCodeKind: SourceCodeKind.Script));

var document1 = solution.GetRequiredDocument(documentId);
Assert.Equal(SourceCodeKind.Script, document1.SourceCodeKind);
Assert.Null(document1.DocumentState.ParseOptions);

var document2 = document1.WithSourceCodeKind(SourceCodeKind.Regular);
Assert.Equal(SourceCodeKind.Regular, document2.SourceCodeKind);
Assert.Null(document2.DocumentState.ParseOptions);
}

[Fact]
public void ChangingFilePathOfFileInNoCompilationProjectWorks()
{
Expand Down

0 comments on commit c556b4a

Please sign in to comment.