From bc0b404fbeed7a62a7052c57a07b3e4082a5d5e5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 May 2020 12:21:19 -0700 Subject: [PATCH 01/18] initial --- .../MetadataAsSource/MetadataAsSourceTests.cs | 5 +- .../AbstractMetadataAsSourceService.cs | 23 ------- .../CodeGeneration/AttributeGenerator.cs | 63 +++++++++++++++---- .../CSharpCodeGenerationHelpers.cs | 9 +++ .../CodeGeneration/TypeParameterGenerator.cs | 3 +- .../NullableSyntaxAnnotation.cs | 15 +++++ 6 files changed, 77 insertions(+), 41 deletions(-) create mode 100644 src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs index 559aa0ad569bd..93e805883971b 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs @@ -591,10 +591,7 @@ await GenerateAndVerifySourceAsync(metadataSource, symbolName, LanguageNames.CSh // {CodeAnalysisResources.InMemoryAssembly} #endregion -using System.Runtime.CompilerServices; - -[NullableContextAttribute(1)] -public interface [|C|]<[NullableAttribute(2)] T> +public interface [|C|] {{ bool Equals([AllowNullAttribute] T other); }}"); diff --git a/src/Features/Core/Portable/MetadataAsSource/AbstractMetadataAsSourceService.cs b/src/Features/Core/Portable/MetadataAsSource/AbstractMetadataAsSourceService.cs index b1e5be14d07b8..89f3d1183793d 100644 --- a/src/Features/Core/Portable/MetadataAsSource/AbstractMetadataAsSourceService.cs +++ b/src/Features/Core/Portable/MetadataAsSource/AbstractMetadataAsSourceService.cs @@ -41,8 +41,6 @@ public async Task AddSourceToAsync(Document document, Compilation symb CreateCodeGenerationOptions(newSemanticModel.SyntaxTree.GetLocation(new TextSpan()), symbol), cancellationToken).ConfigureAwait(false); - document = await RemoveSimplifierAnnotationsFromImportsAsync(document, cancellationToken).ConfigureAwait(false); - var docCommentFormattingService = document.GetLanguageService(); var docWithDocComments = await ConvertDocCommentsToRegularCommentsAsync(document, docCommentFormattingService, cancellationToken).ConfigureAwait(false); @@ -55,27 +53,6 @@ public async Task AddSourceToAsync(Document document, Compilation symb return await Simplifier.ReduceAsync(formattedDoc, reducers, null, cancellationToken).ConfigureAwait(false); } - /// - /// adds to Import Directives it adds, - /// which causes the to remove import directives when thety are only used by attributes. - /// Presumably this is because MetadataAsSource isn't actually semantically valid code. - /// - /// To fix this we remove these annotations. - /// - private static async Task RemoveSimplifierAnnotationsFromImportsAsync(Document document, CancellationToken cancellationToken) - { - var syntaxFacts = document.GetLanguageService(); - - var importDirectives = (await document.GetSyntaxRootAsync().ConfigureAwait(false)) - .DescendantNodesAndSelf() - .Where(syntaxFacts.IsUsingOrExternOrImport); - - return await document.ReplaceNodesAsync( - importDirectives, - (o, c) => c.WithoutAnnotations(Simplifier.Annotation), - cancellationToken).ConfigureAwait(false); - } - /// /// provide formatting rules to be used when formatting MAS file /// diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs index a1d87442510a1..d6a6600726123 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -16,53 +17,89 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeGeneration { internal static class AttributeGenerator { - public static SyntaxList GenerateAttributeLists( + public static (SyntaxList, bool isNullable) GenerateAttributeLists( ImmutableArray attributes, CodeGenerationOptions options, SyntaxToken? target = null) { if (options.MergeAttributes) { - var attributeNodes = attributes.OrderBy(a => a.AttributeClass.Name).Select(a => GenerateAttribute(a, options)).WhereNotNull().ToList(); - return attributeNodes.Count == 0 + var pairs = attributes.OrderBy(a => a.AttributeClass.Name).Select(a => GenerateAttribute(a, options)).ToList(); + var isNullable = pairs.Any(t => t.isNullable); + var attributeNodes = pairs.Select(p => p.syntax).WhereNotNull().ToList(); + + var list = attributeNodes.Count == 0 ? default : SyntaxFactory.SingletonList(SyntaxFactory.AttributeList( target.HasValue ? SyntaxFactory.AttributeTargetSpecifier(target.Value) : null, SyntaxFactory.SeparatedList(attributeNodes))); + return (list, isNullable); } else { - var attributeDeclarations = attributes.OrderBy(a => a.AttributeClass.Name).Select(a => GenerateAttributeDeclaration(a, target, options)).WhereNotNull().ToList(); - return attributeDeclarations.Count == 0 - ? default - : SyntaxFactory.List(attributeDeclarations); + var pairs = attributes.OrderBy(a => a.AttributeClass.Name).Select(a => GenerateAttributeDeclaration(a, target, options)).ToList(); + var isNullable = pairs.Any(t => t.isNullable); + + var list = SyntaxFactory.List(pairs.Select(t => t.syntax).WhereNotNull()); + return (list, isNullable); } } - private static AttributeListSyntax GenerateAttributeDeclaration( + private static (AttributeListSyntax syntax, bool isNullable) GenerateAttributeDeclaration( AttributeData attribute, SyntaxToken? target, CodeGenerationOptions options) { - var attributeSyntax = GenerateAttribute(attribute, options); - return attributeSyntax == null + var (attributeSyntax, isNullable) = GenerateAttribute(attribute, options); + var resultSyntax = attributeSyntax == null ? null : SyntaxFactory.AttributeList( target.HasValue ? SyntaxFactory.AttributeTargetSpecifier(target.Value) : null, SyntaxFactory.SingletonSeparatedList(attributeSyntax)); + + return (resultSyntax, isNullable); } - private static AttributeSyntax GenerateAttribute(AttributeData attribute, CodeGenerationOptions options) + private static (AttributeSyntax syntax, bool isNullable) GenerateAttribute(AttributeData attribute, CodeGenerationOptions options) { + NullableAnnotation + // Never add the internal nullable attributes the compiler generates. + if (IsCompilerInternalNulllableAttribute(attribute)) + return (null, isNullable: true); + if (!options.MergeAttributes) { var reusableSyntax = GetReuseableSyntaxNodeForAttribute(attribute, options); if (reusableSyntax != null) { - return reusableSyntax; + return (reusableSyntax, isNullable: false); } } var attributeArguments = GenerateAttributeArgumentList(attribute); - return !(attribute.AttributeClass.GenerateTypeSyntax() is NameSyntax nameSyntax) ? null : SyntaxFactory.Attribute(nameSyntax, attributeArguments); + var syntax = !(attribute.AttributeClass.GenerateTypeSyntax() is NameSyntax nameSyntax) ? null : SyntaxFactory.Attribute(nameSyntax, attributeArguments); + + return (syntax, isNullable: false); + } + + private static bool IsCompilerInternalNulllableAttribute(AttributeData attribute) + { + // from https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-metadata.md + var attrClass = attribute.AttributeClass; + var name = attrClass.Name; + + if (name != "NullableAttribute" && name != "NullableContextAttribute") + return false; + + + var ns = attrClass.ContainingNamespace; + return ns?.Name == nameof(System.Runtime.CompilerServices) && + ns.ContainingNamespace?.Name == nameof(System.Runtime) && + ns.ContainingNamespace.ContainingNamespace?.Name == nameof(System) && + ns.ContainingNamespace.ContainingNamespace.ContainingNamespace?.IsGlobalNamespace == true; + } + + private static bool IsSystemRuntimeCompilerServicesNamespace(INamespaceSymbol containingNamespace) + { + throw new NotImplementedException(); } private static AttributeArgumentListSyntax GenerateAttributeArgumentList(AttributeData attribute) diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpCodeGenerationHelpers.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpCodeGenerationHelpers.cs index b0adf9fcfdb7c..62049bf48cb79 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpCodeGenerationHelpers.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpCodeGenerationHelpers.cs @@ -30,6 +30,15 @@ public static TDeclarationSyntax ConditionallyAddFormattingAnnotationTo( + TDeclarationSyntax result, + bool isNullable) where TDeclarationSyntax : SyntaxNode + { + return isNullable + ? result.WithAdditionalAnnotations(NullableAnnotation.Instance) + : result; + } + internal static void AddAccessibilityModifiers( Accessibility accessibility, ArrayBuilder tokens, diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/TypeParameterGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/TypeParameterGenerator.cs index f996df0594046..8c0dd1d29deb7 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/TypeParameterGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/TypeParameterGenerator.cs @@ -27,8 +27,9 @@ private static TypeParameterSyntax GenerateTypeParameter(ITypeParameterSymbol sy symbol.Variance == VarianceKind.In ? SyntaxFactory.Token(SyntaxKind.InKeyword) : symbol.Variance == VarianceKind.Out ? SyntaxFactory.Token(SyntaxKind.OutKeyword) : default; + var (attributes, isNullable) = AttributeGenerator.GenerateAttributeLists(symbol.GetAttributes(), options) return SyntaxFactory.TypeParameter( - AttributeGenerator.GenerateAttributeLists(symbol.GetAttributes(), options), + attributes, varianceKeyword, symbol.Name.ToIdentifierToken()); } diff --git a/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs b/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs new file mode 100644 index 0000000000000..4d53de64e6cc7 --- /dev/null +++ b/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs @@ -0,0 +1,15 @@ +// 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. + +namespace Microsoft.CodeAnalysis.CodeGeneration +{ + /// + /// Annotation placed if the code generator encounters a NullableAttribute or NullableContextAttribute while + /// generating the code. + /// + internal sealed class NullableSyntaxAnnotation + { + public static readonly SyntaxAnnotation Instance = new SyntaxAnnotation(nameof(NullableAnnotation)); + } +} From 3164a4f251a9bbf1ca13812c54a8590e9afcbcad Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 May 2020 14:48:30 -0700 Subject: [PATCH 02/18] Generate #nullable --- .../MetadataAsSourceTests.CSharp.cs | 6 - .../MetadataAsSourceTests.VisualBasic.cs | 3 - .../MetadataAsSource/MetadataAsSourceTests.cs | 71 +++++-- .../CSharpMetadataAsSourceService.cs | 195 ++++++++++++++---- .../MetadataAsSource/FormattingRule.cs | 67 ++++++ .../AbstractMetadataAsSourceService.cs | 6 +- .../VisualBasicMetadataAsSourceService.vb | 6 + .../CodeGeneration/AttributeGenerator.cs | 77 ++++--- .../CSharpCodeGenerationHelpers.cs | 9 - .../CodeGeneration/TypeParameterGenerator.cs | 3 +- .../CSharpSimplificationService.cs | 1 - .../NullableSyntaxAnnotation.cs | 4 +- .../ITypeParameterSymbolExtensions.cs | 9 +- .../Extensions/ITypeSymbolExtensions.cs | 20 +- 14 files changed, 357 insertions(+), 120 deletions(-) create mode 100644 src/Features/CSharp/Portable/MetadataAsSource/FormattingRule.cs diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.CSharp.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.CSharp.cs index c4bba9a44689b..d0571c8d0e34e 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.CSharp.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.CSharp.cs @@ -43,14 +43,10 @@ await GenerateAndVerifySourceAsync(metadataSource, symbolName, LanguageNames.CSh // {CodeAnalysisResources.InMemoryAssembly} #endregion -using System; -using System.Runtime.CompilerServices; public class [|C|] {{ - [NativeIntegerAttribute] public nint i; - [NativeIntegerAttribute] public nuint i2; public C(); @@ -88,8 +84,6 @@ await context.GenerateAndVerifySourceAsync("System.ValueTuple", // System.ValueTuple.dll #endregion -using System; -using System; using System.Collections; namespace System diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.VisualBasic.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.VisualBasic.cs index 0287feff0e964..1843c64efa49f 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.VisualBasic.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.VisualBasic.cs @@ -44,7 +44,6 @@ public async Task BracketedIdentifierSimplificationTest() ' mscorlib.v4_6_1038_0.dll #End Region -Imports System Imports System.Runtime.InteropServices Namespace System @@ -95,8 +94,6 @@ await context.GenerateAndVerifySourceAsync("System.ValueTuple", ' System.ValueTuple.dll #End Region -Imports System -Imports System Imports System.Collections Namespace System diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs index 93e805883971b..ff422cf709b08 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs @@ -599,7 +599,6 @@ await GenerateAndVerifySourceAsync(metadataSource, symbolName, LanguageNames.Vis ' {CodeAnalysisResources.InMemoryAssembly} #End Region -Imports System.Runtime.CompilerServices Public Interface [|C|](Of T) @@ -1701,7 +1700,6 @@ await GenerateAndVerifySourceAsync(metadataSource, symbolName, LanguageNames.Vis ' {CodeAnalysisResources.InMemoryAssembly} #End Region -Imports System.Runtime.CompilerServices Public Structure [|S|] @@ -1763,7 +1761,6 @@ await GenerateAndVerifySourceAsync(metadataSource, symbolName, LanguageNames.Vis #End Region Imports System -Imports System.Runtime.CompilerServices Public Structure [|S|] @@ -1794,7 +1791,6 @@ await GenerateAndVerifySourceAsync(metadataSource, symbolName, LanguageNames.Vis #End Region Imports System -Imports System.Runtime.CompilerServices Public Structure [|S|] @@ -1829,7 +1825,6 @@ await GenerateAndVerifySourceAsync(metadataSource, symbolName, LanguageNames.Vis ' {CodeAnalysisResources.InMemoryAssembly} #End Region -Imports System.Runtime.CompilerServices Public Structure S Public Sub [|M|]() @@ -1864,7 +1859,6 @@ await GenerateAndVerifySourceAsync(metadataSource, symbolName, LanguageNames.Vis ' {CodeAnalysisResources.InMemoryAssembly} #End Region -Imports System.Runtime.CompilerServices Public Structure S @@ -1995,7 +1989,6 @@ await GenerateAndVerifySourceAsync(metadataSource, symbolName, LanguageNames.Vis ' {CodeAnalysisResources.InMemoryAssembly} #End Region -Imports System.Runtime.CompilerServices Public Structure S @@ -2241,7 +2234,6 @@ await GenerateAndVerifySourceAsync(metadataSource, symbolName, LanguageNames.Vis #End Region Imports System -Imports System.Runtime.CompilerServices Public Structure S @@ -2268,6 +2260,8 @@ void M() // {CodeAnalysisResources.InMemoryAssembly} #endregion +#nullable enable + using System.Runtime.CompilerServices; public class [|TestType|]<[NullableAttribute(1)] T> where T : notnull @@ -2310,13 +2304,12 @@ void M() // {CodeAnalysisResources.InMemoryAssembly} #endregion -using System.Runtime.CompilerServices; +#nullable enable public class TestType {{ public TestType(); - [NullableContextAttribute(1)] public void [|M|]() where T : notnull; }}"; @@ -2349,9 +2342,63 @@ class C // {CodeAnalysisResources.InMemoryAssembly} #endregion -using System.Runtime.CompilerServices; +#nullable enable + +public delegate void [|D|]() where T : notnull;"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable1() + { + var metadata = @" +#nullable enable + +using System; + +public class TestType +{ + public void M1(string) + { + } + +#nullable disable + + public void M(string) + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|]<int>(); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion -public delegate void [|D|]<[NullableAttribute(1)] T>() where T : notnull;"; +#nullable enable + +public class TestType +{{ + public TestType(); + + public void [|M|]() where T : notnull; +}}"; using var context = TestContext.Create( LanguageNames.CSharp, diff --git a/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs b/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs index 2133e2705b58e..803f84f18f0be 100644 --- a/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs +++ b/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs @@ -7,19 +7,23 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeGeneration; using Microsoft.CodeAnalysis.CSharp.DocumentationComments; using Microsoft.CodeAnalysis.CSharp.Simplification; -using Microsoft.CodeAnalysis.CSharp.Utilities; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.DocumentationComments; using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Formatting.Rules; using Microsoft.CodeAnalysis.MetadataAsSource; +using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Simplification; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.MetadataAsSource { - internal class CSharpMetadataAsSourceService : AbstractMetadataAsSourceService + internal partial class CSharpMetadataAsSourceService : AbstractMetadataAsSourceService { private static readonly AbstractFormattingRule s_memberSeparationRule = new FormattingRule(); public static readonly CSharpMetadataAsSourceService Instance = new CSharpMetadataAsSourceService(); @@ -37,16 +41,14 @@ protected override async Task AddAssemblyInfoRegionAsync(Document docu .WithTrailingTrivia(new[] { SyntaxFactory.Space, SyntaxFactory.PreprocessingMessage(assemblyInfo) }); var oldRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var newRoot = oldRoot.WithLeadingTrivia(new[] - { - SyntaxFactory.Trivia(regionTrivia), - SyntaxFactory.CarriageReturnLineFeed, - SyntaxFactory.Comment("// " + assemblyPath), - SyntaxFactory.CarriageReturnLineFeed, - SyntaxFactory.Trivia(SyntaxFactory.EndRegionDirectiveTrivia(true)), - SyntaxFactory.CarriageReturnLineFeed, - SyntaxFactory.CarriageReturnLineFeed - }); + var newRoot = oldRoot.WithPrependedLeadingTrivia( + SyntaxFactory.Trivia(regionTrivia), + SyntaxFactory.CarriageReturnLineFeed, + SyntaxFactory.Comment("// " + assemblyPath), + SyntaxFactory.CarriageReturnLineFeed, + SyntaxFactory.Trivia(SyntaxFactory.EndRegionDirectiveTrivia(true)), + SyntaxFactory.CarriageReturnLineFeed, + SyntaxFactory.CarriageReturnLineFeed); return document.WithSyntaxRoot(newRoot); } @@ -71,55 +73,162 @@ protected override ImmutableArray GetReducers() new CSharpParenthesizedPatternReducer(), new CSharpDefaultExpressionReducer()); - private class FormattingRule : AbstractMetadataFormattingRule + /// + /// Adds #nullable enable and #nullable disable annotations to the file as necessary. Note that + /// this does not try to be 100% accurate, but rather it handles the most common cases out there. Specifically, + /// if a file contains any nullable annotated/not-annotated types, then we prefix the file with #nullable + /// enable. Then if we hit any members that explicitly have *oblivious* types, not no annotated or + /// non-annotated types, then we switch to #nullable disable for those specific members. + /// + /// This is technically innacurate for possible, but very uncommon cases. For example, if the user's code + /// explicitly did something like this: + /// + /// + /// public void Goo(string goo, + /// #nullable disable + /// string bar + /// #nullable enable + /// string baz); + /// + /// + /// Then we would be unable to handle that. However, this is highly unlikely to happen, and so we accept the + /// inaccuracy for the purpose of simplicity and for handling the much more common cases of either the entire + /// file being annotated, or the user individually disabling annotations at the member level. + /// + protected override async Task AddNullableRegionsAsync(Document document, CancellationToken cancellationToken) + { + var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); + var options = (CSharpParseOptions)tree.Options; + + // Only valid for C# 8 and above. + if (options.LanguageVersion < LanguageVersion.CSharp8) + return document; + + var root = await tree.GetRootAsync(cancellationToken).ConfigureAwait(false); + + var (_, annotated, notAnnotated) = GetNullableAnnotations(root); + + // If there are no annotated or not-annotated types, then no need to add `#nullable enable`. + if (!annotated && !notAnnotated) + return document; + + var newRoot = AddNullableRegions(root, cancellationToken); + newRoot = newRoot.WithPrependedLeadingTrivia(CreateNullableTrivia(enable: true)); + + return document.WithSyntaxRoot(newRoot); + } + + private (bool oblivious, bool annotated, bool notAnnotated) GetNullableAnnotations(SyntaxNode node) { - protected override AdjustNewLinesOperation GetAdjustNewLinesOperationBetweenMembersAndUsings(SyntaxToken token1, SyntaxToken token2) + return (HasAnnotation(node, NullableSyntaxAnnotation.None), + HasAnnotation(node, NullableSyntaxAnnotation.Annotated), + HasAnnotation(node, NullableSyntaxAnnotation.NotAnnotated)); + } + + private bool HasAnnotation(SyntaxNode node, SyntaxAnnotation annotation) + { + // see if any child nodes have this annotation. Ignore anything in attributes (like `[Obsolete]void Goo()` + // as these are not impacted by `#nullable` regions. Instead, we only care about signature types. + var annotatedChildren = node.GetAnnotatedNodes(annotation); + return annotatedChildren.Any(n => n.GetAncestorOrThis() == null); + } + + private static SyntaxTrivia[] CreateNullableTrivia(bool enable) + { + var keyword = enable ? SyntaxKind.EnableKeyword : SyntaxKind.DisableKeyword; + return new[] { - var previousToken = token1; - var currentToken = token2; + SyntaxFactory.Trivia(SyntaxFactory.NullableDirectiveTrivia(SyntaxFactory.Token(keyword), isActive: enable)), + SyntaxFactory.ElasticCarriageReturnLineFeed, + SyntaxFactory.ElasticCarriageReturnLineFeed, + }; + } - // We are not between members or usings if the last token wasn't the end of a statement or if the current token - // is the end of a scope. - if ((previousToken.Kind() != SyntaxKind.SemicolonToken && previousToken.Kind() != SyntaxKind.CloseBraceToken) || - currentToken.Kind() == SyntaxKind.CloseBraceToken) + private SyntaxNode AddNullableRegions(SyntaxNode node, CancellationToken cancellationToken) + { + return node switch + { + CompilationUnitSyntax compilationUnit => compilationUnit.WithMembers(AddNullableRegions(compilationUnit.Members, cancellationToken)), + NamespaceDeclarationSyntax ns => ns.WithMembers(AddNullableRegions(ns.Members, cancellationToken)), + TypeDeclarationSyntax type => AddNullableRegionsAroundTypeMembers(type, cancellationToken), + _ => node, + }; + } + + private SyntaxList AddNullableRegions( + SyntaxList members, + CancellationToken cancellationToken) + { + using var _ = ArrayBuilder.GetInstance(out var builder); + + foreach (var member in members) + builder.Add((MemberDeclarationSyntax)AddNullableRegions(member, cancellationToken)); + + return SyntaxFactory.List(builder); + } + + private TypeDeclarationSyntax AddNullableRegionsAroundTypeMembers( + TypeDeclarationSyntax type, CancellationToken cancellationToken) + { + using var _ = ArrayBuilder.GetInstance(out var builder); + + var currentlyEnabled = true; + + foreach (var member in type.Members) + { + if (member is BaseTypeDeclarationSyntax) { - return null; + // if we hit a type, and we're currently disabled, then switch us back to enabled for that type. + // This ensures whenever we walk into a type-decl, we're always in the enabled-state. + builder.Add(TransitionTo(member, enabled: true, ref currentlyEnabled)); + continue; } - SyntaxNode previousMember = FormattingRangeHelper.GetEnclosingMember(previousToken); - SyntaxNode nextMember = FormattingRangeHelper.GetEnclosingMember(currentToken); + // we hit a member. see what sort of types it contained. + var (oblivious, annotated, notAnnotated) = GetNullableAnnotations(member); - // Is the previous statement an using directive? If so, treat it like a member to add - // the right number of lines. - if (previousToken.Kind() == SyntaxKind.SemicolonToken && previousToken.Parent.Kind() == SyntaxKind.UsingDirective) + // if we have null annotations, transition us back to the enabled state + if (annotated || notAnnotated) { - previousMember = previousToken.Parent; + builder.Add(TransitionTo(member, enabled: true, ref currentlyEnabled)); } - - if (previousMember == null || nextMember == null || previousMember == nextMember) + else if (oblivious) { - return null; + // if we didn't have null annotations, and we had an explicit oblivious type, + // then definitely transition us to the disabled state + builder.Add(TransitionTo(member, enabled: false, ref currentlyEnabled)); } - - // If we have two members of the same kind, we won't insert a blank line - if (previousMember.Kind() == nextMember.Kind()) + else { - return FormattingOperations.CreateAdjustNewLinesOperation(1, AdjustNewLinesOption.ForceLines); + // had no types at all. no need to change state. + builder.Add(member); } - - // Force a blank line between the two nodes by counting the number of lines of - // trivia and adding one to it. - var triviaList = token1.TrailingTrivia.Concat(token2.LeadingTrivia); - return FormattingOperations.CreateAdjustNewLinesOperation(GetNumberOfLines(triviaList) + 1, AdjustNewLinesOption.ForceLines); } - public override void AddAnchorIndentationOperations(List list, SyntaxNode node, in NextAnchorIndentationOperationAction nextOperation) + var result = type.WithMembers(SyntaxFactory.List(builder)); + if (!currentlyEnabled) { - return; + // switch us back to enabled as we leave the type. + result = result.WithCloseBraceToken( + result.CloseBraceToken.WithPrependedLeadingTrivia(CreateNullableTrivia(enable: true))); } - protected override bool IsNewLine(char c) - => SyntaxFacts.IsNewLine(c); + return result; + } + + private MemberDeclarationSyntax TransitionTo(MemberDeclarationSyntax member, bool enabled, ref bool currentlyEnabled) + { + if (enabled == currentlyEnabled) + { + // already in the right state. don't start a #nullable region + return member; + } + else + { + // switch to the desired state and add the right trivia to the node. + currentlyEnabled = enabled; + return member.WithPrependedLeadingTrivia(CreateNullableTrivia(currentlyEnabled)); + } } } } diff --git a/src/Features/CSharp/Portable/MetadataAsSource/FormattingRule.cs b/src/Features/CSharp/Portable/MetadataAsSource/FormattingRule.cs new file mode 100644 index 0000000000000..0920547925e38 --- /dev/null +++ b/src/Features/CSharp/Portable/MetadataAsSource/FormattingRule.cs @@ -0,0 +1,67 @@ +// 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. + +using System.Collections.Generic; +using System.Linq; +using Microsoft.CodeAnalysis.CSharp.Utilities; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Formatting.Rules; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.CSharp.MetadataAsSource +{ + internal partial class CSharpMetadataAsSourceService + { + private class FormattingRule : AbstractMetadataFormattingRule + { + protected override AdjustNewLinesOperation GetAdjustNewLinesOperationBetweenMembersAndUsings(SyntaxToken token1, SyntaxToken token2) + { + var previousToken = token1; + var currentToken = token2; + + // We are not between members or usings if the last token wasn't the end of a statement or if the current token + // is the end of a scope. + if ((previousToken.Kind() != SyntaxKind.SemicolonToken && previousToken.Kind() != SyntaxKind.CloseBraceToken) || + currentToken.Kind() == SyntaxKind.CloseBraceToken) + { + return null; + } + + SyntaxNode previousMember = FormattingRangeHelper.GetEnclosingMember(previousToken); + SyntaxNode nextMember = FormattingRangeHelper.GetEnclosingMember(currentToken); + + // Is the previous statement an using directive? If so, treat it like a member to add + // the right number of lines. + if (previousToken.Kind() == SyntaxKind.SemicolonToken && previousToken.Parent.Kind() == SyntaxKind.UsingDirective) + { + previousMember = previousToken.Parent; + } + + if (previousMember == null || nextMember == null || previousMember == nextMember) + { + return null; + } + + // If we have two members of the same kind, we won't insert a blank line + if (previousMember.Kind() == nextMember.Kind()) + { + return FormattingOperations.CreateAdjustNewLinesOperation(1, AdjustNewLinesOption.ForceLines); + } + + // Force a blank line between the two nodes by counting the number of lines of + // trivia and adding one to it. + var triviaList = token1.TrailingTrivia.Concat(token2.LeadingTrivia); + return FormattingOperations.CreateAdjustNewLinesOperation(GetNumberOfLines(triviaList) + 1, AdjustNewLinesOption.ForceLines); + } + + public override void AddAnchorIndentationOperations(List list, SyntaxNode node, in NextAnchorIndentationOperationAction nextOperation) + { + return; + } + + protected override bool IsNewLine(char c) + => SyntaxFacts.IsNewLine(c); + } + } +} diff --git a/src/Features/Core/Portable/MetadataAsSource/AbstractMetadataAsSourceService.cs b/src/Features/Core/Portable/MetadataAsSource/AbstractMetadataAsSourceService.cs index 89f3d1183793d..4ff18c0060eb2 100644 --- a/src/Features/Core/Portable/MetadataAsSource/AbstractMetadataAsSourceService.cs +++ b/src/Features/Core/Portable/MetadataAsSource/AbstractMetadataAsSourceService.cs @@ -5,12 +5,10 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeGeneration; using Microsoft.CodeAnalysis.DocumentationComments; -using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Formatting.Rules; using Microsoft.CodeAnalysis.LanguageServices; @@ -41,6 +39,8 @@ public async Task AddSourceToAsync(Document document, Compilation symb CreateCodeGenerationOptions(newSemanticModel.SyntaxTree.GetLocation(new TextSpan()), symbol), cancellationToken).ConfigureAwait(false); + document = await AddNullableRegionsAsync(document, cancellationToken).ConfigureAwait(false); + var docCommentFormattingService = document.GetLanguageService(); var docWithDocComments = await ConvertDocCommentsToRegularCommentsAsync(document, docCommentFormattingService, cancellationToken).ConfigureAwait(false); @@ -53,6 +53,8 @@ public async Task AddSourceToAsync(Document document, Compilation symb return await Simplifier.ReduceAsync(formattedDoc, reducers, null, cancellationToken).ConfigureAwait(false); } + protected abstract Task AddNullableRegionsAsync(Document document, CancellationToken cancellationToken); + /// /// provide formatting rules to be used when formatting MAS file /// diff --git a/src/Features/VisualBasic/Portable/MetadataAsSource/VisualBasicMetadataAsSourceService.vb b/src/Features/VisualBasic/Portable/MetadataAsSource/VisualBasicMetadataAsSourceService.vb index 0d4f35ec180ec..03593e0166d19 100644 --- a/src/Features/VisualBasic/Portable/MetadataAsSource/VisualBasicMetadataAsSourceService.vb +++ b/src/Features/VisualBasic/Portable/MetadataAsSource/VisualBasicMetadataAsSourceService.vb @@ -4,6 +4,7 @@ Imports System.Collections.Immutable Imports System.Threading +Imports Microsoft.CodeAnalysis Imports Microsoft.CodeAnalysis.DocumentationComments Imports Microsoft.CodeAnalysis.Formatting Imports Microsoft.CodeAnalysis.Formatting.Rules @@ -49,6 +50,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.MetadataAsSource Return document.WithSyntaxRoot(newRoot) End Function + Protected Overrides Function AddNullableRegionsAsync(document As Document, cancellationToken As CancellationToken) As Task(Of Document) + ' VB has no equivalent to #nullable enable + Return Task.FromResult(document) + End Function + Protected Overrides Async Function ConvertDocCommentsToRegularCommentsAsync(document As Document, docCommentFormattingService As IDocumentationCommentFormattingService, cancellationToken As CancellationToken) As Task(Of Document) Dim syntaxRoot = Await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(False) diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs index d6a6600726123..0c864e30159b7 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs @@ -2,7 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; +#nullable enable + using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -17,79 +18,84 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeGeneration { internal static class AttributeGenerator { - public static (SyntaxList, bool isNullable) GenerateAttributeLists( + public static SyntaxList GenerateAttributeLists( ImmutableArray attributes, CodeGenerationOptions options, SyntaxToken? target = null) { if (options.MergeAttributes) { - var pairs = attributes.OrderBy(a => a.AttributeClass.Name).Select(a => GenerateAttribute(a, options)).ToList(); - var isNullable = pairs.Any(t => t.isNullable); - var attributeNodes = pairs.Select(p => p.syntax).WhereNotNull().ToList(); - - var list = attributeNodes.Count == 0 + var attributeNodes = + attributes.OrderBy(a => a.AttributeClass?.Name) + .Select(a => TryGenerateAttribute(a, options)) + .WhereNotNull().ToList(); + return attributeNodes.Count == 0 ? default : SyntaxFactory.SingletonList(SyntaxFactory.AttributeList( target.HasValue ? SyntaxFactory.AttributeTargetSpecifier(target.Value) : null, SyntaxFactory.SeparatedList(attributeNodes))); - return (list, isNullable); } else { - var pairs = attributes.OrderBy(a => a.AttributeClass.Name).Select(a => GenerateAttributeDeclaration(a, target, options)).ToList(); - var isNullable = pairs.Any(t => t.isNullable); - - var list = SyntaxFactory.List(pairs.Select(t => t.syntax).WhereNotNull()); - return (list, isNullable); + var attributeDeclarations = + attributes.OrderBy(a => a.AttributeClass?.Name) + .Select(a => TryGenerateAttributeDeclaration(a, target, options)) + .WhereNotNull().ToList(); + return attributeDeclarations.Count == 0 + ? default + : SyntaxFactory.List(attributeDeclarations); } } - private static (AttributeListSyntax syntax, bool isNullable) GenerateAttributeDeclaration( + private static AttributeListSyntax? TryGenerateAttributeDeclaration( AttributeData attribute, SyntaxToken? target, CodeGenerationOptions options) { - var (attributeSyntax, isNullable) = GenerateAttribute(attribute, options); - var resultSyntax = attributeSyntax == null + var attributeSyntax = TryGenerateAttribute(attribute, options); + return attributeSyntax == null ? null : SyntaxFactory.AttributeList( - target.HasValue ? SyntaxFactory.AttributeTargetSpecifier(target.Value) : null, + target.HasValue + ? SyntaxFactory.AttributeTargetSpecifier(target.Value) + : null, SyntaxFactory.SingletonSeparatedList(attributeSyntax)); - - return (resultSyntax, isNullable); } - private static (AttributeSyntax syntax, bool isNullable) GenerateAttribute(AttributeData attribute, CodeGenerationOptions options) + private static AttributeSyntax? TryGenerateAttribute(AttributeData attribute, CodeGenerationOptions options) { - NullableAnnotation - // Never add the internal nullable attributes the compiler generates. - if (IsCompilerInternalNulllableAttribute(attribute)) - return (null, isNullable: true); + if (IsCompilerInternalAttribute(attribute)) + return null; if (!options.MergeAttributes) { var reusableSyntax = GetReuseableSyntaxNodeForAttribute(attribute, options); if (reusableSyntax != null) { - return (reusableSyntax, isNullable: false); + return reusableSyntax; } } - var attributeArguments = GenerateAttributeArgumentList(attribute); - var syntax = !(attribute.AttributeClass.GenerateTypeSyntax() is NameSyntax nameSyntax) ? null : SyntaxFactory.Attribute(nameSyntax, attributeArguments); + if (attribute.AttributeClass == null) + return null; - return (syntax, isNullable: false); + var attributeArguments = GenerateAttributeArgumentList(attribute); + return attribute.AttributeClass.GenerateTypeSyntax() is NameSyntax nameSyntax + ? SyntaxFactory.Attribute(nameSyntax, attributeArguments) + : null; } - private static bool IsCompilerInternalNulllableAttribute(AttributeData attribute) + + private static bool IsCompilerInternalAttribute(AttributeData attribute) { // from https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-metadata.md var attrClass = attribute.AttributeClass; + if (attrClass == null) + return false; + var name = attrClass.Name; - if (name != "NullableAttribute" && name != "NullableContextAttribute") + if (name != "NullableAttribute" && name != "NullableContextAttribute" && name != "NativeIntegerAttribute") return false; - var ns = attrClass.ContainingNamespace; return ns?.Name == nameof(System.Runtime.CompilerServices) && ns.ContainingNamespace?.Name == nameof(System.Runtime) && @@ -97,17 +103,10 @@ private static bool IsCompilerInternalNulllableAttribute(AttributeData attribute ns.ContainingNamespace.ContainingNamespace.ContainingNamespace?.IsGlobalNamespace == true; } - private static bool IsSystemRuntimeCompilerServicesNamespace(INamespaceSymbol containingNamespace) - { - throw new NotImplementedException(); - } - - private static AttributeArgumentListSyntax GenerateAttributeArgumentList(AttributeData attribute) + private static AttributeArgumentListSyntax? GenerateAttributeArgumentList(AttributeData attribute) { if (attribute.ConstructorArguments.Length == 0 && attribute.NamedArguments.Length == 0) - { return null; - } var arguments = new List(); arguments.AddRange(attribute.ConstructorArguments.Select(c => diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpCodeGenerationHelpers.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpCodeGenerationHelpers.cs index 62049bf48cb79..b0adf9fcfdb7c 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpCodeGenerationHelpers.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpCodeGenerationHelpers.cs @@ -30,15 +30,6 @@ public static TDeclarationSyntax ConditionallyAddFormattingAnnotationTo( - TDeclarationSyntax result, - bool isNullable) where TDeclarationSyntax : SyntaxNode - { - return isNullable - ? result.WithAdditionalAnnotations(NullableAnnotation.Instance) - : result; - } - internal static void AddAccessibilityModifiers( Accessibility accessibility, ArrayBuilder tokens, diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/TypeParameterGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/TypeParameterGenerator.cs index 8c0dd1d29deb7..f996df0594046 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/TypeParameterGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/TypeParameterGenerator.cs @@ -27,9 +27,8 @@ private static TypeParameterSyntax GenerateTypeParameter(ITypeParameterSymbol sy symbol.Variance == VarianceKind.In ? SyntaxFactory.Token(SyntaxKind.InKeyword) : symbol.Variance == VarianceKind.Out ? SyntaxFactory.Token(SyntaxKind.OutKeyword) : default; - var (attributes, isNullable) = AttributeGenerator.GenerateAttributeLists(symbol.GetAttributes(), options) return SyntaxFactory.TypeParameter( - attributes, + AttributeGenerator.GenerateAttributeLists(symbol.GetAttributes(), options), varianceKeyword, symbol.Name.ToIdentifierToken()); } diff --git a/src/Workspaces/CSharp/Portable/Simplification/CSharpSimplificationService.cs b/src/Workspaces/CSharp/Portable/Simplification/CSharpSimplificationService.cs index c9a2f04d2a377..f39761e71c76a 100644 --- a/src/Workspaces/CSharp/Portable/Simplification/CSharpSimplificationService.cs +++ b/src/Workspaces/CSharp/Portable/Simplification/CSharpSimplificationService.cs @@ -183,7 +183,6 @@ protected override void GetUnusedNamespaceImports(SemanticModel model, HashSet internal sealed class NullableSyntaxAnnotation { - public static readonly SyntaxAnnotation Instance = new SyntaxAnnotation(nameof(NullableAnnotation)); + public static readonly SyntaxAnnotation None = new SyntaxAnnotation($"{nameof(NullableAnnotation)}.{NullableAnnotation.None}"); + public static readonly SyntaxAnnotation Annotated = new SyntaxAnnotation($"{nameof(NullableAnnotation)}.{NullableAnnotation.Annotated}"); + public static readonly SyntaxAnnotation NotAnnotated = new SyntaxAnnotation($"{nameof(NullableAnnotation)}.{NullableAnnotation.NotAnnotated}"); } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs index 868abea333905..b94750b09bae0 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs @@ -8,6 +8,7 @@ using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeGeneration; using Microsoft.CodeAnalysis.CSharp.Syntax; namespace Microsoft.CodeAnalysis.CSharp.Extensions @@ -53,7 +54,13 @@ private static void AddConstraintClauses( } else if (typeParameter.HasNotNullConstraint) { - constraints.Add(SyntaxFactory.TypeConstraint(SyntaxFactory.IdentifierName("notnull"))); + var constraint = SyntaxFactory.TypeConstraint(SyntaxFactory.IdentifierName("notnull")); + +#if !CODE_STYLE + constraint = constraint.WithAdditionalAnnotations(NullableSyntaxAnnotation.Annotated); +#endif + + constraints.Add(constraint); } var constraintTypes = diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs index 6331688f373c0..f1e43cd00e060 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs @@ -10,6 +10,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeGeneration; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -41,7 +42,8 @@ public static TypeSyntax GenerateTypeSyntax( private static TypeSyntax GenerateTypeSyntax( INamespaceOrTypeSymbol symbol, bool nameSyntax, bool allowVar = true) { - if (symbol is ITypeSymbol type && type.ContainsAnonymousType()) + var type = symbol as ITypeSymbol; + if (type != null && type.ContainsAnonymousType()) { // something with an anonymous type can only be represented with 'var', regardless // of what the user's preferences might be. @@ -56,6 +58,22 @@ private static TypeSyntax GenerateTypeSyntax( syntax = syntax.WithAdditionalAnnotations(DoNotAllowVarAnnotation.Annotation); } +#if !CODE_STYLE + + if (type != null && type.IsReferenceType) + { + syntax = syntax.WithAdditionalAnnotations( + type.NullableAnnotation switch + { + NullableAnnotation.None => NullableSyntaxAnnotation.None, + NullableAnnotation.Annotated => NullableSyntaxAnnotation.Annotated, + NullableAnnotation.NotAnnotated => NullableSyntaxAnnotation.NotAnnotated, + _ => throw ExceptionUtilities.UnexpectedValue(type.NullableAnnotation), + }); + } + +#endif + return syntax; } From 7cd72269e943f82e81ee723d20c6f130bf4a7fb8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 May 2020 15:02:08 -0700 Subject: [PATCH 03/18] Add tests --- .../MetadataAsSource/MetadataAsSourceTests.cs | 243 +++++++++++++++++- .../ITypeParameterSymbolExtensions.cs | 8 +- 2 files changed, 230 insertions(+), 21 deletions(-) diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs index ff422cf709b08..c4422114f3494 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs @@ -2260,11 +2260,7 @@ void M() // {CodeAnalysisResources.InMemoryAssembly} #endregion -#nullable enable - -using System.Runtime.CompilerServices; - -public class [|TestType|]<[NullableAttribute(1)] T> where T : notnull +public class [|TestType|] where T : notnull {{ public TestType(); }}"; @@ -2304,8 +2300,6 @@ void M() // {CodeAnalysisResources.InMemoryAssembly} #endregion -#nullable enable - public class TestType {{ public TestType(); @@ -2342,8 +2336,6 @@ class C // {CodeAnalysisResources.InMemoryAssembly} #endregion -#nullable enable - public delegate void [|D|]() where T : notnull;"; using var context = TestContext.Create( @@ -2369,13 +2361,13 @@ public async Task TestNullableEnableDisable1() public class TestType { - public void M1(string) + public void M1(string s) { } #nullable disable - public void M(string) + public void M2(string s) { } }"; @@ -2384,20 +2376,243 @@ class C { void M() { - var obj = new TestType().[|M1|]<int>(); + var obj = new TestType().[|M1|](null); } }"; var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null // {CodeAnalysisResources.InMemoryAssembly} #endregion -#nullable enable +#nullable enable public class TestType {{ public TestType(); - public void [|M|]() where T : notnull; + public void [|M1|](string s); +#nullable disable + public void M2(string s); + +#nullable enable +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable2() + { + var metadata = @" +using System; + +public class TestType +{ + public void M1(string s) + { + } + +#nullable enable + + public void M2(string s) + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](null); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +#nullable enable + +public class TestType +{{ + public TestType(); + +#nullable disable + public void [|M1|](string s); +#nullable enable + public void M2(string s); +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable3() + { + var metadata = @" +#nullable enable + +using System; + +public class TestType +{ + public void M1(string s) + { + } + +#nullable disable + + public void M2(string s) + { + } + + public void M3(string s) + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](null); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +#nullable enable + +public class TestType +{{ + public TestType(); + + public void [|M1|](string s); +#nullable disable + public void M2(string s); + public void M3(string s); + +#nullable enable +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable4() + { + var metadata = @" +#nullable enable + +using System; + +public class TestType +{ + public void M1(ICloneable s) + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](null); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +#nullable enable + +using System; + +public class TestType +{{ + public TestType(); + + public void [|M1|](ICloneable s); +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable5() + { + var metadata = @" +#nullable enable + +using System; + +public class TestType +{ + public void M1(ICloneable s) + { +#nullable disable + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](null); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +#nullable enable + +using System; + +public class TestType +{{ + public TestType(); + + public void [|M1|](ICloneable s); }}"; using var context = TestContext.Create( diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs index b94750b09bae0..3194ba4c1296b 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs @@ -54,13 +54,7 @@ private static void AddConstraintClauses( } else if (typeParameter.HasNotNullConstraint) { - var constraint = SyntaxFactory.TypeConstraint(SyntaxFactory.IdentifierName("notnull")); - -#if !CODE_STYLE - constraint = constraint.WithAdditionalAnnotations(NullableSyntaxAnnotation.Annotated); -#endif - - constraints.Add(constraint); + constraints.Add(SyntaxFactory.TypeConstraint(SyntaxFactory.IdentifierName("notnull"))); } var constraintTypes = From 5674b62ac84b9a15147a5e09731aed5a758f2b6b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 May 2020 21:08:21 -0700 Subject: [PATCH 04/18] remove using --- .../CSharp/Extensions/ITypeParameterSymbolExtensions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs index 3194ba4c1296b..868abea333905 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeParameterSymbolExtensions.cs @@ -8,7 +8,6 @@ using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeGeneration; using Microsoft.CodeAnalysis.CSharp.Syntax; namespace Microsoft.CodeAnalysis.CSharp.Extensions From f8ecb16d0cc5b0a3d53fb904ff80c9221d92e32d Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Thu, 14 May 2020 15:22:22 -0700 Subject: [PATCH 05/18] Update src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs --- .../Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs b/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs index 803f84f18f0be..3ab59e771021d 100644 --- a/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs +++ b/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs @@ -77,7 +77,7 @@ protected override ImmutableArray GetReducers() /// Adds #nullable enable and #nullable disable annotations to the file as necessary. Note that /// this does not try to be 100% accurate, but rather it handles the most common cases out there. Specifically, /// if a file contains any nullable annotated/not-annotated types, then we prefix the file with #nullable - /// enable. Then if we hit any members that explicitly have *oblivious* types, not no annotated or + /// enable. Then if we hit any members that explicitly have *oblivious* types, but no annotated or /// non-annotated types, then we switch to #nullable disable for those specific members. /// /// This is technically innacurate for possible, but very uncommon cases. For example, if the user's code From 45b6c2e25f134effe536e79125ca10c0d4cd9e3e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 May 2020 15:35:33 -0700 Subject: [PATCH 06/18] Add comment --- .../CodeGeneration/NullableSyntaxAnnotation.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs b/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs index 2f7c5cc482697..c2f31ee2a8cf6 100644 --- a/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs +++ b/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs @@ -2,16 +2,18 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using Microsoft.CodeAnalysis.Editing; + namespace Microsoft.CodeAnalysis.CodeGeneration { /// - /// Annotation placed if the code generator encounters a NullableAttribute or NullableContextAttribute while - /// generating the code. + /// Annotation placed on s that the converts to a node. This + /// information tracks the original nullable state of the symbol and is used by metadata-as-source to determine if + /// it needs to add #nullable directives in the file. /// internal sealed class NullableSyntaxAnnotation { - public static readonly SyntaxAnnotation None = new SyntaxAnnotation($"{nameof(NullableAnnotation)}.{NullableAnnotation.None}"); - public static readonly SyntaxAnnotation Annotated = new SyntaxAnnotation($"{nameof(NullableAnnotation)}.{NullableAnnotation.Annotated}"); - public static readonly SyntaxAnnotation NotAnnotated = new SyntaxAnnotation($"{nameof(NullableAnnotation)}.{NullableAnnotation.NotAnnotated}"); + public static readonly SyntaxAnnotation Oblivious = new SyntaxAnnotation($"{nameof(NullableSyntaxAnnotation)}.{Oblivious}"); + public static readonly SyntaxAnnotation NotOblivious = new SyntaxAnnotation($"{nameof(NullableSyntaxAnnotation)}.{NotOblivious}"); } } From 6601df1617a169583047f84efb6ec9bfec14d2cc Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 May 2020 15:44:30 -0700 Subject: [PATCH 07/18] Simplify --- .../CSharpMetadataAsSourceService.cs | 17 +++++++++-------- .../CodeGeneration/NullableSyntaxAnnotation.cs | 8 +++++++- .../CSharp/Extensions/ITypeSymbolExtensions.cs | 6 +++--- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs b/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs index 3ab59e771021d..1bca978c9a2b4 100644 --- a/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs +++ b/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs @@ -106,10 +106,10 @@ protected override async Task AddNullableRegionsAsync(Document documen var root = await tree.GetRootAsync(cancellationToken).ConfigureAwait(false); - var (_, annotated, notAnnotated) = GetNullableAnnotations(root); + var (_, annotatedOrNotAnnotated) = GetNullableAnnotations(root); // If there are no annotated or not-annotated types, then no need to add `#nullable enable`. - if (!annotated && !notAnnotated) + if (!annotatedOrNotAnnotated) return document; var newRoot = AddNullableRegions(root, cancellationToken); @@ -118,11 +118,10 @@ protected override async Task AddNullableRegionsAsync(Document documen return document.WithSyntaxRoot(newRoot); } - private (bool oblivious, bool annotated, bool notAnnotated) GetNullableAnnotations(SyntaxNode node) + private (bool oblivious, bool annotatedOrNotAnnotated) GetNullableAnnotations(SyntaxNode node) { - return (HasAnnotation(node, NullableSyntaxAnnotation.None), - HasAnnotation(node, NullableSyntaxAnnotation.Annotated), - HasAnnotation(node, NullableSyntaxAnnotation.NotAnnotated)); + return (HasAnnotation(node, NullableSyntaxAnnotation.Oblivious), + HasAnnotation(node, NullableSyntaxAnnotation.AnnotatedOrNotAnnotated)); } private bool HasAnnotation(SyntaxNode node, SyntaxAnnotation annotation) @@ -176,6 +175,8 @@ private TypeDeclarationSyntax AddNullableRegionsAroundTypeMembers( foreach (var member in type.Members) { + cancellationToken.ThrowIfCancellationRequested(); + if (member is BaseTypeDeclarationSyntax) { // if we hit a type, and we're currently disabled, then switch us back to enabled for that type. @@ -185,10 +186,10 @@ private TypeDeclarationSyntax AddNullableRegionsAroundTypeMembers( } // we hit a member. see what sort of types it contained. - var (oblivious, annotated, notAnnotated) = GetNullableAnnotations(member); + var (oblivious, annotatedOrNotAnnotated) = GetNullableAnnotations(member); // if we have null annotations, transition us back to the enabled state - if (annotated || notAnnotated) + if (annotatedOrNotAnnotated) { builder.Add(TransitionTo(member, enabled: true, ref currentlyEnabled)); } diff --git a/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs b/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs index c2f31ee2a8cf6..925ce03bfc64e 100644 --- a/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs +++ b/src/Workspaces/Core/Portable/CodeGeneration/NullableSyntaxAnnotation.cs @@ -13,7 +13,13 @@ namespace Microsoft.CodeAnalysis.CodeGeneration /// internal sealed class NullableSyntaxAnnotation { + /// + /// For string~ types. + /// public static readonly SyntaxAnnotation Oblivious = new SyntaxAnnotation($"{nameof(NullableSyntaxAnnotation)}.{Oblivious}"); - public static readonly SyntaxAnnotation NotOblivious = new SyntaxAnnotation($"{nameof(NullableSyntaxAnnotation)}.{NotOblivious}"); + /// + /// For string! or string? types. + /// + public static readonly SyntaxAnnotation AnnotatedOrNotAnnotated = new SyntaxAnnotation($"{nameof(NullableSyntaxAnnotation)}.{AnnotatedOrNotAnnotated}"); } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs index f1e43cd00e060..77e53f1c502d8 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs @@ -65,9 +65,9 @@ private static TypeSyntax GenerateTypeSyntax( syntax = syntax.WithAdditionalAnnotations( type.NullableAnnotation switch { - NullableAnnotation.None => NullableSyntaxAnnotation.None, - NullableAnnotation.Annotated => NullableSyntaxAnnotation.Annotated, - NullableAnnotation.NotAnnotated => NullableSyntaxAnnotation.NotAnnotated, + NullableAnnotation.None => NullableSyntaxAnnotation.Oblivious, + NullableAnnotation.Annotated => NullableSyntaxAnnotation.AnnotatedOrNotAnnotated, + NullableAnnotation.NotAnnotated => NullableSyntaxAnnotation.AnnotatedOrNotAnnotated, _ => throw ExceptionUtilities.UnexpectedValue(type.NullableAnnotation), }); } From f21590df2549fdf2acdf09d37078a08caa27b4ff Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 May 2020 15:52:56 -0700 Subject: [PATCH 08/18] Filter out dynamic attribute. --- .../MetadataAsSource/MetadataAsSourceTests.cs | 44 +++++++++++++++++++ .../CodeGeneration/AttributeGenerator.cs | 7 ++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs index c4422114f3494..7dc6172adc53b 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs @@ -2627,5 +2627,49 @@ public class TestType var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); context.VerifyResult(metadataAsSourceFile, expected); } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestDynamic1() + { + var metadata = @" +using System; + +public class TestType +{ + public void M1(dynamic s) + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](null); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +public class TestType +{{ + public TestType(); + + public void [|M1|](dynamic s); +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } } } diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs index 0c864e30159b7..afa7b550a3ed1 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/AttributeGenerator.cs @@ -93,8 +93,13 @@ private static bool IsCompilerInternalAttribute(AttributeData attribute) var name = attrClass.Name; - if (name != "NullableAttribute" && name != "NullableContextAttribute" && name != "NativeIntegerAttribute") + if (name != "NullableAttribute" && + name != "NullableContextAttribute" && + name != "NativeIntegerAttribute" && + name != "DynamicAttribute") + { return false; + } var ns = attrClass.ContainingNamespace; return ns?.Name == nameof(System.Runtime.CompilerServices) && From 9183eb709b54a24e99e8ee9b25bc29751609c17b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 May 2020 16:09:22 -0700 Subject: [PATCH 09/18] Fix nullable ref type parameters. --- .../MetadataAsSource/MetadataAsSourceTests.cs | 278 ++++++++++++++++++ ...olExtensions.TypeSyntaxGeneratorVisitor.cs | 8 +- 2 files changed, 285 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs index 7dc6172adc53b..96d3176beb5fc 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs @@ -2628,6 +2628,284 @@ public class TestType context.VerifyResult(metadataAsSourceFile, expected); } + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable6() + { + var metadata = @" +#nullable enable + +using System; + +public class TestType +{ + public void M1(T? s) where T : class + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](""""); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +#nullable enable + +public class TestType +{{ + public TestType(); + + public void [|M1|](T? s) where T : class; +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable7() + { + var metadata = @" +#nullable enable + +using System; + +public class TestType +{ + public void M1(T s) where T : class + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](""""); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +#nullable enable + +public class TestType +{{ + public TestType(); + + public void [|M1|](T s) where T : class; +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable8() + { + var metadata = @" +#nullable enable + +using System; + +public class TestType +{ + public void M1(T? s) where T : struct + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](0); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +public class TestType +{{ + public TestType(); + + public void [|M1|](T? s) where T : struct; +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable9() + { + var metadata = @" +#nullable enable + +using System; + +public class TestType +{ + public void M1(T s) where T : struct + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](0); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +public class TestType +{{ + public TestType(); + + public void [|M1|](T s) where T : struct; +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable10() + { + var metadata = @" +#nullable enable + +using System; + +public class TestType +{ + public void M1(T s) + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](""""); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +public class TestType +{{ + public TestType(); + + public void [|M1|](T s); +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable11() + { + var metadata = @" +using System; + +public class TestType +{ + public void M1(T s) + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](""""); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +public class TestType +{{ + public TestType(); + + public void [|M1|](T s); +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] public async Task TestDynamic1() { diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.TypeSyntaxGeneratorVisitor.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.TypeSyntaxGeneratorVisitor.cs index ce43f3b0d2afc..731a2028d3b64 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.TypeSyntaxGeneratorVisitor.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.TypeSyntaxGeneratorVisitor.cs @@ -304,7 +304,13 @@ public override TypeSyntax VisitPointerType(IPointerTypeSymbol symbol) } public override TypeSyntax VisitTypeParameter(ITypeParameterSymbol symbol) - => AddInformationTo(symbol.Name.ToIdentifierName(), symbol); + { + TypeSyntax typeSyntax = AddInformationTo(symbol.Name.ToIdentifierName(), symbol); + if (symbol.NullableAnnotation == NullableAnnotation.Annotated) + typeSyntax = AddInformationTo(SyntaxFactory.NullableType(typeSyntax), symbol); + + return typeSyntax; + } } } } From 6c8c3a04c37147e82874063e03c0721f88363d8c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 May 2020 16:17:38 -0700 Subject: [PATCH 10/18] Add tests --- .../MetadataAsSource/MetadataAsSourceTests.cs | 66 ++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs index 96d3176beb5fc..7c8fb731e215a 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs @@ -2743,7 +2743,7 @@ class C { void M() { - var obj = new TestType().[|M1|](0); + var obj = new TestType().[|M1|]((int?)0); } }"; var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null @@ -2906,6 +2906,70 @@ public class TestType context.VerifyResult(metadataAsSourceFile, expected); } + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable12() + { + var metadata = @" +#nullable enable + +using System; + +namespace N +{ + public class TestType + { + public void M1(string s) + { + } + + #nullable disable + + public void M2(string s) + { + } + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new N.TestType().[|M1|](null); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +#nullable enable + +namespace N +{{ + public class TestType + {{ + public TestType(); + + public void [|M1|](string s); +#nullable disable + public void M2(string s); + +#nullable enable + }} +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] public async Task TestDynamic1() { From 02c58e02e19ac1ab70e896613d8fd242bca551eb Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 May 2020 16:29:45 -0700 Subject: [PATCH 11/18] Add test for nested type --- .../MetadataAsSource/MetadataAsSourceTests.cs | 74 +++++++++++++++++++ .../CSharpMetadataAsSourceService.cs | 13 ++-- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs index 7c8fb731e215a..9cf7a324ad0ea 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs @@ -2970,6 +2970,80 @@ public class TestType context.VerifyResult(metadataAsSourceFile, expected); } + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] + public async Task TestNullableEnableDisable13() + { + var metadata = @" +#nullable enable + +using System; + +public class TestType +{ + public void M1(string s) + { + } + +#nullable disable + + public class Nested + { + public void NestedM(string s) + { + } + } + +#nullable enable + + public void M2(string s) + { + } +}"; + var sourceWithSymbolReference = @" +class C +{ + void M() + { + var obj = new TestType().[|M1|](null); + } +}"; + var expected = $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null +// {CodeAnalysisResources.InMemoryAssembly} +#endregion + +#nullable enable + +public class TestType +{{ + public TestType(); + + public void [|M1|](string s); + public void M2(string s); + + public class Nested + {{ + public Nested(); + +#nullable disable + public void NestedM(string s); + +#nullable enable + }} +}}"; + + using var context = TestContext.Create( + LanguageNames.CSharp, + SpecializedCollections.SingletonEnumerable(metadata), + includeXmlDocComments: false, + languageVersion: "CSharp8", + sourceWithSymbolReference: sourceWithSymbolReference, + metadataLanguageVersion: "CSharp8"); + + var navigationSymbol = await context.GetNavigationSymbolAsync(); + var metadataAsSourceFile = await context.GenerateSourceAsync(navigationSymbol); + context.VerifyResult(metadataAsSourceFile, expected); + } + [Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)] public async Task TestDynamic1() { diff --git a/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs b/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs index 1bca978c9a2b4..a24fc06bf6dd6 100644 --- a/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs +++ b/src/Features/CSharp/Portable/MetadataAsSource/CSharpMetadataAsSourceService.cs @@ -143,13 +143,14 @@ private static SyntaxTrivia[] CreateNullableTrivia(bool enable) }; } - private SyntaxNode AddNullableRegions(SyntaxNode node, CancellationToken cancellationToken) + private TSyntax AddNullableRegions(TSyntax node, CancellationToken cancellationToken) + where TSyntax : SyntaxNode { return node switch { - CompilationUnitSyntax compilationUnit => compilationUnit.WithMembers(AddNullableRegions(compilationUnit.Members, cancellationToken)), - NamespaceDeclarationSyntax ns => ns.WithMembers(AddNullableRegions(ns.Members, cancellationToken)), - TypeDeclarationSyntax type => AddNullableRegionsAroundTypeMembers(type, cancellationToken), + CompilationUnitSyntax compilationUnit => (TSyntax)(object)compilationUnit.WithMembers(AddNullableRegions(compilationUnit.Members, cancellationToken)), + NamespaceDeclarationSyntax ns => (TSyntax)(object)ns.WithMembers(AddNullableRegions(ns.Members, cancellationToken)), + TypeDeclarationSyntax type => (TSyntax)(object)AddNullableRegionsAroundTypeMembers(type, cancellationToken), _ => node, }; } @@ -161,7 +162,7 @@ private SyntaxList AddNullableRegions( using var _ = ArrayBuilder.GetInstance(out var builder); foreach (var member in members) - builder.Add((MemberDeclarationSyntax)AddNullableRegions(member, cancellationToken)); + builder.Add(AddNullableRegions(member, cancellationToken)); return SyntaxFactory.List(builder); } @@ -181,7 +182,7 @@ private TypeDeclarationSyntax AddNullableRegionsAroundTypeMembers( { // if we hit a type, and we're currently disabled, then switch us back to enabled for that type. // This ensures whenever we walk into a type-decl, we're always in the enabled-state. - builder.Add(TransitionTo(member, enabled: true, ref currentlyEnabled)); + builder.Add(TransitionTo(AddNullableRegions(member, cancellationToken), enabled: true, ref currentlyEnabled)); continue; } From d9c2d616901abf3aee84c846d9fd07727ef6ae9c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 May 2020 16:30:44 -0700 Subject: [PATCH 12/18] Whitespace --- .../Test/MetadataAsSource/MetadataAsSourceTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs index 9cf7a324ad0ea..58b6a6034b9ad 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs @@ -3028,7 +3028,7 @@ public class Nested public void NestedM(string s); #nullable enable - }} + }} }}"; using var context = TestContext.Create( From f8de6566cbab6b2ebecd0fc6080d34b448bf06c3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 May 2020 21:51:49 -0700 Subject: [PATCH 13/18] Fix test --- .../GenerateConstructorFromMembersTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/CSharpTest/GenerateFromMembers/GenerateConstructorFromMembers/GenerateConstructorFromMembersTests.cs b/src/EditorFeatures/CSharpTest/GenerateFromMembers/GenerateConstructorFromMembers/GenerateConstructorFromMembersTests.cs index c67eaa36c27d0..6ac99d74b436d 100644 --- a/src/EditorFeatures/CSharpTest/GenerateFromMembers/GenerateConstructorFromMembers/GenerateConstructorFromMembersTests.cs +++ b/src/EditorFeatures/CSharpTest/GenerateFromMembers/GenerateConstructorFromMembers/GenerateConstructorFromMembersTests.cs @@ -1069,7 +1069,7 @@ class Z where T : class string b; T? c; - public Z(int a, string b, T c{|Navigation:)|} + public Z(int a, string b, T? c{|Navigation:)|} { this.a = a; this.b = b ?? throw new ArgumentNullException(nameof(b)); From ad2a2e15b83d83e88002189e8d9789c1a94a41b9 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 May 2020 23:48:07 -0700 Subject: [PATCH 14/18] lint --- .../Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs index 77e53f1c502d8..f46c55ad4604f 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs @@ -10,7 +10,6 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeGeneration; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Shared.Extensions; From 240bf6b9195a45c825f6e230741b0177d0eb8207 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 15 May 2020 00:36:59 -0700 Subject: [PATCH 15/18] lint --- .../Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs index f46c55ad4604f..e19544cc8bdb7 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs @@ -16,6 +16,10 @@ using Microsoft.CodeAnalysis.Simplification; using Roslyn.Utilities; +#if !CODE_STYLE +using Microsoft.CodeAnalysis.CodeGeneration; +#endif + namespace Microsoft.CodeAnalysis.CSharp.Extensions { internal static partial class ITypeSymbolExtensions From ab0f7acc78ad7de685d8d0ebf688008d53c1b2b4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 15 May 2020 10:23:24 -0700 Subject: [PATCH 16/18] Remove param --- .../Core/Portable/FindSymbols/SymbolFinder.cs | 2 +- .../Core/Portable/Remote/RemoteArguments.cs | 2 +- .../SymbolKey/SymbolKey.SymbolKeyReader.cs | 40 +++++++------------ .../Core/Portable/SymbolKey/SymbolKey.cs | 23 +++++------ 4 files changed, 26 insertions(+), 41 deletions(-) diff --git a/src/Workspaces/Core/Portable/FindSymbols/SymbolFinder.cs b/src/Workspaces/Core/Portable/FindSymbols/SymbolFinder.cs index d063f25556ada..6655081145f59 100644 --- a/src/Workspaces/Core/Portable/FindSymbols/SymbolFinder.cs +++ b/src/Workspaces/Core/Portable/FindSymbols/SymbolFinder.cs @@ -213,7 +213,7 @@ public static IEnumerable FindSimilarSymbols(TSymbol symbol, C // We may be talking about different compilations. So do not try to resolve locations. var result = new HashSet(); - var resolution = key.Resolve(compilation, resolveLocations: false, cancellationToken: cancellationToken); + var resolution = key.Resolve(compilation, cancellationToken: cancellationToken); foreach (var current in resolution.OfType()) { result.Add(current); diff --git a/src/Workspaces/Core/Portable/Remote/RemoteArguments.cs b/src/Workspaces/Core/Portable/Remote/RemoteArguments.cs index 58008f84f17ec..dcf144ee7c574 100644 --- a/src/Workspaces/Core/Portable/Remote/RemoteArguments.cs +++ b/src/Workspaces/Core/Portable/Remote/RemoteArguments.cs @@ -76,7 +76,7 @@ public async Task TryRehydrateAsync( // The server and client should both be talking about the same compilation. As such // locations in symbols are save to resolve as we rehydrate the SymbolKey. var symbol = SymbolKey.ResolveString( - SymbolKeyData, compilation, resolveLocations: true, cancellationToken: cancellationToken).GetAnySymbol(); + SymbolKeyData, compilation, cancellationToken: cancellationToken).GetAnySymbol(); if (symbol == null) { diff --git a/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.SymbolKeyReader.cs b/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.SymbolKeyReader.cs index 51ee0c7a650d3..cbf4cdc0388b0 100644 --- a/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.SymbolKeyReader.cs +++ b/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.SymbolKeyReader.cs @@ -275,7 +275,6 @@ private class SymbolKeyReader : Reader public SymbolEquivalenceComparer Comparer { get; private set; } private readonly List _methodSymbolStack = new List(); - private bool _resolveLocations; public SymbolKeyReader() { @@ -289,7 +288,6 @@ public override void Dispose() _idToResult.Clear(); Compilation = null; IgnoreAssemblyKey = false; - _resolveLocations = true; Comparer = null; _methodSymbolStack.Clear(); @@ -299,11 +297,11 @@ public override void Dispose() public static SymbolKeyReader GetReader( string data, Compilation compilation, - bool ignoreAssemblyKey, bool resolveLocations, + bool ignoreAssemblyKey, CancellationToken cancellationToken) { var reader = s_readerPool.Allocate(); - reader.Initialize(data, compilation, ignoreAssemblyKey, resolveLocations, cancellationToken); + reader.Initialize(data, compilation, ignoreAssemblyKey, cancellationToken); return reader; } @@ -311,13 +309,11 @@ private void Initialize( string data, Compilation compilation, bool ignoreAssemblyKey, - bool resolveLocations, CancellationToken cancellationToken) { base.Initialize(data, cancellationToken); Compilation = compilation; IgnoreAssemblyKey = ignoreAssemblyKey; - _resolveLocations = resolveLocations; Comparer = ignoreAssemblyKey ? SymbolEquivalenceComparer.IgnoreAssembliesInstance @@ -508,15 +504,12 @@ public Location ReadLocation() var start = ReadInteger(); var length = ReadInteger(); - if (_resolveLocations) + // The syntax tree can be null if we're resolving this location in a compilation + // that does not contain this file. In this case, just map this location to None. + var syntaxTree = GetSyntaxTree(filePath); + if (syntaxTree != null) { - // The syntax tree can be null if we're resolving this location in a compilation - // that does not contain this file. In this case, just map this location to None. - var syntaxTree = GetSyntaxTree(filePath); - if (syntaxTree != null) - { - return Location.Create(syntaxTree, new TextSpan(start, length)); - } + return Location.Create(syntaxTree, new TextSpan(start, length)); } } else if (kind == LocationKind.MetadataFile) @@ -524,20 +517,17 @@ public Location ReadLocation() var assemblyResolution = ReadSymbolKey(); var moduleName = ReadString(); - if (_resolveLocations) + // We may be resolving in a compilation where we don't have a module + // with this name. In that case, just map this location to none. + if (assemblyResolution.GetAnySymbol() is IAssemblySymbol assembly) { - // We may be resolving in a compilation where we don't have a module - // with this name. In that case, just map this location to none. - if (assemblyResolution.GetAnySymbol() is IAssemblySymbol assembly) + var module = GetModule(assembly.Modules, moduleName); + if (module != null) { - var module = GetModule(assembly.Modules, moduleName); - if (module != null) + var location = FirstOrDefault(module.Locations); + if (location != null) { - var location = FirstOrDefault(module.Locations); - if (location != null) - { - return location; - } + return location; } } } diff --git a/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.cs b/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.cs index 3039c93efd558..cbaeadca57ddb 100644 --- a/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.cs +++ b/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.cs @@ -77,11 +77,10 @@ namespace Microsoft.CodeAnalysis /// The SymbolKey for both 'M' methods will be the same. The SymbolKey will then resolve to both methods. /// /// - /// s are not guaranteed to work across different versions of Roslyn. - /// They can be persisted in their form and used across sessions with - /// the same version of Roslyn. However, future versions may change the encoded format and may - /// no longer be able to previous - /// keys. As such, only persist if using for a cache that can be regenerated if necessary. + /// s are not guaranteed to work across different versions of Roslyn. They can be persisted + /// in their form and used across sessions with the same version of Roslyn. However, future + /// versions may change the encoded format and may no longer be able to previous keys. As + /// such, only persist if using for a cache that can be regenerated if necessary. /// /// internal partial struct SymbolKey @@ -128,11 +127,10 @@ internal static IEqualityComparer GetComparer(bool ignoreCase = false internal static SymbolKeyResolution ResolveString( string symbolKey, Compilation compilation, - bool ignoreAssemblyKey = false, bool resolveLocations = true, - CancellationToken cancellationToken = default) + bool ignoreAssemblyKey = false, CancellationToken cancellationToken = default) { using var reader = SymbolKeyReader.GetReader( - symbolKey, compilation, ignoreAssemblyKey, resolveLocations, cancellationToken); + symbolKey, compilation, ignoreAssemblyKey, cancellationToken); var version = reader.ReadFormatVersion(); if (version != FormatVersion) { @@ -158,15 +156,12 @@ internal static string CreateStringWorker(int version, ISymbol symbol, Cancellat /// /// Tries to resolve this in the given - /// to a matching symbol. - /// should only be given if the symbol was produced from a compilation - /// that has the exact same source as the compilation we're resolving against. Otherwise - /// the locations resolved may not actually be correct in the final compilation. + /// to a matching symbol. /// public SymbolKeyResolution Resolve( - Compilation compilation, bool ignoreAssemblyKey = false, bool resolveLocations = true, CancellationToken cancellationToken = default) + Compilation compilation, bool ignoreAssemblyKey = false, CancellationToken cancellationToken = default) { - return ResolveString(_symbolKeyData, compilation, ignoreAssemblyKey, resolveLocations, cancellationToken); + return ResolveString(_symbolKeyData, compilation, ignoreAssemblyKey, cancellationToken); } /// From bca9b9fbb3196a413140cfba244f87011255c187 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 15 May 2020 10:56:56 -0700 Subject: [PATCH 17/18] Fix --- src/Workspaces/CoreTest/SymbolKeyTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Workspaces/CoreTest/SymbolKeyTests.cs b/src/Workspaces/CoreTest/SymbolKeyTests.cs index 478340c7b2437..b9419cef5e785 100644 --- a/src/Workspaces/CoreTest/SymbolKeyTests.cs +++ b/src/Workspaces/CoreTest/SymbolKeyTests.cs @@ -223,7 +223,7 @@ public void M(C c) { } public A GetA(A a) { return a; } public A GetA(A a, B b) { return a; } public B GetB(A a, B b) { return b; } - publi C GetC() { return default(C); } + public C GetC() { return default(C); } } public class C @@ -699,7 +699,7 @@ object LocalFunction() Assert.NotNull(found); // note: we don't check that the symbols are equal. That's because the compiler - // doesn't guarantee that the TypeParameters will be hte same across successive + // doesn't guarantee that the TypeParameters will be the same across successive // invocations. Assert.Equal(symbol.OriginalDefinition, found.OriginalDefinition); @@ -736,7 +736,7 @@ void Method((C, int) t) // Validate that if the client does ask to resolve locations that we // do not crash if those locations cannot be found. - var found = SymbolKey.ResolveString(id, compilation2, resolveLocations: true).GetAnySymbol(); + var found = SymbolKey.ResolveString(id, compilation2).GetAnySymbol(); Assert.NotNull(found); Assert.Equal(symbol.Name, found.Name); @@ -773,7 +773,7 @@ void Method((C a, int b) t) // Validate that if the client does ask to resolve locations that we // do not crash if those locations cannot be found. - var found = SymbolKey.ResolveString(id, compilation2, resolveLocations: true).GetAnySymbol(); + var found = SymbolKey.ResolveString(id, compilation2).GetAnySymbol(); Assert.NotNull(found); Assert.Equal(symbol.Name, found.Name); From 759c6cd67038bf8e50f8eb526f5e8eec96125210 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 15 May 2020 12:30:09 -0700 Subject: [PATCH 18/18] lint --- .../Core/Portable/SymbolKey/SymbolKeyResolution.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Workspaces/Core/Portable/SymbolKey/SymbolKeyResolution.cs b/src/Workspaces/Core/Portable/SymbolKey/SymbolKeyResolution.cs index ca267d8b6f850..573e00e44bfaf 100644 --- a/src/Workspaces/Core/Portable/SymbolKey/SymbolKeyResolution.cs +++ b/src/Workspaces/Core/Portable/SymbolKey/SymbolKeyResolution.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System.Collections.Immutable; -using System.Threading; #if DEBUG using System.Diagnostics; @@ -12,11 +11,10 @@ namespace Microsoft.CodeAnalysis { /// - /// The result of . - /// If the could be uniquely mapped to a single - /// then that will be returned in . Otherwise, if - /// the key resolves to multiple symbols (which can happen in error scenarios), then - /// and will be returned. + /// The result of . If the could be uniquely mapped to a + /// single then that will be returned in . Otherwise, if the key resolves + /// to multiple symbols (which can happen in error scenarios), then and will be returned. /// /// If no symbol can be found will be null and /// will be empty.