Add readonly to struct methods

.editorconfig

@@ -390,8 +390,11 @@ dotnet_diagnostic.IDE0241.severity = suggestion
 # Struct can be made 'readonly'
 dotnet_diagnostic.IDE0250.severity = suggestion
 
+# Struct methods can be made 'readonly'
+dotnet_diagnostic.IDE0251.severity = suggestion
+
 # Null check can be simplified
 dotnet_diagnostic.IDE0270.severity = suggestion
 
 # naming rule violation
-dotnet_diagnostic.IDE1006.severity = suggestion
+dotnet_diagnostic.IDE1006.severity = suggestion

src/Build/Collections/ConvertingEnumerable.cs

@@ -82,7 +82,7 @@ internal ConvertingEnumerator(IEnumerator<TFrom2> backingEnumerator, Func<TFrom2
             /// <summary>
             /// Get the current element, converted
             /// </summary>
-            public TTo2 Current
+            public readonly TTo2 Current
             {
                 get
                 {

src/Build/Construction/ProjectElementContainer.cs

@@ -742,7 +742,7 @@ internal ProjectElementSiblingEnumerable(ProjectElement initial, bool forwards =
             /// <summary>
             /// Get enumerator
             /// </summary>
-            public IEnumerator<ProjectElement> GetEnumerator()
+            public readonly IEnumerator<ProjectElement> GetEnumerator()
             {
                 return _enumerator;
             }
@@ -808,7 +808,7 @@ object System.Collections.IEnumerator.Current
                 /// <summary>
                 /// Dispose. Do nothing.
                 /// </summary>
-                public void Dispose()
+                public readonly void Dispose()
                 {
                 }
 

src/Build/Definition/Project.cs

@@ -2558,7 +2558,7 @@ public static CumulativeRemoveElementData Create()
                     };
                 }
 
-                public void AccumulateInformationFromRemoveItemSpec(EvaluationItemSpec removeSpec)
+                public readonly void AccumulateInformationFromRemoveItemSpec(EvaluationItemSpec removeSpec)
                 {
                     IEnumerable<string> removeSpecFragmentStrings = removeSpec.FlattenFragmentsAsStrings();
                     var removeGlob = removeSpec.ToMSBuildGlob();

src/Build/Definition/ToolsetReader.cs

@@ -817,7 +817,7 @@ private MSBuildExtensionsPathReferenceKind(string value)
         /// <summary>
         /// Returns the corresponding property name - eg. "$(MSBuildExtensionsPath32)"
         /// </summary>
-        public string MSBuildPropertyName => String.Format($"$({StringRepresentation})");
+        public readonly string MSBuildPropertyName => String.Format($"$({StringRepresentation})");
 
         /// <summary>
         /// Tries to find a reference to MSBuildExtensionsPath* property in the given string

src/Build/Evaluation/Expander.cs

@@ -203,7 +203,7 @@ public void Add(ReadOnlyMemory<char> span)
             /// concatenation of the string representation of the values, each additionally subjected
             /// to file path adjustment.
             /// </returns>
-            public object GetResult()
+            public readonly object GetResult()
             {
                 CheckDisposed();
                 if (_firstObject != null)
@@ -228,7 +228,7 @@ public void Dispose()
             /// <summary>
             /// Throws <see cref="ObjectDisposedException"/> if this concatenator is already disposed.
             /// </summary>
-            private void CheckDisposed() =>
+            private readonly void CheckDisposed() =>
                 ErrorUtilities.VerifyThrowObjectDisposed(!_disposed, nameof(SpanBasedConcatenator));
 
             /// <summary>
@@ -3113,7 +3113,7 @@ private struct FunctionBuilder<T>
             /// </summary>
             public UsedUninitializedProperties UsedUninitializedProperties { get; set; }
 
-            internal Function<T> Build()
+            internal readonly Function<T> Build()
             {
                 return new Function<T>(
                     ReceiverType,

src/Build/Evaluation/ItemsAndMetadataPair.cs

@@ -40,7 +40,7 @@ internal ItemsAndMetadataPair(HashSet<string> items, Dictionary<string, Metadata
         /// </summary>
         internal HashSet<string> Items
         {
-            get
+            readonly get
             {
                 return _items;
             }
@@ -58,7 +58,7 @@ internal HashSet<string> Items
         /// </summary>
         internal Dictionary<string, MetadataReference> Metadata
         {
-            get
+            readonly get
             {
                 return _metadata;
             }

src/Build/Evaluation/LazyItemEvaluator.cs

@@ -135,7 +135,7 @@ public ItemData(I item, ProjectItemElement originatingItemElement, int elementOr
                 _normalizedItemValue = normalizedItemValue;
             }
 
-            public ItemData Clone(IItemFactory<I, I> itemFactory, ProjectItemElement initialItemElementForFactory)
+            public readonly ItemData Clone(IItemFactory<I, I> itemFactory, ProjectItemElement initialItemElementForFactory)
             {
                 // setting the factory's item element to the original item element that produced the item
                 // otherwise you get weird things like items that appear to have been produced by update elements

src/Build/Graph/ProjectGraph.cs

@@ -766,7 +766,7 @@ public ProjectGraphBuildRequest(ProjectGraphNode node, ImmutableList<string> tar
 
             public ImmutableList<string> RequestedTargets { get; }
 
-            public bool Equals(ProjectGraphBuildRequest other)
+            public readonly bool Equals(ProjectGraphBuildRequest other)
             {
                 if (Node != other.Node
                     || RequestedTargets.Count != other.RequestedTargets.Count)
@@ -786,12 +786,12 @@ public bool Equals(ProjectGraphBuildRequest other)
                 return true;
             }
 
-            public override bool Equals(object obj)
+            public override readonly bool Equals(object obj)
             {
                 return !(obj is null) && obj is ProjectGraphBuildRequest graphNodeWithTargets && Equals(graphNodeWithTargets);
             }
 
-            public override int GetHashCode()
+            public override readonly int GetHashCode()
             {
                 unchecked
                 {

src/Build/Graph/ProjectGraphEntryPoint.cs

@@ -61,7 +61,7 @@ internal static IEnumerable<ProjectGraphEntryPoint> CreateEnumerable(IEnumerable
             }
         }
 
-        internal IEnumerable<ProjectGraphEntryPoint> AsEnumerable()
+        internal readonly IEnumerable<ProjectGraphEntryPoint> AsEnumerable()
         {
             yield return this;
         }

src/Build/Xml/ProjectXmlUtilities.XmlElementChildIterator.cs

@@ -63,7 +63,7 @@ public XmlElementChildIterator GetEnumerator()
                 return this;
             }
 
-            public XmlElementWithLocation Current
+            public readonly XmlElementWithLocation Current
             {
                 get
                 {
@@ -73,7 +73,7 @@ public XmlElementWithLocation Current
                 }
             }
 
-            private XmlElementWithLocation GetNextNode(XmlNode child)
+            private readonly XmlElementWithLocation GetNextNode(XmlNode child)
             {
                 while (child != null)
                 {

src/Deprecated/Engine/ItemsAndProperties/ExpressionShredder.cs

@@ -425,13 +425,13 @@ internal ItemsAndMetadataPair(Hashtable items, Dictionary<string, MetadataRefere
 
         internal Hashtable Items
         {
-            get { return items; }
+            readonly get { return items; }
             set { items = value; }
         }
 
         internal Dictionary<string, MetadataReference> Metadata
         {
-            get { return metadata; }
+            readonly get { return metadata; }
             set { metadata = value; }
         }
     }

src/Framework/BuildEngineResult.cs

@@ -42,7 +42,7 @@ public BuildEngineResult(bool result, List<IDictionary<string, ITaskItem[]>> tar
         /// <summary>
         /// Did the build pass or fail. True means the build succeeded, False means the build failed.
         /// </summary>
-        public bool Result
+        public readonly bool Result
         {
             get
             {

src/Framework/NativeMethods.cs

@@ -345,7 +345,7 @@ private struct PROCESS_BASIC_INFORMATION
         public UIntPtr UniqueProcessId;
         public UIntPtr InheritedFromUniqueProcessId;
 
-        public uint Size
+        public readonly uint Size
         {
             get
             {

src/Framework/Profiler/EvaluationLocation.cs

@@ -101,7 +101,7 @@ public struct EvaluationLocation
         public EvaluationLocationKind Kind { get; }
 
         /// <nodoc/>
-        public bool IsEvaluationPass => File == null;
+        public readonly bool IsEvaluationPass => File == null;
 
         /// <nodoc/>
         public static EvaluationLocation CreateLocationForCondition(long? parentId, EvaluationPass evaluationPass, string evaluationDescription, string file,
@@ -183,7 +183,7 @@ public EvaluationLocation(EvaluationPass evaluationPass, string evaluationPassDe
         public static EvaluationLocation EmptyLocation { get; } = CreateEmptyLocation();
 
         /// <nodoc/>
-        public EvaluationLocation WithEvaluationPass(EvaluationPass evaluationPass, string passDescription = null)
+        public readonly EvaluationLocation WithEvaluationPass(EvaluationPass evaluationPass, string passDescription = null)
         {
             return new EvaluationLocation(this.Id, evaluationPass, passDescription ?? PassDefaultDescription[evaluationPass],
                 this.File, this.Line, this.ElementName, this.ElementDescription, this.Kind);

src/Shared/FileMatcher.cs

@@ -796,7 +796,7 @@ private struct RecursionState
             /// <summary>
             /// True if a SearchData.DirectoryPattern is specified but we have not descended into a matching directory.
             /// </summary>
-            public bool IsLookingForMatchingDirectory => (SearchData.DirectoryPattern != null && !IsInsideMatchingDirectory);
+            public readonly bool IsLookingForMatchingDirectory => (SearchData.DirectoryPattern != null && !IsInsideMatchingDirectory);
         }
 
         /// <summary>

src/Shared/FileSystem/WindowsNative.cs

@@ -141,7 +141,7 @@ public EnumerateDirectoryResult(string directory, EnumerateDirectoryStatus statu
             /// <summary>
             /// Indicates if enumeration succeeded.
             /// </summary>
-            public bool Succeeded
+            public readonly bool Succeeded
             {
                 get { return Status == EnumerateDirectoryStatus.Success; }
             }
@@ -153,7 +153,7 @@ public bool Succeeded
             /// <remarks>
             /// This is a good <c>default:</c> case when switching on every possible <see cref="EnumerateDirectoryStatus"/>
             /// </remarks>
-            public NativeWin32Exception ThrowForUnknownError()
+            public readonly NativeWin32Exception ThrowForUnknownError()
             {
                 Debug.Assert(Status == EnumerateDirectoryStatus.UnknownError);
                 throw CreateExceptionForError();
@@ -173,7 +173,7 @@ public NativeWin32Exception ThrowForKnownError()
             /// <summary>
             /// Creates (but does not throw) an exception for this result. The result must not be successful.
             /// </summary>
-            public NativeWin32Exception CreateExceptionForError()
+            public readonly NativeWin32Exception CreateExceptionForError()
             {
                 Debug.Assert(Status != EnumerateDirectoryStatus.Success);
                 if (Status == EnumerateDirectoryStatus.UnknownError)

src/Shared/Pair.cs

@@ -44,15 +44,15 @@ public Pair(TKey key, TValue value)
         /// <summary>
         /// Key
         /// </summary>
-        internal TKey Key
+        internal readonly TKey Key
         {
             get { return _key; }
         }
 
         /// <summary>
         /// Value
         /// </summary>
-        internal TValue Value
+        internal readonly TValue Value
         {
             get { return _value; }
         }

src/StringTools/InternableString.Simple.cs

@@ -57,7 +57,7 @@ public Enumerator(InternableString spanBuilder)
             /// <summary>
             /// Returns the current character.
             /// </summary>
-            public char Current => (_string._builder == null ? _string.FirstString[_charIndex] : _string._builder[_charIndex]);
+            public readonly char Current => (_string._builder == null ? _string.FirstString[_charIndex] : _string._builder[_charIndex]);
 
             /// <summary>
             /// Moves to the next character.
@@ -88,7 +88,7 @@ public bool MoveNext()
         /// <summary>
         /// A convenience getter to ensure that we always operate on a non-null string.
         /// </summary>
-        private string FirstString => _firstString ?? string.Empty;
+        private readonly string FirstString => _firstString ?? string.Empty;
 
         /// <summary>
         /// Constructs a new InternableString wrapping the given string.
@@ -116,13 +116,13 @@ internal InternableString(SpanBasedStringBuilder builder)
         /// <summary>
         /// Gets the length of the string.
         /// </summary>
-        public int Length => (_builder == null ? FirstString.Length : _builder.Length);
+        public readonly int Length => (_builder == null ? FirstString.Length : _builder.Length);
 
         /// <summary>
         /// Creates a new enumerator for enumerating characters in this string. Does not allocate.
         /// </summary>
         /// <returns>The enumerator.</returns>
-        public Enumerator GetEnumerator()
+        public readonly Enumerator GetEnumerator()
         {
             return new Enumerator(this);
         }
@@ -132,7 +132,7 @@ public Enumerator GetEnumerator()
         /// </summary>
         /// <param name="other">Another string.</param>
         /// <returns>True if this string is equal to <paramref name="other"/>.</returns>
-        public bool Equals(string other)
+        public readonly bool Equals(string other)
         {
             if (other.Length != Length)
             {
@@ -163,7 +163,7 @@ public bool Equals(string other)
         /// System.String in which case the original string is returned.
         /// </summary>
         /// <returns>The string.</returns>
-        public string ExpensiveConvertToString()
+        public readonly string ExpensiveConvertToString()
         {
             // Special case: if we hold just one string, we can directly return it.
             if (_firstString != null)
@@ -178,9 +178,9 @@ public string ExpensiveConvertToString()
         /// </summary>
         /// <param name="str">The string to compare to.</param>
         /// <returns>True is this instance wraps the given string.</returns>
-        public bool ReferenceEquals(string str)
+        public readonly bool ReferenceEquals(string str)
         {
-            return Object.ReferenceEquals(str, _firstString);
+            return ReferenceEquals(str, _firstString);
         }
 
         /// <summary>
@@ -198,7 +198,7 @@ public override unsafe string ToString()
         /// Implements the simple yet very decently performing djb2 hash function (xor version).
         /// </summary>
         /// <returns>A stable hashcode of the string represented by this instance.</returns>
-        public override int GetHashCode()
+        public override readonly int GetHashCode()
         {
             uint hash = (5381 << 16) + 5381;
             bool isOddIndex = false;

src/StringTools/InternableString.cs

@@ -43,7 +43,7 @@ internal Enumerator(scoped ref InternableString str)
             /// <summary>
             /// Returns the current character.
             /// </summary>
-            public ref readonly char Current
+            public readonly ref readonly char Current
             {
                 get
                 {
@@ -178,7 +178,7 @@ public Enumerator GetEnumerator()
         /// </summary>
         /// <param name="other">Another string.</param>
         /// <returns>True if this string is equal to <paramref name="other"/>.</returns>
-        public bool Equals(string other)
+        public readonly bool Equals(string other)
         {
             if (other.Length != Length)
             {
@@ -210,7 +210,7 @@ public bool Equals(string other)
         /// System.String in which case the original string is returned.
         /// </summary>
         /// <returns>The string.</returns>
-        public unsafe string ExpensiveConvertToString()
+        public readonly unsafe string ExpensiveConvertToString()
         {
             if (Length == 0)
             {
@@ -268,7 +268,7 @@ public unsafe string ExpensiveConvertToString()
 
                 // The invariant that Length is the sum of span lengths is critical in this unsafe method.
                 // Violating it may lead to memory corruption and, since this code tends to run under a lock,
-                // to hangs caused by the lock getting orphaned. Attempt to detect that and throw now, 
+                // to hangs caused by the lock getting orphaned. Attempt to detect that and throw now,
                 // before the corruption causes further problems.
                 if (destPtr != resultPtr + Length)
                 {
@@ -283,7 +283,7 @@ public unsafe string ExpensiveConvertToString()
         /// </summary>
         /// <param name="str">The string to compare to.</param>
         /// <returns>True is this instance wraps the given string.</returns>
-        public bool ReferenceEquals(string str)
+        public readonly bool ReferenceEquals(string str)
         {
             if (_inlineSpan.Length == Length)
             {
@@ -317,7 +317,7 @@ public override string ToString()
         /// characters that feed into the same operation but straddle multiple spans. Note that it must return the same value for
         /// a given string regardless of how it's split into spans (e.g. { "AB" } and { "A", "B" } have the same hash code).
         /// </remarks>
-        public override unsafe int GetHashCode()
+        public override readonly unsafe int GetHashCode()
         {
             uint hash = (5381 << 16) + 5381;
             bool hashedOddNumberOfCharacters = false;