Skip to content

Commit

Permalink
Merge pull request #18695 from jasonmalinowski/fix-codemodel-crash-if…
Browse files Browse the repository at this point in the history 
…-file-is-closed

Fix CodeModel crash if the file is closed
  • Loading branch information
@jasonmalinowski
jasonmalinowski authored Apr 24, 2017
2 parents 8432de2 + 208da19 commit 6969b51
Show file tree
Hide file tree
Showing 14 changed files with 132 additions and 157 deletions.
8 changes: 4 additions & 4 deletions src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ class D { }

await VerifyRootTypeNameAsync(workspace, "C");

workspace.OnDocumentClosed(document.Id);
workspace.CloseDocument(document.Id);
}
}

Expand Down Expand Up @@ -405,7 +405,7 @@ public void TestRemoveProjectWithClosedDocuments()

workspace.AddTestProject(project1);
workspace.OnDocumentOpened(document.Id, document.GetOpenTextContainer());
workspace.OnDocumentClosed(document.Id);
workspace.CloseDocument(document.Id);
workspace.OnProjectRemoved(project1.Id);
}
}
Expand All @@ -425,7 +425,7 @@ public void TestRemoveOpenedDocument()

Assert.Throws<ArgumentException>(() => workspace.OnDocumentRemoved(document.Id));

workspace.OnDocumentClosed(document.Id);
workspace.CloseDocument(document.Id);
workspace.OnProjectRemoved(project1.Id);
}
}
Expand Down Expand Up @@ -692,7 +692,7 @@ public async Task TestOpenAndChangeDocument()
var syntaxTree = await doc.GetSyntaxTreeAsync(CancellationToken.None);
Assert.True(syntaxTree.GetRoot().Width() > 0, "syntaxTree.GetRoot().Width should be > 0");

workspace.OnDocumentClosed(document.Id);
workspace.CloseDocument(document.Id);
workspace.OnProjectRemoved(project1.Id);
}
}
Expand Down
43 changes: 13 additions & 30 deletions src/EditorFeatures/TestUtilities/Workspaces/TestHostDocument.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
Expand All @@ -20,7 +21,7 @@ public class TestHostDocument
{
private readonly ExportProvider _exportProvider;
private HostLanguageServices _languageServiceProvider;
private readonly string _initialText;
private readonly Lazy<string> _initialText;
private IWpfTextView _textView;

private DocumentId _id;
Expand All @@ -32,7 +33,6 @@ public class TestHostDocument
private readonly SourceCodeKind _sourceCodeKind;
private readonly string _filePath;
private readonly IReadOnlyList<string> _folders;
private readonly TextLoader _loader;

public DocumentId Id
{
Expand Down Expand Up @@ -82,14 +82,7 @@ public bool IsGenerated
}
}

public TextLoader Loader
{
get
{
return _loader;
}
}

public TextLoader Loader { get; }
public int? CursorPosition { get; }
public IList<TextSpan> SelectedSpans { get; }
public IDictionary<string, IList<TextSpan>> AnnotatedSpans { get; }
Expand Down Expand Up @@ -118,6 +111,7 @@ internal TestHostDocument(
_languageServiceProvider = languageServiceProvider;
this.TextBuffer = textBuffer;
this.InitialTextSnapshot = textBuffer.CurrentSnapshot;
_initialText = new Lazy<string>(() => this.InitialTextSnapshot.GetText());
_filePath = filePath;
_folders = folders;
_name = filePath;
Expand All @@ -137,7 +131,7 @@ internal TestHostDocument(
this.AnnotatedSpans.Add(namedSpanList);
}

_loader = new TestDocumentLoader(this);
Loader = new TestDocumentLoader(this);
}

public TestHostDocument(
Expand All @@ -148,10 +142,10 @@ public TestHostDocument(
{
_exportProvider = TestExportProvider.ExportProviderWithCSharpAndVisualBasic;
_id = id;
_initialText = text;
_initialText = new Lazy<string>(() => text);
_name = displayName;
_sourceCodeKind = sourceCodeKind;
_loader = new TestDocumentLoader(this);
Loader = new TestDocumentLoader(this);
_filePath = filePath;
_folders = folders;
}
Expand All @@ -178,7 +172,7 @@ internal void SetProject(TestHostProject project)
{
var contentTypeService = _languageServiceProvider.GetService<IContentTypeLanguageService>();
var contentType = contentTypeService.GetDefaultContentType();
this.TextBuffer = _exportProvider.GetExportedValue<ITextBufferFactoryService>().CreateTextBuffer(_initialText, contentType);
this.TextBuffer = _exportProvider.GetExportedValue<ITextBufferFactoryService>().CreateTextBuffer(_initialText.Value, contentType);
this.InitialTextSnapshot = this.TextBuffer.CurrentSnapshot;
}
}
Expand All @@ -194,18 +188,13 @@ internal TestDocumentLoader(TestHostDocument hostDocument)

public override Task<TextAndVersion> LoadTextAndVersionAsync(Workspace workspace, DocumentId documentId, CancellationToken cancellationToken)
{
return Task.FromResult(TextAndVersion.Create(_hostDocument.LoadText(cancellationToken), VersionStamp.Create(), "test"));
}
}

public IContentType ContentType
{
get
{
return this.TextBuffer.ContentType;
// Create a simple SourceText so that way we're not backing "closed" files by editors to best reflect
// what closed files look like in reality.
var text = SourceText.From(_hostDocument.GetTextBuffer().CurrentSnapshot.GetText());
return Task.FromResult(TextAndVersion.Create(text, VersionStamp.Create(), _hostDocument.FilePath));
}
}

public IWpfTextView GetTextView()
{
if (_textView == null)
Expand Down Expand Up @@ -234,12 +223,6 @@ public ITextBuffer GetTextBuffer()
return this.TextBuffer;
}

public SourceText LoadText(CancellationToken cancellationToken = default(CancellationToken))
{
var loadedBuffer = _exportProvider.GetExportedValue<ITextBufferFactoryService>().CreateTextBuffer(this.InitialTextSnapshot.GetText(), this.InitialTextSnapshot.ContentType);
return loadedBuffer.CurrentSnapshot.AsText();
}

public SourceTextContainer GetOpenTextContainer()
{
return this.GetTextBuffer().AsTextContainer();
Expand Down
17 changes: 5 additions & 12 deletions src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,7 @@ public void AddTestProject(TestHostProject project)
{
base.OnDocumentOpened(documentId, textContainer, isCurrentContext);
}

public void OnDocumentClosed(DocumentId documentId)
{
var testDocument = this.GetTestDocument(documentId);
this.OnDocumentClosed(documentId, testDocument.Loader);
}


public new void OnParseOptionsChanged(ProjectId projectId, ParseOptions parseOptions)
{
base.OnParseOptionsChanged(projectId, parseOptions);
Expand Down Expand Up @@ -594,15 +588,14 @@ private void MapMarkupSpans(IDictionary<string, IList<TextSpan>> markupSpans, ou

public override void OpenDocument(DocumentId documentId, bool activate = true)
{
OnDocumentOpened(documentId, this.CurrentSolution.GetDocument(documentId).GetTextAsync().Result.Container);
var testDocument = this.GetTestDocument(documentId);
OnDocumentOpened(documentId, testDocument.GetOpenTextContainer());
}

public override void CloseDocument(DocumentId documentId)
{
var currentDoc = this.CurrentSolution.GetDocument(documentId);

OnDocumentClosed(documentId,
TextLoader.From(TextAndVersion.Create(currentDoc.GetTextAsync().Result, currentDoc.GetTextVersionAsync().Result)));
var testDocument = this.GetTestDocument(documentId);
this.OnDocumentClosed(documentId, testDocument.Loader);
}

public void ChangeDocument(DocumentId documentId, SourceText text)
Expand Down
51 changes: 23 additions & 28 deletions src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelService.NodeLocator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.LanguageServices.Implementation.Utilities;
Expand All @@ -18,22 +19,16 @@ internal partial class CSharpCodeModelService
{
protected override AbstractNodeLocator CreateNodeLocator()
{
return new NodeLocator(this);
return new NodeLocator();
}

private class NodeLocator : AbstractNodeLocator
{
public NodeLocator(CSharpCodeModelService codeModelService)
: base(codeModelService)
{
}
protected override string LanguageName => LanguageNames.CSharp;

protected override EnvDTE.vsCMPart DefaultPart
{
get { return EnvDTE.vsCMPart.vsCMPartWholeWithAttributes; }
}
protected override EnvDTE.vsCMPart DefaultPart => EnvDTE.vsCMPart.vsCMPartWholeWithAttributes;

protected override VirtualTreePoint? GetStartPoint(SourceText text, SyntaxNode node, EnvDTE.vsCMPart part)
protected override VirtualTreePoint? GetStartPoint(SourceText text, OptionSet options, SyntaxNode node, EnvDTE.vsCMPart part)
{
switch (node.Kind())
{
Expand All @@ -53,16 +48,16 @@ protected override EnvDTE.vsCMPart DefaultPart
case SyntaxKind.DestructorDeclaration:
case SyntaxKind.OperatorDeclaration:
case SyntaxKind.ConversionOperatorDeclaration:
return GetStartPoint(text, (BaseMethodDeclarationSyntax)node, part);
return GetStartPoint(text, options, (BaseMethodDeclarationSyntax)node, part);
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.IndexerDeclaration:
case SyntaxKind.EventDeclaration:
return GetStartPoint(text, (BasePropertyDeclarationSyntax)node, part);
return GetStartPoint(text, options, (BasePropertyDeclarationSyntax)node, part);
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.AddAccessorDeclaration:
case SyntaxKind.RemoveAccessorDeclaration:
return GetStartPoint(text, (AccessorDeclarationSyntax)node, part);
return GetStartPoint(text, options, (AccessorDeclarationSyntax)node, part);
case SyntaxKind.DelegateDeclaration:
return GetStartPoint(text, (DelegateDeclarationSyntax)node, part);
case SyntaxKind.NamespaceDeclaration:
Expand All @@ -81,7 +76,7 @@ protected override EnvDTE.vsCMPart DefaultPart
}
}

protected override VirtualTreePoint? GetEndPoint(SourceText text, SyntaxNode node, EnvDTE.vsCMPart part)
protected override VirtualTreePoint? GetEndPoint(SourceText text, OptionSet options, SyntaxNode node, EnvDTE.vsCMPart part)
{
switch (node.Kind())
{
Expand Down Expand Up @@ -141,7 +136,7 @@ private VirtualTreePoint GetBodyStartPoint(SourceText text, SyntaxToken openBrac
: new VirtualTreePoint(openBrace.SyntaxTree, text, openBrace.Span.End);
}

private VirtualTreePoint GetBodyStartPoint(SourceText text, SyntaxToken openBrace, SyntaxToken closeBrace, int memberStartColumn)
private VirtualTreePoint GetBodyStartPoint(SourceText text, OptionSet options, SyntaxToken openBrace, SyntaxToken closeBrace, int memberStartColumn)
{
Debug.Assert(!openBrace.IsMissing);
Debug.Assert(!closeBrace.IsMissing);
Expand Down Expand Up @@ -181,7 +176,7 @@ private VirtualTreePoint GetBodyStartPoint(SourceText text, SyntaxToken openBrac

// If the line is all whitespace then place the caret at the first indent after the start
// of the member.
var indentSize = GetTabSize(text);
var indentSize = GetTabSize(options);
var lineText = lineAfterOpenBrace.ToString();

var lineEndColumn = lineText.GetColumnFromLineOffset(lineText.Length, indentSize);
Expand Down Expand Up @@ -347,7 +342,7 @@ private VirtualTreePoint GetStartPoint(SourceText text, BaseTypeDeclarationSynta
return new VirtualTreePoint(node.SyntaxTree, text, startPosition);
}

private VirtualTreePoint GetStartPoint(SourceText text, BaseMethodDeclarationSyntax node, EnvDTE.vsCMPart part)
private VirtualTreePoint GetStartPoint(SourceText text, OptionSet options, BaseMethodDeclarationSyntax node, EnvDTE.vsCMPart part)
{
int startPosition;

Expand Down Expand Up @@ -380,9 +375,9 @@ private VirtualTreePoint GetStartPoint(SourceText text, BaseMethodDeclarationSyn
if (node.Body != null && !node.Body.OpenBraceToken.IsMissing)
{
var line = text.Lines.GetLineFromPosition(node.SpanStart);
var indentation = line.GetColumnOfFirstNonWhitespaceCharacterOrEndOfLine(GetTabSize(text));
var indentation = line.GetColumnOfFirstNonWhitespaceCharacterOrEndOfLine(GetTabSize(options));

return GetBodyStartPoint(text, node.Body.OpenBraceToken, node.Body.CloseBraceToken, indentation);
return GetBodyStartPoint(text, options, node.Body.OpenBraceToken, node.Body.CloseBraceToken, indentation);
}
else
{
Expand Down Expand Up @@ -436,7 +431,7 @@ private AccessorDeclarationSyntax FindFirstAccessorNode(BasePropertyDeclarationS
return node.AccessorList.Accessors.FirstOrDefault();
}

private VirtualTreePoint GetStartPoint(SourceText text, BasePropertyDeclarationSyntax node, EnvDTE.vsCMPart part)
private VirtualTreePoint GetStartPoint(SourceText text, OptionSet options, BasePropertyDeclarationSyntax node, EnvDTE.vsCMPart part)
{
int startPosition;

Expand Down Expand Up @@ -467,17 +462,17 @@ private VirtualTreePoint GetStartPoint(SourceText text, BasePropertyDeclarationS
if (firstAccessorNode != null)
{
var line = text.Lines.GetLineFromPosition(firstAccessorNode.SpanStart);
var indentation = line.GetColumnOfFirstNonWhitespaceCharacterOrEndOfLine(GetTabSize(text));
var indentation = line.GetColumnOfFirstNonWhitespaceCharacterOrEndOfLine(GetTabSize(options));

if (firstAccessorNode.Body != null)
{
return GetBodyStartPoint(text, firstAccessorNode.Body.OpenBraceToken, firstAccessorNode.Body.CloseBraceToken, indentation);
return GetBodyStartPoint(text, options, firstAccessorNode.Body.OpenBraceToken, firstAccessorNode.Body.CloseBraceToken, indentation);
}
else if (!firstAccessorNode.SemicolonToken.IsMissing)
{
// This is total weirdness from the old C# code model with auto props.
// If there isn't a body, the semi-colon is used
return GetBodyStartPoint(text, firstAccessorNode.SemicolonToken, firstAccessorNode.SemicolonToken, indentation);
return GetBodyStartPoint(text, options, firstAccessorNode.SemicolonToken, firstAccessorNode.SemicolonToken, indentation);
}
}

Expand All @@ -487,9 +482,9 @@ private VirtualTreePoint GetStartPoint(SourceText text, BasePropertyDeclarationS
if (node.AccessorList != null && !node.AccessorList.OpenBraceToken.IsMissing)
{
var line = text.Lines.GetLineFromPosition(node.SpanStart);
var indentation = line.GetColumnOfFirstNonWhitespaceCharacterOrEndOfLine(GetTabSize(text));
var indentation = line.GetColumnOfFirstNonWhitespaceCharacterOrEndOfLine(GetTabSize(options));

return GetBodyStartPoint(text, node.AccessorList.OpenBraceToken, node.AccessorList.CloseBraceToken, indentation);
return GetBodyStartPoint(text, options, node.AccessorList.OpenBraceToken, node.AccessorList.CloseBraceToken, indentation);
}

throw Exceptions.ThrowEFail();
Expand All @@ -501,7 +496,7 @@ private VirtualTreePoint GetStartPoint(SourceText text, BasePropertyDeclarationS
return new VirtualTreePoint(node.SyntaxTree, text, startPosition);
}

private VirtualTreePoint GetStartPoint(SourceText text, AccessorDeclarationSyntax node, EnvDTE.vsCMPart part)
private VirtualTreePoint GetStartPoint(SourceText text, OptionSet options, AccessorDeclarationSyntax node, EnvDTE.vsCMPart part)
{
int startPosition;

Expand All @@ -526,9 +521,9 @@ private VirtualTreePoint GetStartPoint(SourceText text, AccessorDeclarationSynta
if (node.Body != null && !node.Body.OpenBraceToken.IsMissing)
{
var line = text.Lines.GetLineFromPosition(node.SpanStart);
var indentation = line.GetColumnOfFirstNonWhitespaceCharacterOrEndOfLine(GetTabSize(text));
var indentation = line.GetColumnOfFirstNonWhitespaceCharacterOrEndOfLine(GetTabSize(options));

return GetBodyStartPoint(text, node.Body.OpenBraceToken, node.Body.CloseBraceToken, indentation);
return GetBodyStartPoint(text, options, node.Body.OpenBraceToken, node.Body.CloseBraceToken, indentation);
}

throw Exceptions.ThrowEFail();
Expand Down
5 changes: 3 additions & 2 deletions src/VisualStudio/Core/Def/Implementation/ICodeModelNavigationPointService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Options;

namespace Microsoft.VisualStudio.LanguageServices.Implementation
{
Expand All @@ -11,11 +12,11 @@ internal interface ICodeModelNavigationPointService : ILanguageService
/// <summary>
/// Retrieves the start point of a given node for the specified EnvDTE.vsCMPart.
/// </summary>
VirtualTreePoint? GetStartPoint(SyntaxNode node, EnvDTE.vsCMPart? part = null);
VirtualTreePoint? GetStartPoint(SyntaxNode node, OptionSet options, EnvDTE.vsCMPart? part = null);

/// <summary>
/// Retrieves the end point of a given node for the specified EnvDTE.vsCMPart.
/// </summary>
VirtualTreePoint? GetEndPoint(SyntaxNode node, EnvDTE.vsCMPart? part = null);
VirtualTreePoint? GetEndPoint(SyntaxNode node, OptionSet options, EnvDTE.vsCMPart? part = null);
}
}
Loading

0 comments on commit 6969b51

Please sign in to comment.