diff --git a/src/EditorFeatures/CSharpTest/AddUsing/AddUsingTests_NuGet.cs b/src/EditorFeatures/CSharpTest/AddUsing/AddUsingTests_NuGet.cs index 53155eedcb87e2fca7b6b8081538282c1d8aafa7..28f2e728a387ffabc795bec4c34fd43abb1605f6 100644 --- a/src/EditorFeatures/CSharpTest/AddUsing/AddUsingTests_NuGet.cs +++ b/src/EditorFeatures/CSharpTest/AddUsing/AddUsingTests_NuGet.cs @@ -54,7 +54,7 @@ public async Task TestSearchPackageSingleName() // Make a loose mock for the installer service. We don't care what this test // calls on it. var installerServiceMock = new Mock(MockBehavior.Loose); - installerServiceMock.SetupGet(i => i.IsEnabled).Returns(true); + installerServiceMock.Setup(i => i.IsEnabled(It.IsAny())).Returns(true); installerServiceMock.SetupGet(i => i.PackageSources).Returns(NugetPackageSources); installerServiceMock.Setup(s => s.TryInstallPackage(It.IsAny(), It.IsAny(), It.IsAny(), "NuGetPackage", It.IsAny(), It.IsAny(), It.IsAny())) .Returns(true); @@ -83,7 +83,7 @@ public async Task TestSearchPackageMultipleNames() // Make a loose mock for the installer service. We don't care what this test // calls on it. var installerServiceMock = new Mock(MockBehavior.Loose); - installerServiceMock.SetupGet(i => i.IsEnabled).Returns(true); + installerServiceMock.Setup(i => i.IsEnabled(It.IsAny())).Returns(true); installerServiceMock.SetupGet(i => i.PackageSources).Returns(NugetPackageSources); installerServiceMock.Setup(s => s.TryInstallPackage(It.IsAny(), It.IsAny(), It.IsAny(), "NuGetPackage", It.IsAny(), It.IsAny(), It.IsAny())) .Returns(true); @@ -112,7 +112,7 @@ public async Task TestMissingIfPackageAlreadyInstalled() // Make a loose mock for the installer service. We don't care what this test // calls on it. var installerServiceMock = new Mock(MockBehavior.Loose); - installerServiceMock.SetupGet(i => i.IsEnabled).Returns(true); + installerServiceMock.Setup(i => i.IsEnabled(It.IsAny())).Returns(true); installerServiceMock.SetupGet(i => i.PackageSources).Returns(NugetPackageSources); installerServiceMock.Setup(s => s.IsInstalled(It.IsAny(), It.IsAny(), "NuGetPackage")) .Returns(true); @@ -135,7 +135,7 @@ public async Task TestOptionsOffered() // Make a loose mock for the installer service. We don't care what this test // calls on it. var installerServiceMock = new Mock(MockBehavior.Loose); - installerServiceMock.SetupGet(i => i.IsEnabled).Returns(true); + installerServiceMock.Setup(i => i.IsEnabled(It.IsAny())).Returns(true); installerServiceMock.SetupGet(i => i.PackageSources).Returns(NugetPackageSources); installerServiceMock.Setup(s => s.GetInstalledVersions("NuGetPackage")) .Returns(ImmutableArray.Create("1.0", "2.0")); @@ -177,7 +177,7 @@ public async Task TestOptionsOffered() public async Task TestInstallGetsCalledNoVersion() { var installerServiceMock = new Mock(MockBehavior.Loose); - installerServiceMock.SetupGet(i => i.IsEnabled).Returns(true); + installerServiceMock.Setup(i => i.IsEnabled(It.IsAny())).Returns(true); installerServiceMock.SetupGet(i => i.PackageSources).Returns(NugetPackageSources); installerServiceMock.Setup(s => s.TryInstallPackage(It.IsAny(), It.IsAny(), It.IsAny(), "NuGetPackage", /*versionOpt*/ null, It.IsAny(), It.IsAny())) .Returns(true); @@ -205,7 +205,7 @@ class C public async Task TestInstallGetsCalledWithVersion() { var installerServiceMock = new Mock(MockBehavior.Loose); - installerServiceMock.SetupGet(i => i.IsEnabled).Returns(true); + installerServiceMock.Setup(i => i.IsEnabled(It.IsAny())).Returns(true); installerServiceMock.SetupGet(i => i.PackageSources).Returns(NugetPackageSources); installerServiceMock.Setup(s => s.GetInstalledVersions("NuGetPackage")) .Returns(ImmutableArray.Create("1.0")); @@ -235,7 +235,7 @@ class C public async Task TestFailedInstallRollsBackFile() { var installerServiceMock = new Mock(MockBehavior.Loose); - installerServiceMock.SetupGet(i => i.IsEnabled).Returns(true); + installerServiceMock.Setup(i => i.IsEnabled(It.IsAny())).Returns(true); installerServiceMock.SetupGet(i => i.PackageSources).Returns(NugetPackageSources); installerServiceMock.Setup(s => s.GetInstalledVersions("NuGetPackage")) .Returns(ImmutableArray.Create("1.0")); diff --git a/src/EditorFeatures/VisualBasicTest/Diagnostics/AddImport/AddImportTests_NuGet.vb b/src/EditorFeatures/VisualBasicTest/Diagnostics/AddImport/AddImportTests_NuGet.vb index 92d7484321008067bbce69778517e12c29726e5a..fff18f70367c73b90a68a09cce1f68699828e778 100644 --- a/src/EditorFeatures/VisualBasicTest/Diagnostics/AddImport/AddImportTests_NuGet.vb +++ b/src/EditorFeatures/VisualBasicTest/Diagnostics/AddImport/AddImportTests_NuGet.vb @@ -44,7 +44,7 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.CodeActions.AddImp ' Make a loose mock for the installer service. We don't care what this test ' calls on it. Dim installerServiceMock = New Mock(Of IPackageInstallerService)(MockBehavior.Loose) - installerServiceMock.SetupGet(Function(i) i.IsEnabled).Returns(True) + installerServiceMock.Setup(Function(i) i.IsEnabled(It.IsAny(Of ProjectId))).Returns(True) installerServiceMock.SetupGet(Function(i) i.PackageSources).Returns(NugetPackageSources) installerServiceMock.Setup(Function(s) s.TryInstallPackage(It.IsAny(Of Workspace), It.IsAny(Of DocumentId), It.IsAny(Of String), "NuGetPackage", It.IsAny(Of String), It.IsAny(Of Boolean), It.IsAny(Of CancellationToken))). Returns(True) @@ -71,7 +71,7 @@ End Class", fixProviderData:=New ProviderData(installerServiceMock.Object, packa ' Make a loose mock for the installer service. We don't care what this test ' calls on it. Dim installerServiceMock = New Mock(Of IPackageInstallerService)(MockBehavior.Loose) - installerServiceMock.SetupGet(Function(i) i.IsEnabled).Returns(True) + installerServiceMock.Setup(Function(i) i.IsEnabled(It.IsAny(Of ProjectId))).Returns(True) installerServiceMock.SetupGet(Function(i) i.PackageSources).Returns(NugetPackageSources) installerServiceMock.Setup(Function(s) s.TryInstallPackage(It.IsAny(Of Workspace), It.IsAny(Of DocumentId), It.IsAny(Of String), "NuGetPackage", It.IsAny(Of String), It.IsAny(Of Boolean), It.IsAny(Of CancellationToken))). Returns(True) @@ -98,7 +98,7 @@ End Class", fixProviderData:=New ProviderData(installerServiceMock.Object, packa ' Make a loose mock for the installer service. We don't care what this test ' calls on it. Dim installerServiceMock = New Mock(Of IPackageInstallerService)(MockBehavior.Loose) - installerServiceMock.SetupGet(Function(i) i.IsEnabled).Returns(True) + installerServiceMock.Setup(Function(i) i.IsEnabled(It.IsAny(Of ProjectId))).Returns(True) installerServiceMock.SetupGet(Function(i) i.PackageSources).Returns(NugetPackageSources) installerServiceMock.Setup(Function(s) s.TryInstallPackage(It.IsAny(Of Workspace), It.IsAny(Of DocumentId), It.IsAny(Of String), "NuGetPackage", It.IsAny(Of String), It.IsAny(Of Boolean), It.IsAny(Of CancellationToken))). Returns(False) @@ -123,7 +123,7 @@ End Class", fixProviderData:=New ProviderData(installerServiceMock.Object, packa ' Make a loose mock for the installer service. We don't care what this test ' calls on it. Dim installerServiceMock = New Mock(Of IPackageInstallerService)(MockBehavior.Loose) - installerServiceMock.SetupGet(Function(i) i.IsEnabled).Returns(True) + installerServiceMock.Setup(Function(i) i.IsEnabled(It.IsAny(Of ProjectId))).Returns(True) installerServiceMock.SetupGet(Function(i) i.PackageSources).Returns(NugetPackageSources) installerServiceMock.Setup(Function(s) s.IsInstalled(It.IsAny(Of Workspace)(), It.IsAny(Of ProjectId)(), "NuGetPackage")). Returns(True) @@ -145,7 +145,7 @@ New TestParameters(fixProviderData:=New ProviderData(installerServiceMock.Object ' Make a loose mock for the installer service. We don't care what this test ' calls on it. Dim installerServiceMock = New Mock(Of IPackageInstallerService)(MockBehavior.Loose) - installerServiceMock.SetupGet(Function(i) i.IsEnabled).Returns(True) + installerServiceMock.Setup(Function(i) i.IsEnabled(It.IsAny(Of ProjectId))).Returns(True) installerServiceMock.SetupGet(Function(i) i.PackageSources).Returns(NugetPackageSources) installerServiceMock.Setup(Function(s) s.GetInstalledVersions("NuGetPackage")). Returns(ImmutableArray.Create("1.0", "2.0")) @@ -185,7 +185,7 @@ parameters:=New TestParameters(fixProviderData:=data)) Public Async Function TestInstallGetsCalledNoVersion() As Task Dim installerServiceMock = New Mock(Of IPackageInstallerService)(MockBehavior.Loose) - installerServiceMock.SetupGet(Function(i) i.IsEnabled).Returns(True) + installerServiceMock.Setup(Function(i) i.IsEnabled(It.IsAny(Of ProjectId))).Returns(True) installerServiceMock.SetupGet(Function(i) i.PackageSources).Returns(NugetPackageSources) installerServiceMock.Setup(Function(s) s.TryInstallPackage(It.IsAny(Of Workspace), It.IsAny(Of DocumentId), It.IsAny(Of String), "NuGetPackage", Nothing, It.IsAny(Of Boolean), It.IsAny(Of CancellationToken))). Returns(True) @@ -211,7 +211,7 @@ End Class", fixProviderData:=New ProviderData(installerServiceMock.Object, packa Public Async Function TestInstallGetsCalledWithVersion() As Task Dim installerServiceMock = New Mock(Of IPackageInstallerService)(MockBehavior.Loose) - installerServiceMock.SetupGet(Function(i) i.IsEnabled).Returns(True) + installerServiceMock.Setup(Function(i) i.IsEnabled(It.IsAny(Of ProjectId))).Returns(True) installerServiceMock.SetupGet(Function(i) i.PackageSources).Returns(NugetPackageSources) installerServiceMock.Setup(Function(s) s.GetInstalledVersions("NuGetPackage")). Returns(ImmutableArray.Create("1.0")) diff --git a/src/Features/Core/Portable/AddImport/SymbolReferenceFinder_PackageAssemblySearch.cs b/src/Features/Core/Portable/AddImport/SymbolReferenceFinder_PackageAssemblySearch.cs index 9945ac8a2dfdc845895aee665303b12eca795927..c3bce9a2bfa1dd0477f712b87223acd6fd60989c 100644 --- a/src/Features/Core/Portable/AddImport/SymbolReferenceFinder_PackageAssemblySearch.cs +++ b/src/Features/Core/Portable/AddImport/SymbolReferenceFinder_PackageAssemblySearch.cs @@ -84,7 +84,7 @@ private partial class SymbolReferenceFinder if (symbolSearchService != null && installerService != null && searchNugetPackages && - installerService.IsEnabled) + installerService.IsEnabled(_document.Project.Id)) { foreach (var packageSource in installerService.PackageSources) { diff --git a/src/Features/Core/Portable/AddPackage/AbstractAddPackageCodeFixProvider.cs b/src/Features/Core/Portable/AddPackage/AbstractAddPackageCodeFixProvider.cs index 26178cc3e638a4ad24b08c1e476071825303a8aa..7d5f063e534cdc145cdcf07523f1709d7620f5e3 100644 --- a/src/Features/Core/Portable/AddPackage/AbstractAddPackageCodeFixProvider.cs +++ b/src/Features/Core/Portable/AddPackage/AbstractAddPackageCodeFixProvider.cs @@ -51,7 +51,7 @@ internal abstract partial class AbstractAddPackageCodeFixProvider : CodeFixProvi if (symbolSearchService != null && installerService != null && searchNugetPackages && - installerService.IsEnabled) + installerService.IsEnabled(document.Project.Id)) { foreach (var packageSource in installerService.PackageSources) { diff --git a/src/VisualStudio/Core/Def/Packaging/PackageInstallerService.ProjectState.cs b/src/VisualStudio/Core/Def/Packaging/PackageInstallerService.ProjectState.cs new file mode 100644 index 0000000000000000000000000000000000000000..e4a1f6003d77fd5d0c50f3d791f245e799c5e0ab --- /dev/null +++ b/src/VisualStudio/Core/Def/Packaging/PackageInstallerService.ProjectState.cs @@ -0,0 +1,21 @@ +// 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.Collections.Generic; + +namespace Microsoft.VisualStudio.LanguageServices.Packaging +{ + internal partial class PackageInstallerService + { + private struct ProjectState + { + public readonly bool IsEnabled; + public readonly Dictionary InstalledPackageToVersion; + + public ProjectState(bool isEnabled, Dictionary installedPackageToVersion) + { + IsEnabled = isEnabled; + InstalledPackageToVersion = installedPackageToVersion; + } + } + } +} diff --git a/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs b/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs index 51264ba221f4778ce7a665a219d14a00f21d725e..2731b73f54f7d7509d6ae88e3522d9df937613d2 100644 --- a/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs +++ b/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs @@ -56,8 +56,8 @@ internal partial class PackageInstallerService : AbstractDelayStartedService, IP private bool _solutionChanged; private HashSet _changedProjects = new HashSet(); - private readonly ConcurrentDictionary> _projectToInstalledPackageAndVersion = - new ConcurrentDictionary>(); + private readonly ConcurrentDictionary _projectToInstalledPackageAndVersion = + new ConcurrentDictionary(); [ImportingConstructor] public PackageInstallerService( @@ -77,7 +77,23 @@ internal partial class PackageInstallerService : AbstractDelayStartedService, IP public event EventHandler PackageSourcesChanged; - public bool IsEnabled => _packageServices != null; + private bool IsEnabled => _packageServices != null; + + bool IPackageInstallerService.IsEnabled(ProjectId projectId) + { + if (_packageServices == null) + { + return false; + } + + if (_projectToInstalledPackageAndVersion.TryGetValue(projectId, out var state)) + { + return state.IsEnabled; + } + + // If we haven't scanned the project yet, assume that we're available for it. + return true; + } protected override void EnableService() { @@ -270,10 +286,25 @@ private string GetInstalledVersion(string packageName, EnvDTE.Project dteProject var metadata = installedPackages.FirstOrDefault(m => m.Id == packageName); return metadata?.VersionString; } + catch (ArgumentException e) when (IsKnownNugetIssue(e)) + { + // Nuget may throw an ArgumentException when there is something about the project + // they do not like/support. + } catch (Exception e) when (FatalError.ReportWithoutCrash(e)) { - return null; } + + return null; + } + + private bool IsKnownNugetIssue(ArgumentException exception) + { + // See https://github.com/NuGet/Home/issues/4706 + // Nuget throws on legal projects. We do not want to report this exception + // as it is known (and NFWs are expensive), but we do want to report if we + // run into anything else. + return exception.Message.Contains("is not a valid version string"); } private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) @@ -388,8 +419,9 @@ private ProjectId DequeueNextProject(Solution solution) private void ProcessProjectChange(Solution solution, ProjectId projectId) { this.AssertIsForeground(); + // Remove anything we have associated with this project. - _projectToInstalledPackageAndVersion.TryRemove(projectId, out var installedPackages); + _projectToInstalledPackageAndVersion.TryRemove(projectId, out var projectState); var project = solution.GetProject(projectId); if (project == null) @@ -416,26 +448,35 @@ private void ProcessProjectChange(Solution solution, ProjectId projectId) return; } - installedPackages = new Dictionary(); + var installedPackages = new Dictionary(); + var isEnabled = false; // Calling into NuGet. Assume they may fail for any reason. try { var installedPackageMetadata = _packageServices.GetInstalledPackages(dteProject); installedPackages.AddRange(installedPackageMetadata.Select(m => new KeyValuePair(m.Id, m.VersionString))); + isEnabled = true; + } + catch (ArgumentException e) when (IsKnownNugetIssue(e)) + { + // Nuget may throw an ArgumentException when there is something about the project + // they do not like/support. } catch (Exception e) when (FatalError.ReportWithoutCrash(e)) { } - _projectToInstalledPackageAndVersion.AddOrUpdate(projectId, installedPackages, (_1, _2) => installedPackages); + var state = new ProjectState(isEnabled, installedPackages); + _projectToInstalledPackageAndVersion.AddOrUpdate( + projectId, state, (_1, _2) => state); } public bool IsInstalled(Workspace workspace, ProjectId projectId, string packageName) { ThisCanBeCalledOnAnyThread(); return _projectToInstalledPackageAndVersion.TryGetValue(projectId, out var installedPackages) && - installedPackages.ContainsKey(packageName); + installedPackages.InstalledPackageToVersion.ContainsKey(packageName); } public ImmutableArray GetInstalledVersions(string packageName) @@ -443,10 +484,10 @@ public ImmutableArray GetInstalledVersions(string packageName) ThisCanBeCalledOnAnyThread(); var installedVersions = new HashSet(); - foreach (var installedPackages in _projectToInstalledPackageAndVersion.Values) + foreach (var state in _projectToInstalledPackageAndVersion.Values) { string version = null; - if (installedPackages?.TryGetValue(packageName, out version) == true && version != null) + if (state.InstalledPackageToVersion.TryGetValue(packageName, out version) && version != null) { installedVersions.Add(version); } @@ -494,16 +535,13 @@ public IEnumerable GetProjectsWithInstalledPackage(Solution solution, s foreach (var kvp in this._projectToInstalledPackageAndVersion) { - var installedPackageAndVersion = kvp.Value; - if (installedPackageAndVersion != null) + var state = kvp.Value; + if (state.InstalledPackageToVersion.TryGetValue(packageName, out var installedVersion) && installedVersion == version) { - if (installedPackageAndVersion.TryGetValue(packageName, out var installedVersion) && installedVersion == version) + var project = solution.GetProject(kvp.Key); + if (project != null) { - var project = solution.GetProject(kvp.Key); - if (project != null) - { - result.Add(project); - } + result.Add(project); } } } @@ -631,4 +669,4 @@ public void UninstallPackage(EnvDTE.Project project, string packageId, bool remo => _packageUninstaller.UninstallPackage(project, packageId, removeDependencies); } } -} +} \ No newline at end of file diff --git a/src/VisualStudio/Core/Def/ServicesVisualStudio.csproj b/src/VisualStudio/Core/Def/ServicesVisualStudio.csproj index 7d8e9ac64eaac37dfbf99ed32b285a75ddcae111..1809a21b161354a9dd2ad2358905c207c30675cd 100644 --- a/src/VisualStudio/Core/Def/ServicesVisualStudio.csproj +++ b/src/VisualStudio/Core/Def/ServicesVisualStudio.csproj @@ -174,6 +174,7 @@ + diff --git a/src/Workspaces/Core/Portable/Packaging/IPackageInstallerService.cs b/src/Workspaces/Core/Portable/Packaging/IPackageInstallerService.cs index 46b94400ed46db98ec2ffab41cea47db03f919ac..40b136736450fcd487ccea323446b35cc648a8a8 100644 --- a/src/Workspaces/Core/Portable/Packaging/IPackageInstallerService.cs +++ b/src/Workspaces/Core/Portable/Packaging/IPackageInstallerService.cs @@ -11,7 +11,7 @@ namespace Microsoft.CodeAnalysis.Packaging { internal interface IPackageInstallerService : IWorkspaceService { - bool IsEnabled { get; } + bool IsEnabled(ProjectId projectId); bool IsInstalled(Workspace workspace, ProjectId projectId, string packageName);