From c562bad8fe49c0257daf8b177c6b8b7112c67cf1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 Mar 2023 12:44:22 -0800 Subject: [PATCH 1/6] Duplicate a strict-mode check in make-field-readonly --- ...harpMakeFieldReadonlyDiagnosticAnalyzer.cs | 5 ++ ...ractMakeFieldReadonlyDiagnosticAnalyzer.cs | 18 +++-- ...asicMakeFieldReadonlyDiagnosticAnalyzer.vb | 14 ++++ .../MakeFieldReadonlyTests.vb | 76 +++++++++++++++++++ 4 files changed, 106 insertions(+), 7 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/MakeFieldReadonly/CSharpMakeFieldReadonlyDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/MakeFieldReadonly/CSharpMakeFieldReadonlyDiagnosticAnalyzer.cs index 96e84a44f4034..568a56f4a5692 100644 --- a/src/Analyzers/CSharp/Analyzers/MakeFieldReadonly/CSharpMakeFieldReadonlyDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/MakeFieldReadonly/CSharpMakeFieldReadonlyDiagnosticAnalyzer.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.MakeFieldReadonly; +using Microsoft.CodeAnalysis.Operations; namespace Microsoft.CodeAnalysis.CSharp.Analyzers.MakeFieldReadonly { @@ -20,5 +21,9 @@ internal sealed class CSharpMakeFieldReadonlyDiagnosticAnalyzer protected override bool IsWrittenTo(SemanticModel semanticModel, ThisExpressionSyntax expression, CancellationToken cancellationToken) => expression.IsWrittenTo(semanticModel, cancellationToken); + + // No special logic in C#. + protected override bool IsLanguageSpecificFieldWriteInConstructor(IFieldReferenceOperation fieldReference, ISymbol owningSymbol) + => false; } } diff --git a/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs index 1dea82317b603..b505459f304bc 100644 --- a/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs @@ -32,6 +32,7 @@ protected AbstractMakeFieldReadonlyDiagnosticAnalyzer() protected abstract ISyntaxKinds SyntaxKinds { get; } protected abstract bool IsWrittenTo(SemanticModel semanticModel, TThisExpression expression, CancellationToken cancellationToken); + protected abstract bool IsLanguageSpecificFieldWriteInConstructor(IFieldReferenceOperation fieldReference, ISymbol owningSymbol); public override DiagnosticAnalyzerCategory GetAnalyzerCategory() => DiagnosticAnalyzerCategory.SemanticDocumentAnalysis; @@ -204,7 +205,7 @@ void UpdateFieldStateOnWrite(IFieldSymbol field) }); } - private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbol owningSymbol) + private bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbol owningSymbol) { // Check if the underlying member is being written or a writable reference to the member is taken. var valueUsageInfo = fieldReference.GetValueUsageInfo(owningSymbol); @@ -222,7 +223,7 @@ private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbo var isInStaticConstructor = owningSymbol.IsStaticConstructor(); var field = fieldReference.Field; if ((isInConstructor || isInStaticConstructor) && - field.ContainingType == owningSymbol.ContainingType) + field.ContainingType.OriginalDefinition.Equals(owningSymbol.ContainingType.OriginalDefinition)) { // For instance fields, ensure that the instance reference is being initialized by the constructor. var instanceFieldWrittenInCtor = isInConstructor && @@ -235,11 +236,14 @@ private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbo if (instanceFieldWrittenInCtor || staticFieldWrittenInStaticCtor) { // Finally, ensure that the write is not inside a lambda or local function. - if (fieldReference.TryGetContainingAnonymousFunctionOrLocalFunction() is null) - { - // It is safe to ignore this write. - return false; - } + if (fieldReference.TryGetContainingAnonymousFunctionOrLocalFunction() is not null) + return true; + + if (IsLanguageSpecificFieldWriteInConstructor(fieldReference, owningSymbol)) + return true; + + // It is safe to ignore this write. + return false; } } diff --git a/src/Analyzers/VisualBasic/Analyzers/MakeFieldReadonly/VisualBasicMakeFieldReadonlyDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/MakeFieldReadonly/VisualBasicMakeFieldReadonlyDiagnosticAnalyzer.vb index 87a7a285ca1f1..1958e482890db 100644 --- a/src/Analyzers/VisualBasic/Analyzers/MakeFieldReadonly/VisualBasicMakeFieldReadonlyDiagnosticAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/MakeFieldReadonly/VisualBasicMakeFieldReadonlyDiagnosticAnalyzer.vb @@ -6,6 +6,7 @@ Imports System.Threading Imports Microsoft.CodeAnalysis.Diagnostics Imports Microsoft.CodeAnalysis.LanguageService Imports Microsoft.CodeAnalysis.MakeFieldReadonly +Imports Microsoft.CodeAnalysis.Operations Imports Microsoft.CodeAnalysis.VisualBasic.LanguageService Imports Microsoft.CodeAnalysis.VisualBasic.Syntax @@ -19,5 +20,18 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.MakeFieldReadonly Protected Overrides Function IsWrittenTo(semanticModel As SemanticModel, expression As MeExpressionSyntax, cancellationToken As CancellationToken) As Boolean Return expression.IsWrittenTo(semanticModel, cancellationToken) End Function + + Protected Overrides Function IsLanguageSpecificFieldWriteInConstructor(fieldReference As IFieldReferenceOperation, owningSymbol As ISymbol) As Boolean + ' Legacy VB behavior. If a special "feature:strict" (different from "option strict") flag Is on, + ' then this write Is only ok if the containing types are the same. *Not* simply the original + ' definitions being the same (which the caller has already checked): + ' https//github.com/dotnet/roslyn/blob/93d3aa1a2cf1790b1a0fe2d120f00987d50445c0/src/Compilers/VisualBasic/Portable/Binding/Binder_Expressions.vb#L1868-L1871 + + If fieldReference.SemanticModel.SyntaxTree.Options.Features.ContainsKey("strict") Then + Return Not fieldReference.Field.ContainingType.Equals(owningSymbol.ContainingType) + End If + + Return False + End Function End Class End Namespace diff --git a/src/Analyzers/VisualBasic/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.vb b/src/Analyzers/VisualBasic/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.vb index 3669efbd221de..d02b6ac99015e 100644 --- a/src/Analyzers/VisualBasic/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.vb +++ b/src/Analyzers/VisualBasic/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.vb @@ -12,6 +12,8 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.MakeFieldReadonly Public Class MakeFieldReadonlyTests Inherits AbstractVisualBasicDiagnosticProviderBasedUserDiagnosticTest + Private Shared ReadOnly s_strictFeatureFlag As ParseOptions = VisualBasicParseOptions.Default.WithFeatures({New KeyValuePair(Of String, String)("strict", "true")}) + Friend Overrides Function CreateDiagnosticProviderAndFixer(workspace As Workspace) As (DiagnosticAnalyzer, CodeFixProvider) Return (New VisualBasicMakeFieldReadonlyDiagnosticAnalyzer(), New VisualBasicMakeFieldReadonlyCodeFixProvider()) End Function @@ -1123,5 +1125,79 @@ End Module Await TestInRegularAndScript1Async(initialMarkup, expectedMarkup) End Function + + Public Async Function VBStrictFeatureFlagAssignment1() As Task + Await TestInRegularAndScriptAsync( +" +imports System +imports System.Collections.Generic + +class C(Of T) + private shared [|s_value|] as IEqualityComparer(Of T) + + shared sub new() + C(Of T).s_value = nothing + end sub +end class +", +" +imports System +imports System.Collections.Generic + +class C(Of T) + private shared ReadOnly s_value as IEqualityComparer(Of T) + + shared sub new() + C(Of T).s_value = nothing + end sub +end class +", parseOptions:=s_strictFeatureFlag) + End Function + + + Public Async Function VBStrictFeatureFlagAssignment2() As Task + Await TestMissingInRegularAndScriptAsync( +" +imports System +imports System.Collections.Generic + +class C(Of T) + private shared [|s_value|] as IEqualityComparer(Of T) + + shared sub new() + C(Of string).s_value = nothing + end sub +end class +", New TestParameters(parseOptions:=s_strictFeatureFlag)) + End Function + + + Public Async Function VBStrictFeatureFlagAssignment3() As Task + Await TestInRegularAndScriptAsync( +" +imports System +imports System.Collections.Generic + +class C(Of T) + private shared [|s_value|] as IEqualityComparer(Of T) + + shared sub new() + C(Of string).s_value = nothing + end sub +end class +", +" +imports System +imports System.Collections.Generic + +class C(Of T) + private shared ReadOnly s_value as IEqualityComparer(Of T) + + shared sub new() + C(Of string).s_value = nothing + end sub +end class +") + End Function End Class End Namespace From d51e72f2a4d2e32e466f181e786647d41c26493f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 Mar 2023 12:50:11 -0800 Subject: [PATCH 2/6] Fix --- .../AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs index b505459f304bc..68599f2553bba 100644 --- a/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs @@ -114,7 +114,7 @@ void OnSymbolEnd(SymbolAnalysisContext symbolEndContext) var members = ((INamedTypeSymbol)symbolEndContext.Symbol).GetMembers(); foreach (var member in members) { - if (member is IFieldSymbol field && fieldStateMap.TryRemove(field, out var value)) + if (member is IFieldSymbol field && fieldStateMap.TryRemove(field.OriginalDefinition, out var value)) { var (isCandidate, written) = value; if (isCandidate && !written) @@ -161,9 +161,9 @@ static bool IsDataContractSerializable(IFieldSymbol symbol, INamedTypeSymbol? da void UpdateFieldStateOnWrite(IFieldSymbol field) { Debug.Assert(IsCandidateField(field, threadStaticAttribute, dataContractAttribute, dataMemberAttribute)); - Debug.Assert(fieldStateMap.ContainsKey(field)); + Debug.Assert(fieldStateMap.ContainsKey(field.OriginalDefinition)); - fieldStateMap[field] = (isCandidate: true, written: true); + fieldStateMap[field.OriginalDefinition] = (isCandidate: true, written: true); } // Method to get or initialize the field state. @@ -174,13 +174,13 @@ void UpdateFieldStateOnWrite(IFieldSymbol field) return default; } - if (fieldStateMap.TryGetValue(fieldSymbol, out var result)) + if (fieldStateMap.TryGetValue(fieldSymbol.OriginalDefinition, out var result)) { return result; } result = ComputeInitialFieldState(fieldSymbol, options, threadStaticAttribute, dataContractAttribute, dataMemberAttribute, cancellationToken); - return fieldStateMap.GetOrAdd(fieldSymbol, result); + return fieldStateMap.GetOrAdd(fieldSymbol.OriginalDefinition, result); } // Method to compute the initial field state. From 219207278e40e5e05c4f135e36b0244aaccde90b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 Mar 2023 13:02:33 -0800 Subject: [PATCH 3/6] Move checks --- ...harpMakeFieldReadonlyDiagnosticAnalyzer.cs | 5 - .../MakeFieldReadonlyTests.cs | 110 ++++++++++++++++-- ...ractMakeFieldReadonlyDiagnosticAnalyzer.cs | 27 +++-- ...asicMakeFieldReadonlyDiagnosticAnalyzer.vb | 14 --- 4 files changed, 117 insertions(+), 39 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/MakeFieldReadonly/CSharpMakeFieldReadonlyDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/MakeFieldReadonly/CSharpMakeFieldReadonlyDiagnosticAnalyzer.cs index 568a56f4a5692..96e84a44f4034 100644 --- a/src/Analyzers/CSharp/Analyzers/MakeFieldReadonly/CSharpMakeFieldReadonlyDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/MakeFieldReadonly/CSharpMakeFieldReadonlyDiagnosticAnalyzer.cs @@ -9,7 +9,6 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.MakeFieldReadonly; -using Microsoft.CodeAnalysis.Operations; namespace Microsoft.CodeAnalysis.CSharp.Analyzers.MakeFieldReadonly { @@ -21,9 +20,5 @@ internal sealed class CSharpMakeFieldReadonlyDiagnosticAnalyzer protected override bool IsWrittenTo(SemanticModel semanticModel, ThisExpressionSyntax expression, CancellationToken cancellationToken) => expression.IsWrittenTo(semanticModel, cancellationToken); - - // No special logic in C#. - protected override bool IsLanguageSpecificFieldWriteInConstructor(IFieldReferenceOperation fieldReference, ISymbol owningSymbol) - => false; } } diff --git a/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs b/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs index 4234caa769596..92e40791f51b8 100644 --- a/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs +++ b/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs @@ -2,8 +2,10 @@ // 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.Threading.Tasks; using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Analyzers.MakeFieldReadonly; using Microsoft.CodeAnalysis.CSharp.MakeFieldReadonly; using Microsoft.CodeAnalysis.Diagnostics; @@ -18,6 +20,8 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.MakeFieldReadonly [Trait(Traits.Feature, Traits.Features.CodeActionsMakeFieldReadonly)] public class MakeFieldReadonlyTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest { + private static readonly ParseOptions s_strictFeatureFlag = CSharpParseOptions.Default.WithFeatures(new[] { new KeyValuePair("strict", "true") }); + public MakeFieldReadonlyTests(ITestOutputHelper logger) : base(logger) { @@ -1814,7 +1818,7 @@ void M() } [Fact] - public async Task FieldAssignedToLocalReadOnlyRef() + public async Task FieldAssignedToLocalreadonlyRef() { await TestInRegularAndScript1Async( """ @@ -2035,7 +2039,7 @@ await TestMissingAsync( public class MyClass { [System.Runtime.Serialization.DataMember] - private bool [|isReadOnly|]; + private bool [|isreadonly|]; } @@ -2054,7 +2058,7 @@ await TestInRegularAndScript1Async( public class MyClass { [System.Runtime.Serialization.DataMember] - private bool [|isReadOnly|]; + private bool [|isreadonly|]; } @@ -2067,7 +2071,7 @@ public class MyClass public class MyClass { [System.Runtime.Serialization.DataMember] - private readonly bool isReadOnly; + private readonly bool isreadonly; } @@ -2087,9 +2091,9 @@ await TestInRegularAndScript1Async( public class MyClass { [System.Runtime.Serialization.DataMember] - private bool isReadOnly; + private bool isreadonly; - private bool [|isReadOnly2|]; + private bool [|isreadonly2|]; } @@ -2103,9 +2107,9 @@ public class MyClass public class MyClass { [System.Runtime.Serialization.DataMember] - private bool isReadOnly; + private bool isreadonly; - private readonly bool isReadOnly2; + private readonly bool isreadonly2; } @@ -2124,7 +2128,7 @@ await TestMissingAsync( [System.Runtime.Serialization.DataContractAttribute] public class MyClass { - public bool [|isReadOnly|]; + public bool [|isreadonly|]; } @@ -2188,5 +2192,93 @@ public class C } """); } + + [Fact, WorkItem(47197, "https://github.com/dotnet/roslyn/issues/47197")] + public async Task StrictFeatureFlagAssignment1() + { + await TestInRegularAndScriptAsync( + """ + using System; + using System.Collections.Generic; + + class C + { + private static IEqualityComparer [|s_value|]; + + static C() + { + C.s_value = null; + } + } + """, + """ + using System; + using System.Collections.Generic; + + class C + { + private static readonly IEqualityComparer s_value; + + static C() + { + C.s_value = null; + } + } + """, parseOptions: s_strictFeatureFlag); + } + + [Fact, WorkItem(47197, "https://github.com/dotnet/roslyn/issues/47197")] + public async Task StrictFeatureFlagAssignment2() + { + await TestMissingInRegularAndScriptAsync( + """ + using System; + using System.Collections.Generic; + + class C + { + private static IEqualityComparer [|s_value|]; + + static C() + { + C.s_value = null; + } + } + """, new TestParameters(parseOptions: s_strictFeatureFlag)); + } + + [Fact, WorkItem(47197, "https://github.com/dotnet/roslyn/issues/47197")] + public async Task StrictFeatureFlagAssignment3() + { + await TestInRegularAndScriptAsync( + """ + using System; + using System.Collections.Generic; + + class C + { + private static IEqualityComparer [|s_value|]; + + static C() + { + C.s_value = null; + } + } + """, + """ + using System; + using System.Collections.Generic; + + class C + { + private static readonly IEqualityComparer s_value; + + static C() + { + C.s_value = null; + } + } + """); + } } } diff --git a/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs index 68599f2553bba..b0a609fdb55f4 100644 --- a/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs @@ -32,7 +32,6 @@ protected AbstractMakeFieldReadonlyDiagnosticAnalyzer() protected abstract ISyntaxKinds SyntaxKinds { get; } protected abstract bool IsWrittenTo(SemanticModel semanticModel, TThisExpression expression, CancellationToken cancellationToken); - protected abstract bool IsLanguageSpecificFieldWriteInConstructor(IFieldReferenceOperation fieldReference, ISymbol owningSymbol); public override DiagnosticAnalyzerCategory GetAnalyzerCategory() => DiagnosticAnalyzerCategory.SemanticDocumentAnalysis; @@ -97,15 +96,13 @@ void AnalyzeOperation(OperationAnalysisContext operationContext) // Ignore fields that are not candidates or have already been written outside the constructor/field initializer. if (!isCandidate || written) - { return; - } // Check if this is a field write outside constructor and field initializer, and update field state accordingly. - if (IsFieldWrite(fieldReference, operationContext.ContainingSymbol)) - { - UpdateFieldStateOnWrite(fieldReference.Field); - } + if (!IsFieldWrite(fieldReference, operationContext.ContainingSymbol)) + return; + + UpdateFieldStateOnWrite(fieldReference.Field); } void OnSymbolEnd(SymbolAnalysisContext symbolEndContext) @@ -205,14 +202,12 @@ void UpdateFieldStateOnWrite(IFieldSymbol field) }); } - private bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbol owningSymbol) + private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbol owningSymbol) { // Check if the underlying member is being written or a writable reference to the member is taken. var valueUsageInfo = fieldReference.GetValueUsageInfo(owningSymbol); if (!valueUsageInfo.IsWrittenTo()) - { return false; - } // Writes to fields inside constructor are ignored, except for the below cases: // 1. Instance reference of an instance field being written is not the instance being initialized by the constructor. @@ -222,6 +217,7 @@ private bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbol ownin var isInConstructor = owningSymbol.IsConstructor(); var isInStaticConstructor = owningSymbol.IsStaticConstructor(); var field = fieldReference.Field; + if ((isInConstructor || isInStaticConstructor) && field.ContainingType.OriginalDefinition.Equals(owningSymbol.ContainingType.OriginalDefinition)) { @@ -239,8 +235,17 @@ private bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbol ownin if (fieldReference.TryGetContainingAnonymousFunctionOrLocalFunction() is not null) return true; - if (IsLanguageSpecificFieldWriteInConstructor(fieldReference, owningSymbol)) + // Legacy 'strict' C#/VB behavior. If a special "feature:strict" (different from "option strict") flag Is on, + // then this write Is only ok if the containing types are the same. *Not* simply the original + // definitions being the same (which the caller has already checked): + // + // https://github.com/dotnet/roslyn/blob/8770fb62a36157ed4ca38a16a0283d27321a01a7/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs#L1201-L1203 + // https//github.com/dotnet/roslyn/blob/93d3aa1a2cf1790b1a0fe2d120f00987d50445c0/src/Compilers/VisualBasic/Portable/Binding/Binder_Expressions.vb#L1868-L1871 + if (fieldReference.SemanticModel?.SyntaxTree.Options.Features.ContainsKey("strict") is true && + !field.ContainingType.Equals(owningSymbol.ContainingType)) + { return true; + } // It is safe to ignore this write. return false; diff --git a/src/Analyzers/VisualBasic/Analyzers/MakeFieldReadonly/VisualBasicMakeFieldReadonlyDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/MakeFieldReadonly/VisualBasicMakeFieldReadonlyDiagnosticAnalyzer.vb index 1958e482890db..87a7a285ca1f1 100644 --- a/src/Analyzers/VisualBasic/Analyzers/MakeFieldReadonly/VisualBasicMakeFieldReadonlyDiagnosticAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/MakeFieldReadonly/VisualBasicMakeFieldReadonlyDiagnosticAnalyzer.vb @@ -6,7 +6,6 @@ Imports System.Threading Imports Microsoft.CodeAnalysis.Diagnostics Imports Microsoft.CodeAnalysis.LanguageService Imports Microsoft.CodeAnalysis.MakeFieldReadonly -Imports Microsoft.CodeAnalysis.Operations Imports Microsoft.CodeAnalysis.VisualBasic.LanguageService Imports Microsoft.CodeAnalysis.VisualBasic.Syntax @@ -20,18 +19,5 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.MakeFieldReadonly Protected Overrides Function IsWrittenTo(semanticModel As SemanticModel, expression As MeExpressionSyntax, cancellationToken As CancellationToken) As Boolean Return expression.IsWrittenTo(semanticModel, cancellationToken) End Function - - Protected Overrides Function IsLanguageSpecificFieldWriteInConstructor(fieldReference As IFieldReferenceOperation, owningSymbol As ISymbol) As Boolean - ' Legacy VB behavior. If a special "feature:strict" (different from "option strict") flag Is on, - ' then this write Is only ok if the containing types are the same. *Not* simply the original - ' definitions being the same (which the caller has already checked): - ' https//github.com/dotnet/roslyn/blob/93d3aa1a2cf1790b1a0fe2d120f00987d50445c0/src/Compilers/VisualBasic/Portable/Binding/Binder_Expressions.vb#L1868-L1871 - - If fieldReference.SemanticModel.SyntaxTree.Options.Features.ContainsKey("strict") Then - Return Not fieldReference.Field.ContainingType.Equals(owningSymbol.ContainingType) - End If - - Return False - End Function End Class End Namespace From b4477467680c82067f259492c6c6e122bc186fc7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 Mar 2023 13:25:03 -0800 Subject: [PATCH 4/6] Restore --- .../MakeFieldReadonly/MakeFieldReadonlyTests.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs b/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs index 92e40791f51b8..12a0584c68e58 100644 --- a/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs +++ b/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs @@ -2039,7 +2039,7 @@ await TestMissingAsync( public class MyClass { [System.Runtime.Serialization.DataMember] - private bool [|isreadonly|]; + private bool [|isReadOnly|]; } @@ -2058,7 +2058,7 @@ await TestInRegularAndScript1Async( public class MyClass { [System.Runtime.Serialization.DataMember] - private bool [|isreadonly|]; + private bool [|isReadOnly|]; } @@ -2071,7 +2071,7 @@ public class MyClass public class MyClass { [System.Runtime.Serialization.DataMember] - private readonly bool isreadonly; + private readonly bool isReadOnly; } @@ -2091,9 +2091,9 @@ await TestInRegularAndScript1Async( public class MyClass { [System.Runtime.Serialization.DataMember] - private bool isreadonly; + private bool isReadOnly; - private bool [|isreadonly2|]; + private bool [|isReadOnly2|]; } @@ -2107,9 +2107,9 @@ public class MyClass public class MyClass { [System.Runtime.Serialization.DataMember] - private bool isreadonly; + private bool isReadOnly; - private readonly bool isreadonly2; + private readonly bool isReadOnly2; } @@ -2128,7 +2128,7 @@ await TestMissingAsync( [System.Runtime.Serialization.DataContractAttribute] public class MyClass { - public bool [|isreadonly|]; + public bool [|isReadOnly|]; } From e491ce03f45674706c3c6a475923d6fc59bbcc94 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 7 Mar 2023 13:42:00 -0800 Subject: [PATCH 5/6] Simplify code --- .../MakeFieldReadonlyTests.cs | 16 +-- ...ractMakeFieldReadonlyDiagnosticAnalyzer.cs | 63 +++++----- .../MakeFieldReadonlyTests.vb | 20 +-- .../Core/Extensions/ISymbolExtensions.cs | 116 +++++++++--------- 4 files changed, 96 insertions(+), 119 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs b/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs index 12a0584c68e58..db452eac4a309 100644 --- a/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs +++ b/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs @@ -2250,7 +2250,7 @@ static C() [Fact, WorkItem(47197, "https://github.com/dotnet/roslyn/issues/47197")] public async Task StrictFeatureFlagAssignment3() { - await TestInRegularAndScriptAsync( + await TestMissingInRegularAndScriptAsync( """ using System; using System.Collections.Generic; @@ -2264,20 +2264,6 @@ static C() C.s_value = null; } } - """, - """ - using System; - using System.Collections.Generic; - - class C - { - private static readonly IEqualityComparer s_value; - - static C() - { - C.s_value = null; - } - } """); } } diff --git a/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs index b0a609fdb55f4..7e5d7af549400 100644 --- a/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs @@ -214,45 +214,48 @@ private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbo // 2. Field is being written inside a lambda or local function. // Check if we are in the constructor of the containing type of the written field. - var isInConstructor = owningSymbol.IsConstructor(); + var isInInstanceConstructor = owningSymbol.IsConstructor(); var isInStaticConstructor = owningSymbol.IsStaticConstructor(); + + // If we're not in a constructor, this is definitely a write to the field that would prevent it from + // becoming readonly. + if (!isInInstanceConstructor && !isInStaticConstructor) + return true; + var field = fieldReference.Field; - if ((isInConstructor || isInStaticConstructor) && - field.ContainingType.OriginalDefinition.Equals(owningSymbol.ContainingType.OriginalDefinition)) + // Follow 'strict' C#/VB behavior. Has to be opted into by user with feature-flag "strict", but is + // actually what the languages specify as correct. Specifically the actual types of the containing + // type and field-reference-containing type must be the same (not just their OriginalDefinition + // types). + // + // https://github.com/dotnet/roslyn/blob/8770fb62a36157ed4ca38a16a0283d27321a01a7/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs#L1201-L1203 + // https//github.com/dotnet/roslyn/blob/93d3aa1a2cf1790b1a0fe2d120f00987d50445c0/src/Compilers/VisualBasic/Portable/Binding/Binder_Expressions.vb#L1868-L1871 + if (!field.ContainingType.Equals(owningSymbol.ContainingType)) + return true; + + if (isInStaticConstructor) { - // For instance fields, ensure that the instance reference is being initialized by the constructor. - var instanceFieldWrittenInCtor = isInConstructor && - fieldReference.Instance?.Kind == OperationKind.InstanceReference && - !fieldReference.IsTargetOfObjectMemberInitializer(); - // For static fields, ensure that we are in the static constructor. - var staticFieldWrittenInStaticCtor = isInStaticConstructor && field.IsStatic; - - if (instanceFieldWrittenInCtor || staticFieldWrittenInStaticCtor) + if (!field.IsStatic) + return true; + } + else + { + // For instance fields, ensure that the instance reference is being initialized by the constructor. + Debug.Assert(isInInstanceConstructor); + if (fieldReference.Instance?.Kind != OperationKind.InstanceReference || + fieldReference.IsTargetOfObjectMemberInitializer()) { - // Finally, ensure that the write is not inside a lambda or local function. - if (fieldReference.TryGetContainingAnonymousFunctionOrLocalFunction() is not null) - return true; - - // Legacy 'strict' C#/VB behavior. If a special "feature:strict" (different from "option strict") flag Is on, - // then this write Is only ok if the containing types are the same. *Not* simply the original - // definitions being the same (which the caller has already checked): - // - // https://github.com/dotnet/roslyn/blob/8770fb62a36157ed4ca38a16a0283d27321a01a7/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs#L1201-L1203 - // https//github.com/dotnet/roslyn/blob/93d3aa1a2cf1790b1a0fe2d120f00987d50445c0/src/Compilers/VisualBasic/Portable/Binding/Binder_Expressions.vb#L1868-L1871 - if (fieldReference.SemanticModel?.SyntaxTree.Options.Features.ContainsKey("strict") is true && - !field.ContainingType.Equals(owningSymbol.ContainingType)) - { - return true; - } - - // It is safe to ignore this write. - return false; + return true; } } - return true; + // Finally, ensure that the write is not inside a lambda or local function. + if (fieldReference.TryGetContainingAnonymousFunctionOrLocalFunction() is not null) + return true; + + return false; } private static CodeStyleOption2 GetCodeStyleOption(IFieldSymbol field, AnalyzerOptions options) diff --git a/src/Analyzers/VisualBasic/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.vb b/src/Analyzers/VisualBasic/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.vb index d02b6ac99015e..5f878aee21f74 100644 --- a/src/Analyzers/VisualBasic/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.vb +++ b/src/Analyzers/VisualBasic/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.vb @@ -1126,7 +1126,7 @@ End Module End Function - Public Async Function VBStrictFeatureFlagAssignment1() As Task + Public Async Function StrictFeatureFlagAssignment1() As Task Await TestInRegularAndScriptAsync( " imports System @@ -1155,7 +1155,7 @@ end class End Function - Public Async Function VBStrictFeatureFlagAssignment2() As Task + Public Async Function StrictFeatureFlagAssignment2() As Task Await TestMissingInRegularAndScriptAsync( " imports System @@ -1172,8 +1172,8 @@ end class End Function - Public Async Function VBStrictFeatureFlagAssignment3() As Task - Await TestInRegularAndScriptAsync( + Public Async Function StrictFeatureFlagAssignment3() As Task + Await TestMissingAsync( " imports System imports System.Collections.Generic @@ -1185,18 +1185,6 @@ class C(Of T) C(Of string).s_value = nothing end sub end class -", -" -imports System -imports System.Collections.Generic - -class C(Of T) - private shared ReadOnly s_value as IEqualityComparer(Of T) - - shared sub new() - C(Of string).s_value = nothing - end sub -end class ") End Function End Class diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs index 24533e61624bf..68fa8d01fbfec 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs @@ -107,7 +107,7 @@ where symbol.Equals(impl) public static ImmutableArray ImplicitInterfaceImplementations(this ISymbol symbol) => symbol.ExplicitOrImplicitInterfaceImplementations().Except(symbol.ExplicitInterfaceImplementations()).ToImmutableArray(); - public static bool IsOverridable([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsOverridable([NotNullWhen(true)] this ISymbol? symbol) { // Members can only have overrides if they are virtual, abstract or override and is not // sealed. @@ -116,7 +116,7 @@ public static bool IsOverridable([NotNullWhen(returnValue: true)] this ISymbol? !symbol.IsSealed; } - public static bool IsImplementableMember([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsImplementableMember([NotNullWhen(true)] this ISymbol? symbol) { if (symbol != null && symbol.ContainingType != null && @@ -159,109 +159,109 @@ MethodKind.UserDefinedOperator or return symbol.ContainingType; } - public static bool IsErrorType([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as ITypeSymbol)?.TypeKind == TypeKind.Error; + public static bool IsErrorType([NotNullWhen(true)] this ISymbol? symbol) + => symbol is ITypeSymbol { TypeKind: TypeKind.Error }; - public static bool IsModuleType([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as ITypeSymbol)?.IsModuleType() == true; + public static bool IsModuleType([NotNullWhen(true)] this ISymbol? symbol) + => symbol is ITypeSymbol { TypeKind: TypeKind.Module }; - public static bool IsInterfaceType([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as ITypeSymbol)?.IsInterfaceType() == true; + public static bool IsInterfaceType([NotNullWhen(true)] this ISymbol? symbol) + => symbol is ITypeSymbol { TypeKind: TypeKind.Interface }; - public static bool IsArrayType([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsArrayType([NotNullWhen(true)] this ISymbol? symbol) => symbol?.Kind == SymbolKind.ArrayType; - public static bool IsTupleType([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as ITypeSymbol)?.IsTupleType ?? false; + public static bool IsTupleType([NotNullWhen(true)] this ISymbol? symbol) + => symbol is ITypeSymbol { IsTupleType: true }; - public static bool IsAnonymousFunction([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as IMethodSymbol)?.MethodKind == MethodKind.AnonymousFunction; + public static bool IsAnonymousFunction([NotNullWhen(true)] this ISymbol? symbol) + => symbol is IMethodSymbol { MethodKind: MethodKind.AnonymousFunction }; - public static bool IsKind([NotNullWhen(returnValue: true)] this ISymbol? symbol, SymbolKind kind) + public static bool IsKind([NotNullWhen(true)] this ISymbol? symbol, SymbolKind kind) => symbol.MatchesKind(kind); - public static bool MatchesKind([NotNullWhen(returnValue: true)] this ISymbol? symbol, SymbolKind kind) + public static bool MatchesKind([NotNullWhen(true)] this ISymbol? symbol, SymbolKind kind) => symbol?.Kind == kind; - public static bool MatchesKind([NotNullWhen(returnValue: true)] this ISymbol? symbol, SymbolKind kind1, SymbolKind kind2) + public static bool MatchesKind([NotNullWhen(true)] this ISymbol? symbol, SymbolKind kind1, SymbolKind kind2) { return symbol != null && (symbol.Kind == kind1 || symbol.Kind == kind2); } - public static bool MatchesKind([NotNullWhen(returnValue: true)] this ISymbol? symbol, SymbolKind kind1, SymbolKind kind2, SymbolKind kind3) + public static bool MatchesKind([NotNullWhen(true)] this ISymbol? symbol, SymbolKind kind1, SymbolKind kind2, SymbolKind kind3) { return symbol != null && (symbol.Kind == kind1 || symbol.Kind == kind2 || symbol.Kind == kind3); } - public static bool MatchesKind([NotNullWhen(returnValue: true)] this ISymbol? symbol, params SymbolKind[] kinds) + public static bool MatchesKind([NotNullWhen(true)] this ISymbol? symbol, params SymbolKind[] kinds) { return symbol != null && kinds.Contains(symbol.Kind); } - public static bool IsReducedExtension([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsReducedExtension([NotNullWhen(true)] this ISymbol? symbol) => symbol is IMethodSymbol { MethodKind: MethodKind.ReducedExtension }; - public static bool IsEnumMember([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsEnumMember([NotNullWhen(true)] this ISymbol? symbol) => symbol?.Kind == SymbolKind.Field && symbol.ContainingType.IsEnumType(); public static bool IsExtensionMethod(this ISymbol symbol) - => symbol.Kind == SymbolKind.Method && ((IMethodSymbol)symbol).IsExtensionMethod; + => symbol is IMethodSymbol { IsExtensionMethod: true }; - public static bool IsLocalFunction([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => symbol != null && symbol.Kind == SymbolKind.Method && ((IMethodSymbol)symbol).MethodKind == MethodKind.LocalFunction; + public static bool IsLocalFunction([NotNullWhen(true)] this ISymbol? symbol) + => symbol is IMethodSymbol { MethodKind: MethodKind.LocalFunction }; - public static bool IsAnonymousOrLocalFunction([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsAnonymousOrLocalFunction([NotNullWhen(true)] this ISymbol? symbol) => symbol.IsAnonymousFunction() || symbol.IsLocalFunction(); - public static bool IsModuleMember([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => symbol != null && symbol.ContainingSymbol is INamedTypeSymbol && symbol.ContainingType.TypeKind == TypeKind.Module; + public static bool IsModuleMember([NotNullWhen(true)] this ISymbol? symbol) + => symbol is { ContainingType.TypeKind: TypeKind.Module }; - public static bool IsConstructor([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as IMethodSymbol)?.MethodKind == MethodKind.Constructor; + public static bool IsConstructor([NotNullWhen(true)] this ISymbol? symbol) + => symbol is IMethodSymbol { MethodKind: MethodKind.Constructor }; - public static bool IsStaticConstructor([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as IMethodSymbol)?.MethodKind == MethodKind.StaticConstructor; + public static bool IsStaticConstructor([NotNullWhen(true)] this ISymbol? symbol) + => symbol is IMethodSymbol { MethodKind: MethodKind.StaticConstructor }; - public static bool IsDestructor([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as IMethodSymbol)?.MethodKind == MethodKind.Destructor; + public static bool IsDestructor([NotNullWhen(true)] this ISymbol? symbol) + => symbol is IMethodSymbol { MethodKind: MethodKind.Destructor }; - public static bool IsUserDefinedOperator([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as IMethodSymbol)?.MethodKind == MethodKind.UserDefinedOperator; + public static bool IsUserDefinedOperator([NotNullWhen(true)] this ISymbol? symbol) + => symbol is IMethodSymbol { MethodKind: MethodKind.UserDefinedOperator }; - public static bool IsConversion([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as IMethodSymbol)?.MethodKind == MethodKind.Conversion; + public static bool IsConversion([NotNullWhen(true)] this ISymbol? symbol) + => symbol is IMethodSymbol { MethodKind: MethodKind.Conversion }; - public static bool IsOrdinaryMethod([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as IMethodSymbol)?.MethodKind == MethodKind.Ordinary; + public static bool IsOrdinaryMethod([NotNullWhen(true)] this ISymbol? symbol) + => symbol is IMethodSymbol { MethodKind: MethodKind.Ordinary }; public static bool IsOrdinaryMethodOrLocalFunction([NotNullWhen(true)] this ISymbol? symbol) => symbol is IMethodSymbol { MethodKind: MethodKind.Ordinary or MethodKind.LocalFunction }; - public static bool IsDelegateType([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsDelegateType([NotNullWhen(true)] this ISymbol? symbol) => symbol is ITypeSymbol { TypeKind: TypeKind.Delegate }; - public static bool IsAnonymousType([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsAnonymousType([NotNullWhen(true)] this ISymbol? symbol) => symbol is INamedTypeSymbol { IsAnonymousType: true }; - public static bool IsNormalAnonymousType([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsNormalAnonymousType([NotNullWhen(true)] this ISymbol? symbol) => symbol.IsAnonymousType() && !symbol.IsDelegateType(); - public static bool IsAnonymousDelegateType([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsAnonymousDelegateType([NotNullWhen(true)] this ISymbol? symbol) => symbol.IsAnonymousType() && symbol.IsDelegateType(); - public static bool IsAnonymousTypeProperty([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsAnonymousTypeProperty([NotNullWhen(true)] this ISymbol? symbol) => symbol is IPropertySymbol && symbol.ContainingType.IsNormalAnonymousType(); - public static bool IsTupleField([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => symbol is IFieldSymbol && symbol.ContainingType.IsTupleType; + public static bool IsTupleField([NotNullWhen(true)] this ISymbol? symbol) + => symbol is IFieldSymbol { ContainingType.IsTupleType: true }; - public static bool IsIndexer([NotNullWhen(returnValue: true)] this ISymbol? symbol) - => (symbol as IPropertySymbol)?.IsIndexer == true; + public static bool IsIndexer([NotNullWhen(true)] this ISymbol? symbol) + => symbol is IPropertySymbol { IsIndexer: true }; - public static bool IsWriteableFieldOrProperty([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsWriteableFieldOrProperty([NotNullWhen(true)] this ISymbol? symbol) => symbol switch { IFieldSymbol fieldSymbol => !fieldSymbol.IsReadOnly && !fieldSymbol.IsConst, @@ -269,7 +269,7 @@ public static bool IsWriteableFieldOrProperty([NotNullWhen(returnValue: true)] t _ => false, }; - public static bool IsRequired([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsRequired([NotNullWhen(true)] this ISymbol? symbol) => symbol is IFieldSymbol { IsRequired: true } or IPropertySymbol { IsRequired: true }; public static ITypeSymbol? GetMemberType(this ISymbol? symbol) @@ -406,7 +406,7 @@ public static ImmutableArray GetAllTypeArguments(this ISymbol symbo return results.ToImmutableAndFree(); } - public static bool IsAttribute([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsAttribute([NotNullWhen(true)] this ISymbol? symbol) => (symbol as ITypeSymbol)?.IsAttribute() == true; /// @@ -415,7 +415,7 @@ public static bool IsAttribute([NotNullWhen(returnValue: true)] this ISymbol? sy /// is unsafe, as is int* Goo { get; }. This will return for /// symbols that cannot have the modifier on them. /// - public static bool RequiresUnsafeModifier([NotNullWhen(returnValue: true)] this ISymbol? member) + public static bool RequiresUnsafeModifier([NotNullWhen(true)] this ISymbol? member) { // TODO(cyrusn): Defer to compiler code to handle this once it can. return member?.Accept(new RequiresUnsafeModifierVisitor()) == true; @@ -465,14 +465,14 @@ static string WithArity(string typeName, int arity) => arity > 0 ? typeName + '`' + arity : typeName; } - public static bool IsStaticType([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsStaticType([NotNullWhen(true)] this ISymbol? symbol) => symbol != null && symbol.Kind == SymbolKind.NamedType && symbol.IsStatic; - public static bool IsNamespace([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsNamespace([NotNullWhen(true)] this ISymbol? symbol) => symbol?.Kind == SymbolKind.Namespace; public static bool IsOrContainsAccessibleAttribute( - [NotNullWhen(returnValue: true)] this ISymbol? symbol, ISymbol withinType, IAssemblySymbol withinAssembly, CancellationToken cancellationToken) + [NotNullWhen(true)] this ISymbol? symbol, ISymbol withinType, IAssemblySymbol withinAssembly, CancellationToken cancellationToken) { var namespaceOrType = symbol is IAliasSymbol alias ? alias.Target : symbol as INamespaceOrTypeSymbol; if (namespaceOrType == null) @@ -542,13 +542,13 @@ public static bool IsInaccessibleLocal(this ISymbol symbol, int position) return declarationSyntax != null && position < declarationSyntax.SpanStart; } - public static bool IsAccessor([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsAccessor([NotNullWhen(true)] this ISymbol? symbol) => symbol.IsPropertyAccessor() || symbol.IsEventAccessor(); - public static bool IsPropertyAccessor([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsPropertyAccessor([NotNullWhen(true)] this ISymbol? symbol) => (symbol as IMethodSymbol)?.MethodKind.IsPropertyAccessor() == true; - public static bool IsEventAccessor([NotNullWhen(returnValue: true)] this ISymbol? symbol) + public static bool IsEventAccessor([NotNullWhen(true)] this ISymbol? symbol) => symbol is IMethodSymbol { MethodKind: MethodKind.EventAdd or MethodKind.EventRaise or MethodKind.EventRemove }; public static bool IsFromSource(this ISymbol symbol) @@ -573,7 +573,7 @@ public static bool IsNonImplicitAndFromSource(this ISymbol symbol) /// If the is a type symbol, returns if that type is "awaitable". /// An "awaitable" is any type that exposes a GetAwaiter method which returns a valid "awaiter". This GetAwaiter method may be an instance method or an extension method. /// - public static bool IsAwaitableNonDynamic([NotNullWhen(returnValue: true)] this ISymbol? symbol, SemanticModel semanticModel, int position) + public static bool IsAwaitableNonDynamic([NotNullWhen(true)] this ISymbol? symbol, SemanticModel semanticModel, int position) { var methodSymbol = symbol as IMethodSymbol; ITypeSymbol? typeSymbol = null; From df0cba25d5a3a2c7f0bb1d09ccb513024a730a5a Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 7 Mar 2023 14:02:59 -0800 Subject: [PATCH 6/6] Update src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs Co-authored-by: Sam Harwell --- .../CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs b/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs index db452eac4a309..a98bab6ace582 100644 --- a/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs +++ b/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs @@ -1818,7 +1818,7 @@ void M() } [Fact] - public async Task FieldAssignedToLocalreadonlyRef() + public async Task FieldAssignedToLocalReadOnlyRef() { await TestInRegularAndScript1Async( """