Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue where we were recommending removing the GetEnumerator method used in a foreach #75903

Merged
merged 2 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.RemoveUnusedMembers;
using Microsoft.CodeAnalysis.Shared.Extensions;

Expand All @@ -20,6 +21,8 @@ internal sealed class CSharpRemoveUnusedMembersDiagnosticAnalyzer
TypeDeclarationSyntax,
MemberDeclarationSyntax>
{
protected override ISemanticFacts SemanticFacts => CSharpSemanticFacts.Instance;

protected override IEnumerable<TypeDeclarationSyntax> GetTypeDeclarations(INamedTypeSymbol namedType, CancellationToken cancellationToken)
{
return namedType.DeclaringSyntaxReferences
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.RemoveUnusedMembers;

using static Microsoft.CodeAnalysis.CSharp.UsePatternCombinators.AnalyzedPattern;
using VerifyCS = CSharpCodeFixVerifier<
CSharpRemoveUnusedMembersDiagnosticAnalyzer,
CSharpRemoveUnusedMembersCodeFixProvider>;
Expand Down Expand Up @@ -3243,4 +3242,33 @@ class C

await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/57470")]
public async Task TestForeach()
{
await new VerifyCS.Test
{
TestCode = """
using System;
using System.Collections;

static class Program
{
static void Main(string[] args)
{
foreach (var i in 1..10)
Console.WriteLine(i);
}

static IEnumerator GetEnumerator(this Range range)
{
for (int i = range.Start.Value; i < range.End.Value; i++)
yield return i;
}
}
""",
LanguageVersion = LanguageVersion.CSharp13,
ReferenceAssemblies = ReferenceAssemblies.Net.Net90,
}.RunAsync();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.CodeAnalysis.CodeQuality;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
Expand All @@ -25,8 +26,11 @@ internal abstract class AbstractRemoveUnusedMembersDiagnosticAnalyzer<
TDocumentationCommentTriviaSyntax,
TIdentifierNameSyntax,
TTypeDeclarationSyntax,
TMemberDeclarationSyntax>
: AbstractCodeQualityDiagnosticAnalyzer
TMemberDeclarationSyntax>()
: AbstractCodeQualityDiagnosticAnalyzer(
[s_removeUnusedMembersRule, s_removeUnreadMembersRule],
// We want to analyze references in generated code, but not report unused members in generated code.
GeneratedCodeAnalysisFlags.Analyze)
where TDocumentationCommentTriviaSyntax : SyntaxNode
where TIdentifierNameSyntax : SyntaxNode
where TTypeDeclarationSyntax : TMemberDeclarationSyntax
Expand Down Expand Up @@ -56,11 +60,7 @@ internal abstract class AbstractRemoveUnusedMembersDiagnosticAnalyzer<
new LocalizableResourceString(nameof(AnalyzersResources.Private_member_0_can_be_removed_as_the_value_assigned_to_it_is_never_read), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)),
hasAnyCodeStyleOption: false, isUnnecessary: true);

protected AbstractRemoveUnusedMembersDiagnosticAnalyzer()
: base([s_removeUnusedMembersRule, s_removeUnreadMembersRule],
GeneratedCodeAnalysisFlags.Analyze) // We want to analyze references in generated code, but not report unused members in generated code.
{
}
protected abstract ISemanticFacts SemanticFacts { get; }

protected abstract IEnumerable<TTypeDeclarationSyntax> GetTypeDeclarations(INamedTypeSymbol namedType, CancellationToken cancellationToken);
protected abstract SyntaxList<TMemberDeclarationSyntax> GetMembers(TTypeDeclarationSyntax typeDeclaration);
Expand All @@ -86,7 +86,8 @@ protected virtual void HandleNamedTypeSymbolStart(SymbolStartAnalysisContext con

private sealed class CompilationAnalyzer
{
private readonly object _gate;
private readonly object _gate = new();

/// <summary>
/// State map for candidate member symbols, with the value indicating how each symbol is used in executable code.
/// </summary>
Expand Down Expand Up @@ -114,7 +115,6 @@ private CompilationAnalyzer(
Compilation compilation,
AbstractRemoveUnusedMembersDiagnosticAnalyzer<TDocumentationCommentTriviaSyntax, TIdentifierNameSyntax, TTypeDeclarationSyntax, TMemberDeclarationSyntax> analyzer)
{
_gate = new object();
_analyzer = analyzer;

_taskType = compilation.TaskType();
Expand Down Expand Up @@ -204,15 +204,17 @@ private void RegisterActions(CompilationStartAnalysisContext compilationStartCon
symbolStartContext.RegisterOperationAction(AnalyzeInvocationOperation, OperationKind.Invocation);
symbolStartContext.RegisterOperationAction(AnalyzeNameOfOperation, OperationKind.NameOf);
symbolStartContext.RegisterOperationAction(AnalyzeObjectCreationOperation, OperationKind.ObjectCreation);
symbolStartContext.RegisterOperationAction(AnalyzeLoopOperation, OperationKind.Loop);

// We bail out reporting diagnostics for named types if it contains following kind of operations:
// 1. Invalid operations, i.e. erroneous code:
// We do so to ensure that we don't report false positives during editing scenarios in the IDE, where the user
// is still editing code and fixing unresolved references to symbols, such as overload resolution errors.
// 2. Dynamic operations, where we do not know the exact member being referenced at compile time.
// 3. Operations with OperationKind.None.
symbolStartContext.RegisterOperationAction(_ => hasUnsupportedOperation = true, OperationKind.Invalid, OperationKind.None,
OperationKind.DynamicIndexerAccess, OperationKind.DynamicInvocation, OperationKind.DynamicMemberReference, OperationKind.DynamicObjectCreation);
symbolStartContext.RegisterOperationAction(
_ => hasUnsupportedOperation = true,
OperationKind.Invalid, OperationKind.None, OperationKind.DynamicIndexerAccess, OperationKind.DynamicInvocation, OperationKind.DynamicMemberReference, OperationKind.DynamicObjectCreation);

symbolStartContext.RegisterSymbolEndAction(symbolEndContext => OnSymbolEnd(symbolEndContext, hasUnsupportedOperation));

Expand Down Expand Up @@ -369,6 +371,18 @@ memberReference.Parent is IIncrementOrDecrementOperation ||
}
}

private void AnalyzeLoopOperation(OperationAnalysisContext operationContext)
{
var operation = operationContext.Operation;
if (operation is not IForEachLoopOperation loopOperation)
return;

var symbols = _analyzer.SemanticFacts.GetForEachSymbols(operation.SemanticModel!, loopOperation.Syntax);
OnSymbolUsage(symbols.CurrentProperty, ValueUsageInfo.Read);
OnSymbolUsage(symbols.GetEnumeratorMethod, ValueUsageInfo.Read);
OnSymbolUsage(symbols.MoveNextMethod, ValueUsageInfo.Read);
}

private void AnalyzeInvocationOperation(OperationAnalysisContext operationContext)
{
var targetMethod = ((IInvocationOperation)operationContext.Operation).TargetMethod.OriginalDefinition;
Expand All @@ -380,9 +394,7 @@ private void AnalyzeInvocationOperation(OperationAnalysisContext operationContex
// If the invoked method is a reduced extension method, also mark the original
// method from which it was reduced as "used".
if (targetMethod.ReducedFrom != null)
{
OnSymbolUsage(targetMethod.ReducedFrom, ValueUsageInfo.Read);
}
}

private void AnalyzeNameOfOperation(OperationAnalysisContext operationContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

Imports System.Threading
Imports Microsoft.CodeAnalysis.Diagnostics
Imports Microsoft.CodeAnalysis.LanguageService
Imports Microsoft.CodeAnalysis.RemoveUnusedMembers
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax

Expand All @@ -16,6 +17,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnusedMembers
TypeBlockSyntax,
StatementSyntax)

Protected Overrides ReadOnly Property SemanticFacts As ISemanticFacts = VisualBasicSemanticFacts.Instance

Protected Overrides Sub HandleNamedTypeSymbolStart(context As SymbolStartAnalysisContext, onSymbolUsageFound As Action(Of ISymbol, ValueUsageInfo))
' Mark all methods with handles clause as having a read reference
' to ensure that we consider the method as "used".
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,22 +166,18 @@ private static void AppendAliasNames(IEnumerable<BaseNamespaceDeclarationSyntax>
}
}

public ForEachSymbols GetForEachSymbols(SemanticModel semanticModel, SyntaxNode forEachStatement)
public ForEachSymbols GetForEachSymbols(SemanticModel semanticModel, SyntaxNode node)
{
if (forEachStatement is CommonForEachStatementSyntax csforEachStatement)
{
var info = semanticModel.GetForEachStatementInfo(csforEachStatement);
return new ForEachSymbols(
info.GetEnumeratorMethod,
info.MoveNextMethod,
info.CurrentProperty,
info.DisposeMethod,
info.ElementType);
}
else
{
if (node is not CommonForEachStatementSyntax forEachStatement)
return default;
}

var info = semanticModel.GetForEachStatementInfo(forEachStatement);
return new ForEachSymbols(
info.GetEnumeratorMethod,
info.MoveNextMethod,
info.CurrentProperty,
info.DisposeMethod,
info.ElementType);
}

public SymbolInfo GetCollectionInitializerSymbolInfo(SemanticModel semanticModel, SyntaxNode node, CancellationToken cancellationToken)
Expand Down
Loading