diff --git a/src/Analyzers/CSharp/Analyzers/UsePatternCombinators/AnalyzedPattern.cs b/src/Analyzers/CSharp/Analyzers/UsePatternCombinators/AnalyzedPattern.cs index 3fec548f72e15..c10784732b8c2 100644 --- a/src/Analyzers/CSharp/Analyzers/UsePatternCombinators/AnalyzedPattern.cs +++ b/src/Analyzers/CSharp/Analyzers/UsePatternCombinators/AnalyzedPattern.cs @@ -154,6 +154,19 @@ private Binary(AnalyzedPattern leftPattern, AnalyzedPattern rightPattern, bool i if (!AreEquivalent(target.Syntax, compareTarget.Syntax)) return null; + // An &&/|| expression that looks the same on both sides. This looks like something we could merge into an + // 'and/or' pattern that only executes the target once, and compares the result to multiple constants. + // However, we disqualify certain forms as they strongly imply that executing the target multiple times + // is necessary. For example: + // + // `if (ReadValue(x, ref index) == A && ReadValue(x, ref index) == B)` + // + // This should not get converted to `if (ReadValue(x, ref index) == A and B)`. + // + // This list is not exhaustive and can be expanded as needed. + if (target.Syntax.DescendantNodesAndSelf().OfType().Any(a => a.RefKindKeyword.Kind() is SyntaxKind.RefKeyword)) + return null; + return new Binary(leftPattern, rightPattern, isDisjunctive, token, target); } } diff --git a/src/Analyzers/CSharp/Analyzers/UsePatternCombinators/CSharpUsePatternCombinatorsDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UsePatternCombinators/CSharpUsePatternCombinatorsDiagnosticAnalyzer.cs index 8bc17b8207a5b..0f71c68e633be 100644 --- a/src/Analyzers/CSharp/Analyzers/UsePatternCombinators/CSharpUsePatternCombinatorsDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UsePatternCombinators/CSharpUsePatternCombinatorsDiagnosticAnalyzer.cs @@ -9,7 +9,6 @@ using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.Operations; using Microsoft.CodeAnalysis.Shared.Extensions; namespace Microsoft.CodeAnalysis.CSharp.UsePatternCombinators; @@ -17,8 +16,13 @@ namespace Microsoft.CodeAnalysis.CSharp.UsePatternCombinators; using static AnalyzedPattern; [DiagnosticAnalyzer(LanguageNames.CSharp)] -internal sealed class CSharpUsePatternCombinatorsDiagnosticAnalyzer : - AbstractBuiltInCodeStyleDiagnosticAnalyzer +internal sealed class CSharpUsePatternCombinatorsDiagnosticAnalyzer() : + AbstractBuiltInCodeStyleDiagnosticAnalyzer( + IDEDiagnosticIds.UsePatternCombinatorsDiagnosticId, + EnforceOnBuildValues.UsePatternCombinators, + CSharpCodeStyleOptions.PreferPatternMatching, + s_safePatternTitle, + s_safePatternTitle) { private const string SafeKey = "safe"; @@ -32,15 +36,6 @@ internal sealed class CSharpUsePatternCombinatorsDiagnosticAnalyzer : hasAnyCodeStyleOption: true, s_unsafePatternTitle); - public CSharpUsePatternCombinatorsDiagnosticAnalyzer() - : base(IDEDiagnosticIds.UsePatternCombinatorsDiagnosticId, - EnforceOnBuildValues.UsePatternCombinators, - CSharpCodeStyleOptions.PreferPatternMatching, - s_safePatternTitle, - s_safePatternTitle) - { - } - public static bool IsSafe(Diagnostic diagnostic) => diagnostic.Properties.ContainsKey(SafeKey); diff --git a/src/Features/CSharpTest/UsePatternCombinators/CSharpUsePatternCombinatorsDiagnosticAnalyzerTests.cs b/src/Features/CSharpTest/UsePatternCombinators/CSharpUsePatternCombinatorsDiagnosticAnalyzerTests.cs index a41fc3a0568ff..ef0c58068ab99 100644 --- a/src/Features/CSharpTest/UsePatternCombinators/CSharpUsePatternCombinatorsDiagnosticAnalyzerTests.cs +++ b/src/Features/CSharpTest/UsePatternCombinators/CSharpUsePatternCombinatorsDiagnosticAnalyzerTests.cs @@ -20,20 +20,16 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.UsePatternCombinators; [Trait(Traits.Feature, Traits.Features.CodeActionsUsePatternCombinators)] -public class CSharpUsePatternCombinatorsDiagnosticAnalyzerTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest_NoEditor +public sealed class CSharpUsePatternCombinatorsDiagnosticAnalyzerTests(ITestOutputHelper logger) + : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest_NoEditor(logger) { private static readonly ParseOptions CSharp9 = TestOptions.RegularPreview.WithLanguageVersion(LanguageVersion.CSharp9); - private static readonly OptionsCollection s_disabled = new OptionsCollection(LanguageNames.CSharp) + private static readonly OptionsCollection s_disabled = new(LanguageNames.CSharp) { { CSharpCodeStyleOptions.PreferPatternMatching, new CodeStyleOption2(false, NotificationOption2.None) } }; - public CSharpUsePatternCombinatorsDiagnosticAnalyzerTests(ITestOutputHelper logger) - : base(logger) - { - } - internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace) => (new CSharpUsePatternCombinatorsDiagnosticAnalyzer(), new CSharpUsePatternCombinatorsCodeFixProvider()); @@ -694,4 +690,34 @@ private void M(object o) } """); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75122")] + public async Task TestNotWithMultipleCallsToInvocationWithRefArgument() + { + await TestMissingAsync( + """ + using System; + + static class DataUtils + { + internal static string ReadLine(byte[] bytes, ref int index) + { + throw new NotImplementedException(); + } + } + + class C + { + public void Main(byte[] bytes) + { + int index = 0; + + if ([|DataUtils.ReadLine(bytes, ref index) != "YAFC" || DataUtils.ReadLine(bytes, ref index) != "ProjectPage"|]) + { + throw new InvalidDataException(); + } + } + } + """); + } }