From 4d46f050dd3f1efc31a7915bdc413778da64c068 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 12 Dec 2019 16:23:39 -0800 Subject: [PATCH] PR feedback --- .../Core/Portable/Collections/SmallDictionary.cs | 15 ++++++++------- src/Compilers/Core/Portable/ConstantValue.cs | 2 +- ...stic.DiagnosticWithProgrammaticSuppression.cs | 4 ++-- .../Core/Portable/Diagnostic/Diagnostic.cs | 16 ++++++++-------- .../Diagnostic/Diagnostic_SimpleDiagnostic.cs | 12 ++++++------ .../Portable/Diagnostic/ExternalFileLocation.cs | 2 +- .../Portable/Diagnostic/LocalizableString.cs | 4 ++-- .../Portable/InternalUtilities/TextKeyedCache.cs | 3 +-- src/Compilers/Core/Portable/TreeDumper.cs | 4 ++-- .../Core/Portable/EditAndContinue/EditSession.cs | 2 +- .../Shared/Extensions/LocationExtensions.cs | 15 +++++++++++---- 11 files changed, 43 insertions(+), 36 deletions(-) diff --git a/src/Compilers/Core/Portable/Collections/SmallDictionary.cs b/src/Compilers/Core/Portable/Collections/SmallDictionary.cs index ae790573675..2456ed30493 100644 --- a/src/Compilers/Core/Portable/Collections/SmallDictionary.cs +++ b/src/Compilers/Core/Portable/Collections/SmallDictionary.cs @@ -38,7 +38,8 @@ internal sealed class SmallDictionary : IEnumerable> private AvlNode? _root; public readonly IEqualityComparer Comparer; - public static readonly SmallDictionary Empty = new SmallDictionary(EqualityComparer.Default); + // https://github.com/dotnet/roslyn/issues/40344 + public static readonly SmallDictionary Empty = new SmallDictionary(null!); public SmallDictionary() : this(EqualityComparer.Default) { } @@ -71,7 +72,7 @@ public bool TryGetValue(K key, [MaybeNullWhen(returnValue: false)] out V value) { if (_root != null) { - return TryGetValue(GetHashCode(key), key, out value); + return TryGetValue(GetHashCode(key), key, out value!); } value = default!; @@ -88,7 +89,7 @@ public void Add(K key, V value) get { V value; - if (!TryGetValue(key, out value)) + if (!TryGetValue(key, out value!)) { throw new KeyNotFoundException($"Could not find key {key}"); } @@ -105,7 +106,7 @@ public void Add(K key, V value) public bool ContainsKey(K key) { V value; - return TryGetValue(key, out value); + return TryGetValue(key, out value!); } [Conditional("DEBUG")] @@ -228,7 +229,7 @@ private bool TryGetValue(int hashCode, K key, [MaybeNullWhen(returnValue: false) return true; } - return GetFromList(b.Next, key, out value); + return GetFromList(b.Next, key, out value!); } private bool GetFromList(Node? next, K key, [MaybeNullWhen(returnValue: false)] out V value) @@ -527,7 +528,7 @@ public Enumerator(SmallDictionary dict) // left == right only if both are nulls if (root.Left == root.Right) { - _next = dict._root; + _next = root; } else { @@ -645,7 +646,7 @@ public Enumerator(SmallDictionary dict) // left == right only if both are nulls if (root.Left == root.Right) { - _next = dict._root; + _next = root; } else { diff --git a/src/Compilers/Core/Portable/ConstantValue.cs b/src/Compilers/Core/Portable/ConstantValue.cs index b531e391143..f88fb66d1f4 100644 --- a/src/Compilers/Core/Portable/ConstantValue.cs +++ b/src/Compilers/Core/Portable/ConstantValue.cs @@ -31,7 +31,7 @@ internal enum ConstantValueTypeDiscriminator : byte DateTime, } - internal abstract partial class ConstantValue : IEquatable + internal abstract partial class ConstantValue : IEquatable { public abstract ConstantValueTypeDiscriminator Discriminator { get; } internal abstract SpecialType SpecialType { get; } diff --git a/src/Compilers/Core/Portable/Diagnostic/Diagnostic.DiagnosticWithProgrammaticSuppression.cs b/src/Compilers/Core/Portable/Diagnostic/Diagnostic.DiagnosticWithProgrammaticSuppression.cs index 6504a9db4bd..e3a97594849 100644 --- a/src/Compilers/Core/Portable/Diagnostic/Diagnostic.DiagnosticWithProgrammaticSuppression.cs +++ b/src/Compilers/Core/Portable/Diagnostic/Diagnostic.DiagnosticWithProgrammaticSuppression.cs @@ -40,10 +40,10 @@ public override string Id get { return Descriptor.Id; } } - public override string? GetMessage(IFormatProvider? formatProvider = null) + public override string GetMessage(IFormatProvider? formatProvider = null) => _originalUnsuppressedDiagnostic.GetMessage(formatProvider); - internal override IReadOnlyList? Arguments + internal override IReadOnlyList? Arguments { get { return _originalUnsuppressedDiagnostic.Arguments; } } diff --git a/src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs b/src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs index fb9804451fb..6b935b2f75a 100644 --- a/src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs +++ b/src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs @@ -36,7 +36,7 @@ public abstract partial class Diagnostic : IEquatable, IFormattable public static Diagnostic Create( DiagnosticDescriptor descriptor, Location location, - params object[] messageArgs) + params object?[] messageArgs) { return Create(descriptor, location, null, null, messageArgs); } @@ -57,7 +57,7 @@ public abstract partial class Diagnostic : IEquatable, IFormattable DiagnosticDescriptor descriptor, Location location, ImmutableDictionary? properties, - params object[]? messageArgs) + params object?[] messageArgs) { return Create(descriptor, location, null, properties, messageArgs); } @@ -78,7 +78,7 @@ public abstract partial class Diagnostic : IEquatable, IFormattable DiagnosticDescriptor descriptor, Location location, IEnumerable? additionalLocations, - params object[]? messageArgs) + params object?[] messageArgs) { return Create(descriptor, location, additionalLocations, properties: null, messageArgs: messageArgs); } @@ -105,7 +105,7 @@ public abstract partial class Diagnostic : IEquatable, IFormattable Location location, IEnumerable? additionalLocations, ImmutableDictionary? properties, - params object[]? messageArgs) + params object?[] messageArgs) { return Create(descriptor, location, effectiveSeverity: descriptor.DefaultSeverity, additionalLocations, properties, messageArgs); } @@ -134,7 +134,7 @@ public abstract partial class Diagnostic : IEquatable, IFormattable DiagnosticSeverity effectiveSeverity, IEnumerable? additionalLocations, ImmutableDictionary? properties, - params object[]? messageArgs) + params object?[] messageArgs) { if (descriptor == null) { @@ -300,7 +300,7 @@ internal static Diagnostic Create(DiagnosticInfo info) /// /// Get the culture specific text of the message. /// - public abstract string? GetMessage(IFormatProvider? formatProvider = null); + public abstract string GetMessage(IFormatProvider? formatProvider = null); /// /// Gets the default of the diagnostic's . @@ -466,9 +466,9 @@ internal Diagnostic WithProgrammaticSuppression(ProgrammaticSuppressionInfo prog // compatibility internal virtual int Code { get { return 0; } } - internal virtual IReadOnlyList? Arguments + internal virtual IReadOnlyList? Arguments { - get { return SpecializedCollections.EmptyReadOnlyList(); } + get { return SpecializedCollections.EmptyReadOnlyList(); } } /// diff --git a/src/Compilers/Core/Portable/Diagnostic/Diagnostic_SimpleDiagnostic.cs b/src/Compilers/Core/Portable/Diagnostic/Diagnostic_SimpleDiagnostic.cs index 6e4cd366976..634e910e25f 100644 --- a/src/Compilers/Core/Portable/Diagnostic/Diagnostic_SimpleDiagnostic.cs +++ b/src/Compilers/Core/Portable/Diagnostic/Diagnostic_SimpleDiagnostic.cs @@ -23,7 +23,7 @@ internal sealed class SimpleDiagnostic : Diagnostic private readonly int _warningLevel; private readonly Location _location; private readonly IReadOnlyList _additionalLocations; - private readonly object[] _messageArgs; + private readonly object?[] _messageArgs; private readonly ImmutableDictionary _properties; private readonly bool _isSuppressed; @@ -33,7 +33,7 @@ internal sealed class SimpleDiagnostic : Diagnostic int warningLevel, Location location, IEnumerable? additionalLocations, - object[]? messageArgs, + object?[]? messageArgs, ImmutableDictionary? properties, bool isSuppressed) { @@ -48,7 +48,7 @@ internal sealed class SimpleDiagnostic : Diagnostic _warningLevel = warningLevel; _location = location ?? Location.None; _additionalLocations = additionalLocations?.ToImmutableArray() ?? SpecializedCollections.EmptyReadOnlyList(); - _messageArgs = messageArgs ?? Array.Empty(); + _messageArgs = messageArgs ?? Array.Empty(); _properties = properties ?? ImmutableDictionary.Empty; _isSuppressed = isSuppressed; } @@ -59,7 +59,7 @@ internal sealed class SimpleDiagnostic : Diagnostic int warningLevel, Location location, IEnumerable? additionalLocations, - object[]? messageArgs, + object?[]? messageArgs, ImmutableDictionary? properties, bool isSuppressed = false) { @@ -87,7 +87,7 @@ public override string Id get { return _descriptor.Id; } } - public override string? GetMessage(IFormatProvider? formatProvider = null) + public override string GetMessage(IFormatProvider? formatProvider = null) { if (_messageArgs.Length == 0) { @@ -107,7 +107,7 @@ public override string Id } } - internal override IReadOnlyList Arguments + internal override IReadOnlyList Arguments { get { return _messageArgs; } } diff --git a/src/Compilers/Core/Portable/Diagnostic/ExternalFileLocation.cs b/src/Compilers/Core/Portable/Diagnostic/ExternalFileLocation.cs index ac628cb0c02..67513f44539 100644 --- a/src/Compilers/Core/Portable/Diagnostic/ExternalFileLocation.cs +++ b/src/Compilers/Core/Portable/Diagnostic/ExternalFileLocation.cs @@ -11,7 +11,7 @@ namespace Microsoft.CodeAnalysis /// /// A program location in source code. /// - internal sealed class ExternalFileLocation : Location, IEquatable + internal sealed class ExternalFileLocation : Location, IEquatable { private readonly TextSpan _sourceSpan; private readonly FileLinePositionSpan _lineSpan; diff --git a/src/Compilers/Core/Portable/Diagnostic/LocalizableString.cs b/src/Compilers/Core/Portable/Diagnostic/LocalizableString.cs index 1d7a92dfc0f..50ee9cce074 100644 --- a/src/Compilers/Core/Portable/Diagnostic/LocalizableString.cs +++ b/src/Compilers/Core/Portable/Diagnostic/LocalizableString.cs @@ -21,7 +21,7 @@ public abstract partial class LocalizableString : IFormattable, IEquatable /// Formats the value of the current instance using the optionally specified format. /// - public string? ToString(IFormatProvider? formatProvider) + public string ToString(IFormatProvider? formatProvider) { try { @@ -90,7 +90,7 @@ public bool Equals(LocalizableString? other) /// Provides the implementation of ToString. ToString will provide a default value /// if this method throws an exception. /// - protected abstract string? GetText(IFormatProvider? formatProvider); + protected abstract string GetText(IFormatProvider? formatProvider); /// /// Provides the implementation of GetHashCode. GetHashCode will provide a default value diff --git a/src/Compilers/Core/Portable/InternalUtilities/TextKeyedCache.cs b/src/Compilers/Core/Portable/InternalUtilities/TextKeyedCache.cs index 5c236da86a0..ee114b91ba2 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/TextKeyedCache.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/TextKeyedCache.cs @@ -113,8 +113,7 @@ public void Free() #endregion // Poolable - [return: MaybeNull] - internal T FindItem(char[] chars, int start, int len, int hashCode) + internal T? FindItem(char[] chars, int start, int len, int hashCode) { // get direct element reference to avoid extra range checks ref var localSlot = ref _localTable[LocalIdxFromHash(hashCode)]; diff --git a/src/Compilers/Core/Portable/TreeDumper.cs b/src/Compilers/Core/Portable/TreeDumper.cs index 33064aac1ab..407d3cc4248 100644 --- a/src/Compilers/Core/Portable/TreeDumper.cs +++ b/src/Compilers/Core/Portable/TreeDumper.cs @@ -111,7 +111,7 @@ private void DoDumpCompact(TreeDumperNode node, string indent) public static string DumpXML(TreeDumperNode root, string? indent = null) { var dumper = new TreeDumper(); - dumper.DoDumpXML(root, string.Empty, string.IsNullOrEmpty(indent) ? string.Empty : indent!); + dumper.DoDumpXML(root, string.Empty, indent ?? string.Empty); return dumper._sb.ToString(); } @@ -216,7 +216,7 @@ public TreeDumperNode(string text, object? value, IEnumerable? c public TreeDumperNode(string text) : this(text, null, null) { } public object? Value { get; } - public string? Text { get; } + public string Text { get; } public IEnumerable Children { get; } public TreeDumperNode this[string child] { diff --git a/src/Features/Core/Portable/EditAndContinue/EditSession.cs b/src/Features/Core/Portable/EditAndContinue/EditSession.cs index 6d36a06e4bc..0bc4fb39977 100644 --- a/src/Features/Core/Portable/EditAndContinue/EditSession.cs +++ b/src/Features/Core/Portable/EditAndContinue/EditSession.cs @@ -346,7 +346,7 @@ internal async Task> GetBaseActi case CommittedSolution.DocumentState.OutOfSync: var descriptor = EditAndContinueDiagnosticDescriptors.GetDescriptor(EditAndContinueErrorCode.DocumentIsOutOfSyncWithDebuggee); - outOfSyncDiagnostics.Add(Diagnostic.Create(descriptor, Location.Create(document.FilePath!, textSpan: default, lineSpan: default), new[] { document.FilePath! })); + outOfSyncDiagnostics.Add(Diagnostic.Create(descriptor, Location.Create(document.FilePath!, textSpan: default, lineSpan: default), new[] { document.FilePath })); continue; default: diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/LocationExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/LocationExtensions.cs index b6909fb55d5..c08af932614 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/LocationExtensions.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/LocationExtensions.cs @@ -3,22 +3,29 @@ #nullable enable using System.Threading; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Shared.Extensions { internal static class LocationExtensions { + public static SyntaxTree GetSourceTreeOrThrow(this Location location) + { + Contract.ThrowIfNull(location.SourceTree); + return location.SourceTree; + } + public static SyntaxToken FindToken(this Location location, CancellationToken cancellationToken) - => location.SourceTree!.GetRoot(cancellationToken).FindToken(location.SourceSpan.Start); + => location.GetSourceTreeOrThrow().GetRoot(cancellationToken).FindToken(location.SourceSpan.Start); public static SyntaxNode FindNode(this Location location, CancellationToken cancellationToken) - => location.SourceTree!.GetRoot(cancellationToken).FindNode(location.SourceSpan); + => location.GetSourceTreeOrThrow().GetRoot(cancellationToken).FindNode(location.SourceSpan); public static SyntaxNode FindNode(this Location location, bool getInnermostNodeForTie, CancellationToken cancellationToken) - => location.SourceTree!.GetRoot(cancellationToken).FindNode(location.SourceSpan, getInnermostNodeForTie: getInnermostNodeForTie); + => location.GetSourceTreeOrThrow().GetRoot(cancellationToken).FindNode(location.SourceSpan, getInnermostNodeForTie: getInnermostNodeForTie); public static SyntaxNode FindNode(this Location location, bool findInsideTrivia, bool getInnermostNodeForTie, CancellationToken cancellationToken) - => location.SourceTree!.GetRoot(cancellationToken).FindNode(location.SourceSpan, findInsideTrivia, getInnermostNodeForTie); + => location.GetSourceTreeOrThrow().GetRoot(cancellationToken).FindNode(location.SourceSpan, findInsideTrivia, getInnermostNodeForTie); public static bool IsVisibleSourceLocation(this Location loc) { -- GitLab