Targeting IDE projects to .NET 6 #66655

genlu · 2023-02-01T21:32:46Z

@dotnet/roslyn-ide @dotnet/roslyn-compiler Please take a look. Thanks

genlu · 2023-02-01T21:33:32Z

src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentMetadataAsSourceFileProvider.cs

@@ -257,7 +257,7 @@ internal sealed class PdbSourceDocumentMetadataAsSourceFileProvider : IMetadataA
            // to the document passed in, which we just use the first document for.
            // TODO: Support results from multiple source files: https://github.com/dotnet/roslyn/issues/55834
            var firstDocumentFilePath = sourceFileInfos[0]!.FilePath;
-            var firstDocument = navigateProject.Documents.FirstOrDefault(d => d.FilePath?.Equals(firstDocumentFilePath, StringComparison.OrdinalIgnoreCase) ?? false);
+            var firstDocument = navigateProject.Documents.First(d => d.FilePath?.Equals(firstDocumentFilePath, StringComparison.OrdinalIgnoreCase) ?? false);


@davidwengier Does this look right to you?

i'd like this answered as well.

Seems fine to me. We fill that project with documents from the sourceFileInfos, and check that its got files in it on line 243, so if this every fails, we would want a PRISM bug to tell us.

genlu · 2023-02-01T23:09:11Z

src/Features/CSharp/Portable/Snippets/CSharpConstructorSnippetProvider.cs

@@ -89,7 +89,7 @@ private static string GetIndentation(Document document, SyntaxNode node, SyntaxF
        protected override async Task<Document> AddIndentationToDocumentAsync(Document document, int position, ISyntaxFacts syntaxFacts, CancellationToken cancellationToken)
        {
            var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
-            var snippet = root.GetAnnotatedNodes(_findSnippetAnnotation).FirstOrDefault();
+            var snippet = root.GetAnnotatedNodes(_findSnippetAnnotation).First();


FYI @akhera99 for snippet related change

i don't trust this at all. @akhera99 how do you know you will absolutely find the node here?

I figured it would given that, when we annotate the added snippet, we throw if the added snippet can't be found. I think it's safer to keep it as FirstOrDefault, but I believe we bail out earlier in case it's not found.

tmat · 2023-02-02T03:53:18Z

src/Compilers/Core/Portable/InternalUtilities/CompilerFeatureRequiredAttribute.cs

@@ -5,7 +5,7 @@
 // Copied from:
 // https://github.com/dotnet/runtime/blob/fdd104ec5e1d0d2aa24a6723995a98d0124f724b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/CompilerFeatureRequiredAttribute.cs

-#if !NET6_0_OR_GREATER


Would #if NETSTANDARD be better, so that it is not updated every time we bump version?

seems this type is not in .net 6 either, only in net 7

Here's the runtime PR addign those types
dotnet/runtime#64287

tmat · 2023-02-02T03:53:26Z

src/Compilers/Core/Portable/InternalUtilities/RequiredMemberAttribute.cs

@@ -5,7 +5,7 @@
 // Copied from:
 // https://github.com/dotnet/runtime/blob/fdd104ec5e1d0d2aa24a6723995a98d0124f724b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RequiredMemberAttribute.cs

-#if !NET6_0_OR_GREATER


Would #if NETSTANDARD be better, so that it is not updated every time we bump version?

tmat · 2023-02-02T03:53:31Z

src/Compilers/Core/Portable/InternalUtilities/SetsRequiredMembersAttribute.cs

@@ -5,7 +5,7 @@
 // Copied from:
 // https://github.com/dotnet/runtime/blob/fdd104ec5e1d0d2aa24a6723995a98d0124f724b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/CodeAnalysis/SetsRequiredMembersAttribute.cs

-#if !NET6_0_OR_GREATER


Would #if NETSTANDARD be better, so that it is not updated every time we bump version?

tmat · 2023-02-02T03:55:45Z

src/EditorFeatures/Core/EditorConfigSettings/DataProvider/Analyzer/AnalyzerSettingsProvider.cs

@@ -28,7 +29,7 @@ public AnalyzerSettingsProvider(string fileName, AnalyzerSettingsUpdater setting

        protected override void UpdateOptions(TieredAnalyzerConfigOptions options, ImmutableArray<Project> projectsInScope)
        {
-            var analyzerReferences = projectsInScope.SelectMany(p => p.AnalyzerReferences).DistinctBy(a => a.Id).ToImmutableArray();
+            var analyzerReferences = RoslynEnumerableExtensions.DistinctBy(projectsInScope.SelectMany(p => p.AnalyzerReferences), a => a.Id).ToImmutableArray();


Do we actually need the extension method?

Maybe we can unify them to BCL implementation, but I'd keep the original logic in this PR

sharwell · 2023-02-02T17:40:23Z

src/Compilers/Core/Portable/InternalUtilities/CompilerFeatureRequiredAttribute.cs

@@ -5,7 +5,7 @@
 // Copied from:
 // https://github.com/dotnet/runtime/blob/fdd104ec5e1d0d2aa24a6723995a98d0124f724b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/CompilerFeatureRequiredAttribute.cs

-#if !NET6_0_OR_GREATER


There is no need to change this directive (now or in the future). net7.0 falls under NET6_0_OR_GREATER.

the problem is we are compiled for .NET6, which would exclude this from code, but the type only available in .NET7

src/Compilers/Core/Portable/InternalUtilities/PlatformInformation.cs

sharwell · 2023-02-02T17:46:14Z

src/Dependencies/Collections/Microsoft.CodeAnalysis.Collections.Package.csproj

@@ -2,7 +2,7 @@
 <!-- Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE file in the project root for more information. -->
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
-    <TargetFrameworks>net6.0;netcoreapp3.1;netstandard2.0</TargetFrameworks>
+    <TargetFrameworks>net6.0;netstandard2.0</TargetFrameworks>


❓ Is there a need to change this? This project is usable outside Roslyn. I'd prefer to only increase the version if the new version is providing some new feature/ability.

The reason for this is I'm also gonna bump our BCL reference to 7, which would cause error for project targeting 3.1

packages/system.text.encoding.codepages/7.0.0/buildTransitive/netcoreapp2.0/System.Text.Encoding.CodePages.targets(4,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) System.Text.Encoding.CodePages 7.0.0 doesn't support netcoreapp3.1 and has not been tested with it. Consider upgrading your TargetFramework to net6.0 or later. You may also set true in the project file to ignore this warning and attempt to run in this unsupported configuration at your own risk.

You may also set true in the project file to ignore this warning and attempt to run in this unsupported configuration at your own risk.

Can we set that something to true to ignore the error? This is a source package anyways, so it shouldn't matter.

Sure. I just wasn't aware of a reason that warrants suppressing the warning.

…ion.cs Co-authored-by: Sam Harwell <sam@tunnelvisionlabs.com>

src/Dependencies/Collections/Microsoft.CodeAnalysis.Collections.Package.csproj

…s.Package.csproj

genlu · 2023-02-07T01:24:14Z

@jaredpar @jcouv @dotnet/roslyn-compiler Please take a look. Thanks!

...harp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs

src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceCodeRefactoringProvider.cs

src/Features/CSharp/Portable/Snippets/CSharpElseSnippetProvider.cs

src/Features/Core/Portable/Snippets/SnippetProviders/AbstractSnippetProvider.cs

333fred

Compiler changes LGTM.

genlu · 2023-02-08T00:03:38Z

@CyrusNajmabadi @akhera99 Could you take another look please? Thanks!

akhera99 · 2023-02-08T00:17:46Z

Snippet changes look good

cston

Compiler changes LGTM.

src/Analyzers/CSharp/Analyzers/ConvertNamespace/ConvertNamespaceAnalysis.cs

src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceCodeRefactoringProvider.cs

src/Workspaces/Core/MSBuild/MSBuild/MSBuildWorkspace.cs

src/Workspaces/Core/Portable/Rename/ConflictEngine/MutableConflictResolution.cs

CyrusNajmabadi

signing off on most of the IDE changes. but there are outstanding questions for DavidW and JasonMal that need to be answered. Thanks :)

CyrusNajmabadi · 2023-02-09T18:13:23Z

but there are outstanding questions for DavidW and JasonMal that need to be answered.

:-/

sharwell · 2023-02-09T18:15:55Z

@CyrusNajmabadi the issues that led to those questions were reverted

davidwengier · 2023-02-09T22:34:34Z

I answered mine :P

Targeting IDE projects to .NET 6

5dda8cd

genlu requested review from a team as code owners February 1, 2023 21:32

dotnet-issue-labeler bot added the Area-IDE label Feb 1, 2023

genlu commented Feb 1, 2023

View reviewed changes

fix

97801df

genlu force-pushed the TargetNet6 branch from dc802c2 to 97801df Compare February 1, 2023 23:20

fix

a6a65c5

genlu force-pushed the TargetNet6 branch from bedc191 to a6a65c5 Compare February 2, 2023 00:02

fix

c349ce3

tmat reviewed Feb 2, 2023

View reviewed changes

sharwell reviewed Feb 2, 2023

View reviewed changes

src/Compilers/Core/Portable/InternalUtilities/PlatformInformation.cs Outdated Show resolved Hide resolved

sharwell reviewed Feb 2, 2023

View reviewed changes

Update src/Compilers/Core/Portable/InternalUtilities/PlatformInformat…

12483e6

…ion.cs Co-authored-by: Sam Harwell <sam@tunnelvisionlabs.com>

genlu commented Feb 2, 2023

View reviewed changes

src/Dependencies/Collections/Microsoft.CodeAnalysis.Collections.Package.csproj Outdated Show resolved Hide resolved

Update src/Dependencies/Collections/Microsoft.CodeAnalysis.Collection…

4eeebe5

…s.Package.csproj

jcouv added the Area-Compilers label Feb 7, 2023

CyrusNajmabadi reviewed Feb 7, 2023

View reviewed changes

...harp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Feb 7, 2023

View reviewed changes

src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceCodeRefactoringProvider.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Feb 7, 2023

View reviewed changes

src/Features/CSharp/Portable/Snippets/CSharpElseSnippetProvider.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Feb 7, 2023

View reviewed changes

src/Features/Core/Portable/Snippets/SnippetProviders/AbstractSnippetProvider.cs Outdated Show resolved Hide resolved

333fred approved these changes Feb 7, 2023

View reviewed changes

Address review comments

001d9dc

genlu added 2 commits February 7, 2023 15:51

Merge remote-tracking branch 'origin/TargetNet6' into TargetNet6

82f12d1

Merge remote-tracking branch 'dotnet/main' into TargetNet6

2940423

cston approved these changes Feb 8, 2023

View reviewed changes

Fix

f4f2ef8

genlu force-pushed the TargetNet6 branch from 2ac3b52 to f4f2ef8 Compare February 8, 2023 18:16

genlu requested a review from CyrusNajmabadi February 8, 2023 22:14