Skip to content
New issue

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

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

Already on GitHub? # to your account

Disable 'use pattern matching' when eliding multiple calls that pass by ref #76183

Merged
merged 1 commit into from
Dec 3, 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 @@ -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<ArgumentSyntax>().Any(a => a.RefKindKeyword.Kind() is SyntaxKind.RefKeyword))
return null;

return new Binary(leftPattern, rightPattern, isDisjunctive, token, target);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,20 @@
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;

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";

Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool>(false, NotificationOption2.None) }
};

public CSharpUsePatternCombinatorsDiagnosticAnalyzerTests(ITestOutputHelper logger)
: base(logger)
{
}

internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)
=> (new CSharpUsePatternCombinatorsDiagnosticAnalyzer(), new CSharpUsePatternCombinatorsCodeFixProvider());

Expand Down Expand Up @@ -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();
}
}
}
""");
}
}
Loading