Skip to content

Commit

Permalink
Fix Intellisense and other features of Bicep in Visual Studio that we…
Browse files Browse the repository at this point in the history
…re broken by VS 17.10 (#14532)

Fixes #14316

VS 17.10 added support for language server capabilities. This means that
it started firing notifications only for events that were explicitly
requested by the client (which is normal and works in vscode).
Unfortunately, their LSP implementation lacks support for document
selectors based on language ID (they only support file patterns,
language/scheme are NYI). This means it didn't recognize any of our
registrations and VS was giving us virtually no notifications, e.g. for
file changes. The fix is to use file patterns for VS, language/scheme
for any other hosts including vscode.

I fixed this such that it should only affect the behavior in VS, not
VsCode or other hosts, out of an abundance of caution (it would be
possible to return language/scheme as well as file patterns, but it does
cause minor behavior changes in vscode and would also mean VS behavior
would change in minor ways again after this fix this problem - better to
just make the behavior explicit for both hosts).

The *actual* fix is in DocumentSelectorFactory, almost all of the rest
is just passing along the dependencies it needs to know when to apply
the fix.
  • Loading branch information
StephenWeatherford authored Jul 17, 2024
1 parent 94c1f6a commit 76a95dc
Show file tree
Hide file tree
Showing 29 changed files with 182 additions and 118 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Bicep.Core.UnitTests.Utils;
using Bicep.LangServer.IntegrationTests.Helpers;
using Bicep.LanguageServer;
using Bicep.LanguageServer.Options;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using OmniSharp.Extensions.LanguageServer.Client;
Expand Down Expand Up @@ -39,6 +40,7 @@ public static async Task<LanguageServerHelper> StartServer(TestContext testConte
var serverPipe = new Pipe();

var server = new Server(
BicepLangServerOptions.Default,
options => options
.WithInput(serverPipe.Reader)
.WithOutput(clientPipe.Writer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
using Bicep.Core.UnitTests.Utils;
using Bicep.LanguageServer;
using Bicep.LanguageServer.Handlers;
using Bicep.LanguageServer.Options;
using Bicep.LanguageServer.Providers;
using Bicep.LanguageServer.Utils;
using FluentAssertions;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using OmniSharp.Extensions.LanguageServer.Protocol;
Expand Down Expand Up @@ -62,7 +64,7 @@ private async Task<IEnumerable<CodeAction>> GetCodeActions(
clientCapabilitiesProvider.Setup(m => m.DoesClientSupportShowDocumentRequest()).Returns(clientSupportShowDocumentRequestAndWorkspaceFolders);
clientCapabilitiesProvider.Setup(m => m.DoesClientSupportWorkspaceFolders()).Returns(clientSupportShowDocumentRequestAndWorkspaceFolders);

var bicepEditLinterRuleHandler = new BicepCodeActionHandler(bicepCompilationManager, clientCapabilitiesProvider.Object);
var bicepEditLinterRuleHandler = new BicepCodeActionHandler(bicepCompilationManager, clientCapabilitiesProvider.Object, new DocumentSelectorFactory(BicepLangServerOptions.Default));

var range = new Range()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
using Bicep.LangServer.IntegrationTests.Helpers;
using Bicep.LanguageServer.Configuration;
using Bicep.LanguageServer.Handlers;
using Bicep.LanguageServer.Options;
using Bicep.LanguageServer.Telemetry;
using Bicep.LanguageServer.Utils;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;

Expand Down Expand Up @@ -195,7 +197,8 @@ private async Task ChangeLinterRuleState(Mock<ITelemetryProvider> telemetryProvi
telemetryProvider.Object,
new Workspace());

var bicepTextDocumentSyncHandler = new BicepTextDocumentSyncHandler(compilationManager, bicepConfigChangeHandler);

var bicepTextDocumentSyncHandler = new BicepTextDocumentSyncHandler(compilationManager, bicepConfigChangeHandler, new DocumentSelectorFactory(BicepLangServerOptions.Default));

await bicepTextDocumentSyncHandler.Handle(TextDocumentParamHelper.CreateDidOpenDocumentParams(bicepConfigUri, prevBicepConfigFileContents, 1), CancellationToken.None);

Expand Down
6 changes: 4 additions & 2 deletions src/Bicep.LangServer/Handlers/BicepCodeActionHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ public class BicepCodeActionHandler : CodeActionHandlerBase
{
private readonly IClientCapabilitiesProvider clientCapabilitiesProvider;
private readonly ICompilationManager compilationManager;
private readonly DocumentSelectorFactory documentSelectorFactory;

public BicepCodeActionHandler(ICompilationManager compilationManager, IClientCapabilitiesProvider clientCapabilitiesProvider)
public BicepCodeActionHandler(ICompilationManager compilationManager, IClientCapabilitiesProvider clientCapabilitiesProvider, DocumentSelectorFactory documentSelectorFactory)
{
this.clientCapabilitiesProvider = clientCapabilitiesProvider;
this.compilationManager = compilationManager;
this.documentSelectorFactory = documentSelectorFactory;
}

public override async Task<CommandOrCodeActionContainer?> Handle(CodeActionParams request, CancellationToken cancellationToken)
Expand Down Expand Up @@ -237,7 +239,7 @@ private static CommandOrCodeAction CreateCodeFix(DocumentUri uri, CompilationCon

protected override CodeActionRegistrationOptions CreateRegistrationOptions(CodeActionCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = DocumentSelectorFactory.CreateForBicepAndParams(),
DocumentSelector = documentSelectorFactory.CreateForBicepAndParams(),
CodeActionKinds = new Container<CodeActionKind>(CodeActionKind.QuickFix),
ResolveProvider = false
};
Expand Down
6 changes: 4 additions & 2 deletions src/Bicep.LangServer/Handlers/BicepCodeLensHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ namespace Bicep.LanguageServer.Handlers
public class BicepCodeLensHandler : CodeLensHandlerBase
{
private readonly IModuleDispatcher moduleDispatcher;
private readonly DocumentSelectorFactory documentSelectorFactory;

public BicepCodeLensHandler(IModuleDispatcher moduleDispatcher)
public BicepCodeLensHandler(IModuleDispatcher moduleDispatcher, DocumentSelectorFactory documentSelectorFactory)
{
this.moduleDispatcher = moduleDispatcher;
this.documentSelectorFactory = documentSelectorFactory;
}

public override Task<CodeLensContainer?> Handle(CodeLensParams request, CancellationToken cancellationToken)
Expand All @@ -37,7 +39,7 @@ public override Task<CodeLens> Handle(CodeLens request, CancellationToken cancel
protected override CodeLensRegistrationOptions CreateRegistrationOptions(CodeLensCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = new(
DocumentSelectorFactory.AllSupportedLangIds
documentSelectorFactory.CreateForAllSupportedLangIds()
.Concat(TextDocumentFilter.ForScheme(LangServerConstants.ExternalSourceFileScheme))),
ResolveProvider = false
};
Expand Down
6 changes: 4 additions & 2 deletions src/Bicep.LangServer/Handlers/BicepCompletionHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ public class BicepCompletionHandler : CompletionHandlerBase
private readonly ICompilationManager compilationManager;
private readonly ICompletionProvider completionProvider;
private readonly IFeatureProviderFactory featureProviderFactory;
private readonly DocumentSelectorFactory documentSelectorFactory;

public BicepCompletionHandler(ILogger<BicepCompletionHandler> logger, ICompilationManager compilationManager, ICompletionProvider completionProvider, IFeatureProviderFactory featureProviderFactory)
public BicepCompletionHandler(ILogger<BicepCompletionHandler> logger, ICompilationManager compilationManager, ICompletionProvider completionProvider, IFeatureProviderFactory featureProviderFactory, DocumentSelectorFactory documentSelectorFactory)
{
this.logger = logger;
this.compilationManager = compilationManager;
this.completionProvider = completionProvider;
this.featureProviderFactory = featureProviderFactory;
this.documentSelectorFactory = documentSelectorFactory;
}

public override async Task<CompletionList> Handle(CompletionParams request, CancellationToken cancellationToken)
Expand Down Expand Up @@ -63,7 +65,7 @@ public override Task<CompletionItem> Handle(CompletionItem request, Cancellation

protected override CompletionRegistrationOptions CreateRegistrationOptions(CompletionCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = DocumentSelectorFactory.CreateForBicepAndParams(),
DocumentSelector = documentSelectorFactory.CreateForBicepAndParams(),
AllCommitCharacters = new Container<string>(),
ResolveProvider = false,
TriggerCharacters = new Container<string>(":", " ", ".", "/", "'", "@", "{", "#", "?")
Expand Down
7 changes: 5 additions & 2 deletions src/Bicep.LangServer/Handlers/BicepDefinitionHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,24 @@ public class BicepDefinitionHandler : DefinitionHandlerBase
private readonly ILanguageServerFacade languageServer;
private readonly IModuleDispatcher moduleDispatcher;
private readonly IFeatureProviderFactory featureProviderFactory;
private readonly DocumentSelectorFactory documentSelectorFactory;

public BicepDefinitionHandler(
ISymbolResolver symbolResolver,
ICompilationManager compilationManager,
IFileResolver fileResolver,
ILanguageServerFacade languageServer,
IModuleDispatcher moduleDispatcher,
IFeatureProviderFactory featureProviderFactory) : base()
IFeatureProviderFactory featureProviderFactory,
DocumentSelectorFactory documentSelectorFactory) : base()
{
this.symbolResolver = symbolResolver;
this.compilationManager = compilationManager;
this.fileResolver = fileResolver;
this.languageServer = languageServer;
this.moduleDispatcher = moduleDispatcher;
this.featureProviderFactory = featureProviderFactory;
this.documentSelectorFactory = documentSelectorFactory;
}

public override async Task<LocationOrLocationLinks?> Handle(DefinitionParams request, CancellationToken cancellationToken)
Expand Down Expand Up @@ -98,7 +101,7 @@ public BicepDefinitionHandler(

protected override DefinitionRegistrationOptions CreateRegistrationOptions(DefinitionCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = DocumentSelectorFactory.CreateForBicepAndParams()
DocumentSelector = documentSelectorFactory.CreateForBicepAndParams()
};

private LocationOrLocationLinks HandleUnboundSymbolLocation(DefinitionParams request, CompilationContext context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ namespace Bicep.LanguageServer.Handlers
{
public class BicepDocumentFormattingHandler(
ILogger<BicepDocumentSymbolHandler> logger,
ICompilationManager compilationManager) : DocumentFormattingHandlerBase
ICompilationManager compilationManager,
DocumentSelectorFactory documentSelectorFactory) : DocumentFormattingHandlerBase
{
public override Task<TextEditContainer?> Handle(DocumentFormattingParams request, CancellationToken cancellationToken)
{
Expand Down Expand Up @@ -64,7 +65,7 @@ public class BicepDocumentFormattingHandler(

protected override DocumentFormattingRegistrationOptions CreateRegistrationOptions(DocumentFormattingCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = DocumentSelectorFactory.CreateForBicepAndParams()
DocumentSelector = documentSelectorFactory.CreateForBicepAndParams()
};
}
}
13 changes: 3 additions & 10 deletions src/Bicep.LangServer/Handlers/BicepDocumentHighlightHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,11 @@

namespace Bicep.LanguageServer.Handlers
{
public class BicepDocumentHighlightHandler : DocumentHighlightHandlerBase
public class BicepDocumentHighlightHandler(ISymbolResolver symbolResolver, DocumentSelectorFactory documentSelectorFactory) : DocumentHighlightHandlerBase
{
private readonly ISymbolResolver symbolResolver;

public BicepDocumentHighlightHandler(ISymbolResolver symbolResolver) : base()
{
this.symbolResolver = symbolResolver;
}

public override Task<DocumentHighlightContainer?> Handle(DocumentHighlightParams request, CancellationToken cancellationToken)
{
var result = this.symbolResolver.ResolveSymbol(request.TextDocument.Uri, request.Position);
var result = symbolResolver.ResolveSymbol(request.TextDocument.Uri, request.Position);
if (result == null)
{
return Task.FromResult<DocumentHighlightContainer?>(null);
Expand All @@ -45,7 +38,7 @@ public BicepDocumentHighlightHandler(ISymbolResolver symbolResolver) : base()

protected override DocumentHighlightRegistrationOptions CreateRegistrationOptions(DocumentHighlightCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = DocumentSelectorFactory.CreateForBicepAndParams()
DocumentSelector = documentSelectorFactory.CreateForBicepAndParams()
};
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/Bicep.LangServer/Handlers/BicepDocumentSymbolHandler.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using System.Collections.Immutable;
using System.DirectoryServices.AccountManagement;
using Bicep.Core.Semantics;
using Bicep.Core.Syntax;
using Bicep.LanguageServer.CompilationManager;
Expand All @@ -18,11 +19,13 @@ public class BicepDocumentSymbolHandler : DocumentSymbolHandlerBase
{
private readonly ILogger<BicepDocumentSymbolHandler> logger;
private readonly ICompilationManager compilationManager;
private readonly DocumentSelectorFactory documentSelectorFactory;

public BicepDocumentSymbolHandler(ILogger<BicepDocumentSymbolHandler> logger, ICompilationManager compilationManager)
public BicepDocumentSymbolHandler(ILogger<BicepDocumentSymbolHandler> logger, ICompilationManager compilationManager, DocumentSelectorFactory documentSelectorFactory)
{
this.logger = logger;
this.compilationManager = compilationManager;
this.documentSelectorFactory = documentSelectorFactory;
}

public override async Task<SymbolInformationOrDocumentSymbolContainer?> Handle(DocumentSymbolParams request, CancellationToken cancellationToken)
Expand Down Expand Up @@ -104,7 +107,7 @@ private DocumentSymbol CreateDocumentSymbol(SemanticModel model, DeclaredSymbol

protected override DocumentSymbolRegistrationOptions CreateRegistrationOptions(DocumentSymbolCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = DocumentSelectorFactory.CreateForBicepAndParams()
DocumentSelector = documentSelectorFactory.CreateForBicepAndParams()
};
}
}
7 changes: 5 additions & 2 deletions src/Bicep.LangServer/Handlers/BicepHoverHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,18 @@ public class BicepHoverHandler : HoverHandlerBase
private readonly IModuleDispatcher moduleDispatcher;
private readonly IArtifactRegistryProvider moduleRegistryProvider;
private readonly ISymbolResolver symbolResolver;
private readonly DocumentSelectorFactory documentSelectorFactory;

public BicepHoverHandler(
IModuleDispatcher moduleDispatcher,
IArtifactRegistryProvider moduleRegistryProvider,
ISymbolResolver symbolResolver)
ISymbolResolver symbolResolver,
DocumentSelectorFactory documentSelectorFactory)
{
this.moduleDispatcher = moduleDispatcher;
this.moduleRegistryProvider = moduleRegistryProvider;
this.symbolResolver = symbolResolver;
this.documentSelectorFactory = documentSelectorFactory;
}

public override async Task<Hover?> Handle(HoverParams request, CancellationToken cancellationToken)
Expand Down Expand Up @@ -341,7 +344,7 @@ private static MarkedStringsOrMarkupContent AsMarkdown(IEnumerable<string> markd

protected override HoverRegistrationOptions CreateRegistrationOptions(HoverCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = DocumentSelectorFactory.CreateForBicepAndParams()
DocumentSelector = documentSelectorFactory.CreateForBicepAndParams()
};
}
}
13 changes: 3 additions & 10 deletions src/Bicep.LangServer/Handlers/BicepReferencesHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,12 @@

namespace Bicep.LanguageServer.Handlers
{
public class BicepReferencesHandler : ReferencesHandlerBase
public class BicepReferencesHandler(ISymbolResolver symbolResolver, DocumentSelectorFactory documentSelectorFactory) : ReferencesHandlerBase
{
private readonly ISymbolResolver symbolResolver;

public BicepReferencesHandler(ISymbolResolver symbolResolver)
{
this.symbolResolver = symbolResolver;
}

public override async Task<LocationContainer?> Handle(ReferenceParams request, CancellationToken cancellationToken)
{
await Task.CompletedTask;
var result = this.symbolResolver.ResolveSymbol(request.TextDocument.Uri, request.Position);
var result = symbolResolver.ResolveSymbol(request.TextDocument.Uri, request.Position);
if (result == null)
{
return null;
Expand All @@ -48,7 +41,7 @@ public BicepReferencesHandler(ISymbolResolver symbolResolver)

protected override ReferenceRegistrationOptions CreateRegistrationOptions(ReferenceCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = DocumentSelectorFactory.CreateForBicepAndParams()
DocumentSelector = documentSelectorFactory.CreateForBicepAndParams()
};
}
}
Expand Down
13 changes: 3 additions & 10 deletions src/Bicep.LangServer/Handlers/BicepRenameHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,11 @@

namespace Bicep.LanguageServer.Handlers
{
public class BicepRenameHandler : RenameHandlerBase
public class BicepRenameHandler(ISymbolResolver symbolResolver, DocumentSelectorFactory documentSelectorFactory) : RenameHandlerBase
{
private readonly ISymbolResolver symbolResolver;

public BicepRenameHandler(ISymbolResolver symbolResolver) : base()
{
this.symbolResolver = symbolResolver;
}

public override Task<WorkspaceEdit?> Handle(RenameParams request, CancellationToken cancellationToken)
{
var result = this.symbolResolver.ResolveSymbol(request.TextDocument.Uri, request.Position);
var result = symbolResolver.ResolveSymbol(request.TextDocument.Uri, request.Position);
if (result == null || !(result.Symbol is DeclaredSymbol))
{
// result is not a symbol or it's a built-in symbol that was not declared by the user (namespaces, functions, for example)
Expand Down Expand Up @@ -89,7 +82,7 @@ public BicepRenameHandler(ISymbolResolver symbolResolver) : base()

protected override RenameRegistrationOptions CreateRegistrationOptions(RenameCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = DocumentSelectorFactory.CreateForBicepAndParams(),
DocumentSelector = documentSelectorFactory.CreateForBicepAndParams(),
PrepareProvider = false
};
}
Expand Down
13 changes: 3 additions & 10 deletions src/Bicep.LangServer/Handlers/BicepSemanticTokensHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,11 @@

namespace Bicep.LanguageServer.Handlers
{
public class BicepSemanticTokensHandler : SemanticTokensHandlerBase
public class BicepSemanticTokensHandler(ICompilationManager compilationManager, DocumentSelectorFactory documentSelectorFactory) : SemanticTokensHandlerBase
{
private readonly ICompilationManager compilationManager;

// TODO: Not sure if this needs to be shared.
private readonly SemanticTokensLegend legend = new();

public BicepSemanticTokensHandler(ICompilationManager compilationManager)
{
this.compilationManager = compilationManager;
}

protected override Task<SemanticTokensDocument> GetSemanticTokensDocument(ITextDocumentIdentifierParams @params, CancellationToken cancellationToken)
{
return Task.FromResult(new SemanticTokensDocument(this.legend));
Expand All @@ -31,7 +24,7 @@ protected override Task Tokenize(SemanticTokensBuilder builder, ITextDocumentIde
* do not check for file extension here because that will prevent untitled files from getting syntax highlighting
*/

var compilationContext = this.compilationManager.GetCompilation(identifier.TextDocument.Uri);
var compilationContext = compilationManager.GetCompilation(identifier.TextDocument.Uri);
if (compilationContext is not null)
{
SemanticTokenVisitor.BuildSemanticTokens(builder, compilationContext.Compilation.GetEntrypointSemanticModel());
Expand All @@ -44,7 +37,7 @@ protected override Task Tokenize(SemanticTokensBuilder builder, ITextDocumentIde
{
// the semantic tokens handler requests don't get routed like other handlers
// it seems we can only have one and it must be shared between all the language IDs we support
DocumentSelector = DocumentSelectorFactory.CreateForBicepAndParams(),
DocumentSelector = documentSelectorFactory.CreateForBicepAndParams(),
Legend = this.legend,
Full = new SemanticTokensCapabilityRequestFull
{
Expand Down
Loading

0 comments on commit 76a95dc

Please sign in to comment.