Fix pattern matching bugs in 1.4.1 (#43)

* Fix pattern matching bugs in 1.4.1

Addressed multiple issues related to pattern matching and complex conditional logic that were impacting code analysis accuracy. This update ensures more reliable validation of advanced C# constructs.

* Refactor code and add more test cases

The code has been refactored to remove redundancies, fix incorrect logic, and improve readability. In addition, multiple test cases have been added to verify the absence of diagnostics when accessing a value after checking for failure.

* Refine logic in WalkerResult

The ternary condition inside the CorrectUsage property in WalkerResult.cs has been adjusted. The change ensures that the property correctly combines conditions for CheckedFailure, termination, and non-use of "AccessedValue".

CSharpFunctionalExtensions.Analyzers.Samples/CSharpFunctionalExtensions.Analyzers.Samples.csproj

@@ -15,7 +15,7 @@
                           OutputItemType="Analyzer" ReferenceOutputAssembly="false"/>
     </ItemGroup>
     <ItemGroup>
-      <PackageReference Include="CSharpFunctionalExtensions" Version="2.41.0" />
+      <PackageReference Include="CSharpFunctionalExtensions" Version="2.42.5" />
     </ItemGroup>
 
 </Project>

CSharpFunctionalExtensions.Analyzers.Samples/Class1.cs

@@ -44,22 +44,29 @@ public int Test()
     {
         var y = Result.Success(1);
 
-        if (y.IsFailure) return y.Value;
-
+        if (!y.IsSuccess || y.Value > 0 )  return 0;
+        
         return 1;
     }
 
+    public int Test2()
+    {
+        var y = Result.Success(1);
+        var x =  y.IsSuccess ? y.Value : 0;
+        var foo = y.Value;
+        return y.IsFailure ? 0 : y.Value;
+    }
     public int PatternMatching()
     {
         var result = Result.Success(1);
         var id = result switch
         {
-            { IsSuccess: true }=> result.Value,
-            { Error: "eror" } => 0,
+            { IsSuccess: true, Value: 1 }=> result.Value,
+            { Error: "eror", Value: 1  } => 0,
             _ => throw new ArgumentOutOfRangeException()
         };
 
-        var x = (result.IsFailure)? 0: result.Value;
+        var x = result.IsFailure? 0: result.Value;
         switch (result.IsSuccess)
         {
             case true:
@@ -75,10 +82,29 @@ public void UsingStatementExample()
     {
         var result = Result.Success(1);
         if (result.IsFailure) return;
-
+        //var resultMessage = !result.IsSuccess ? $"{result.Value} success." : "Failed.";
         using (var streamWriter = new StreamWriter("filePath"))
         {
             streamWriter.Write(result.Value);
         }
     }
+
+    public void CombinedCheckExample()
+    {
+        var result = Result.Success(1);
+        //if (result.IsFailure || result.Value == 1 ) return;
+        if (result is { IsSuccess: true, Value: > 1 })
+        {
+            Console.WriteLine("foo" + result.Value);
+        }
+
+        // if (result.IsSuccess && result.Value == 1)
+        // {
+        //     Console.WriteLine("foo" + result.Value);
+        // }
+
+
+    }
+
+
 }

CSharpFunctionalExtensions.Analyzers.Tests/CSharpFunctionalExtensions.Analyzers.Tests.csproj

@@ -22,7 +22,7 @@
             <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
             <PrivateAssets>all</PrivateAssets>
         </PackageReference>
-        <PackageReference Include="CSharpFunctionalExtensions" Version="2.41.0" />
+        <PackageReference Include="CSharpFunctionalExtensions" Version="2.42.5" />
         <PackageReference Include="Microsoft.CodeAnalysis.CodeFix.Testing" Version="1.1.1" />
         <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing.XUnit" Version="1.1.1"/>
         <PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing.XUnit" Version="1.1.1"/>        

CSharpFunctionalExtensions.Analyzers.Tests/UseResultValueWithoutCheckTests/UseResultValueWithoutCheckTests.cs

@@ -63,6 +63,7 @@ public async Task TestNoDiagnostic_AccesValueOnResultObject_WithCheckIsSuccess(s
     [InlineData("""if(result.IsFailure || result.Value == 1) Console.WriteLine("foo");""")]
     [InlineData("""if(!result.IsSuccess || result.Value > 1) Console.WriteLine("foo");""")]
     [InlineData("var x = !result.IsFailure ? result.Value: 0;")]
+    [InlineData("var x = result.IsSuccess ? Foo(result.Value): 0;")]
     public async Task TestNoDiagnostic_AccesValueOnResultObject_WithCheckIsFailure(string source)
     {
         await VerifyNoDiagnosticAsync(AddContext(source), options: CSharpTestOptions());
@@ -91,13 +92,44 @@ public async Task Test_AccessValueAfterCheckForFailure(string source)
     {
         await VerifyDiagnosticAsync(AddContext(source), options: CSharpTestOptions());
     }
+
+    [Theory]
+    [InlineData("if(!result.IsFailure) Console.WriteLine(result.Value);")]
+    [InlineData(" var x = result.IsFailure ? 0 : result.Value;")]
+    [InlineData(" var x = !result.IsSuccess ? 0 : result.Value;")]
+    [InlineData(" var x = result.IsSuccess ? result.Value : 0;")]
+    [InlineData(" var x = !result.IsFailure ? result.Value : 0;")]
+    public async Task TestNoDiagnostic_AccessValueAfterCheckForFailure(string source)
+    {
+        await VerifyNoDiagnosticAsync(AddContext(source), options: CSharpTestOptions());
+    } 
+
+    [Theory]
+    [InlineData("return result.IsSuccess ? result.Value: 0;")]
+    [InlineData("return result.IsSuccess ? Foo(result.Value) : 0;")]
+    [InlineData("return !result.IsSuccess ? 0: result.Value;")]
+    [InlineData("return !result.IsSuccess ? 0: Foo(result.Value);")]
+    public async Task TestNoDiagnostic_AccessValueAfterCheckForSuccessWithReturn(string source)
+    {
+        await VerifyNoDiagnosticAsync(AddReturnValueContext(source), options: CSharpTestOptions());
+    }  
+
+    [Theory]
+    [InlineData("return !result.IsSuccess ? [|result.Value|]: 0;")]
+    [InlineData("return !result.IsSuccess ? Foo([|result.Value|]) : 0;")]
+    [InlineData("return result.IsSuccess ? 0: [|result.Value|];")]
+    [InlineData("return result.IsSuccess ? 0: Foo([|result.Value|]);")]
+    public async Task TestDiagnostic_AccessValueAfterCheckForSuccessWithReturn(string source)
+    {
+        await VerifyDiagnosticAsync(AddReturnValueContext(source), options: CSharpTestOptions());
+    }
 
     [Fact]
     public async Task Test_AccessWithinReturnStatement()
     {
         await VerifyDiagnosticAsync(
             $$"""
-                                        using System;
+                                        using System; 
                                         using CSharpFunctionalExtensions;
 
                                         public class Class2
@@ -193,6 +225,30 @@ public void GetId(int a)
                  var result = {{result}};
                  {{testString}}
               }
+              public int Foo(int bar)
+              {
+                 return bar;
+              }
+          }
+          """;
+
+    private string AddReturnValueContext(string testString, string result = "Result.Success(1)") =>
+        $$"""
+          using System;
+          using CSharpFunctionalExtensions;
+
+          public class FunctionsWithResultObject
+          {
+              public int GetId(int a)
+              {
+                var result = {{result}};
+                {{testString}}
+              }
+              
+              public int Foo(int bar)
+              {
+                return bar;
+              }
           }
           """;
 }

CSharpFunctionalExtensions.Analyzers/CSharpFunctionalExtensions.Analyzers.csproj

@@ -36,7 +36,7 @@
     </PropertyGroup>
 
     <ItemGroup>
-        <PackageReference Include="CSharpFunctionalExtensions" Version="2.41.0">
+        <PackageReference Include="CSharpFunctionalExtensions" Version="2.42.5">
             <PrivateAssets>all</PrivateAssets>
             <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
         </PackageReference>        

CSharpFunctionalExtensions.Analyzers/UseResultValueWithoutCheck/ResultValueWalker.cs

@@ -73,14 +73,35 @@ private void NodeWalkerInternal(SyntaxNode node)
     private void CheckTernaryCondition(ConditionalExpressionSyntax ternary)
     {
         _result.CheckResult = DetermineCheckResult(ternary.Condition);
+        _result.Terminated = true;
+        bool whenTrueContainsAccess = ContainsMatchingMemberAccess(ternary.WhenTrue, _memberAccessValueResult);
+        bool whenFalseContainsAccess = ContainsMatchingMemberAccess(ternary.WhenFalse, _memberAccessValueResult);
+
         _result.AccessedValue = _result.CheckResult switch
         {
-            CheckResult.CheckedSuccess => ternary.WhenTrue == _memberAccessValueResult && ternary.WhenFalse != _memberAccessValueResult,
-            CheckResult.CheckedFailure => ternary.WhenFalse == _memberAccessValueResult && ternary.WhenTrue != _memberAccessValueResult,
+            CheckResult.CheckedSuccess => whenTrueContainsAccess && !whenFalseContainsAccess,
+            CheckResult.CheckedFailure => whenTrueContainsAccess,
             _ => _result.AccessedValue
         };
     }
+
+    private bool ContainsMatchingMemberAccess(SyntaxNode expression, SyntaxNode targetMemberAccess)
+    {
+        if (expression.IsEquivalentTo(targetMemberAccess))
+            return true;
+
+        foreach (var child in expression.DescendantNodes().OfType<ExpressionSyntax>())
+        {
+            if (child is MemberAccessExpressionSyntax memberAccess && memberAccess.IsEquivalentTo(targetMemberAccess))
+                return true;
+
+            if (ContainsMatchingMemberAccess(child, targetMemberAccess))
+                return true;
+
+        }
 
+        return false;
+    }
     private CheckResult BinaryExpressionSyntax(BinaryExpressionSyntax binaryExpression)
     {
         switch (binaryExpression.OperatorToken.Kind())
@@ -178,20 +199,28 @@ private CheckResult DetermineCheckResult(ExpressionSyntax condition)
             case ConditionalExpressionSyntax ternary:
                 return DetermineCheckResult(ternary.Condition);
             case IsPatternExpressionSyntax isPatternExpressionSyntax:
+                var info = DebugInfo(isPatternExpressionSyntax);
                 return isPatternExpressionSyntax.Pattern switch
                 {
                     RecursivePatternSyntax recursivePatternSyntax => CheckedRecusivePattern(recursivePatternSyntax),
-                    _ => throw new ArgumentOutOfRangeException()
                 };
-
-                break;
-            case SwitchExpressionSyntax switchExpressionSyntax:
-                throw new NotImplementedException();
+            // case SwitchExpressionSyntax switchExpressionSyntax:
+            //     throw new NotImplementedException();
         }
 
         return CheckResult.Unchecked;
     }
 
+    private static string DebugInfo(ExpressionSyntax syntax)
+    {
+        var syntaxTree = syntax.SyntaxTree;
+        var lineSpan = syntaxTree.GetLineSpan(syntax.Span);
+        var startLineNumber = lineSpan.StartLinePosition.Line; // 0-based
+        var startCharacterPosition = lineSpan.StartLinePosition.Character; // Ook 0-based
+        var filePath = syntaxTree.FilePath;
+        return $"File: {filePath} Line: {startLineNumber +1}, Char: {startCharacterPosition + 1}";
+    }
+
     private static bool IsSuccess(string leftExpression, string rightExpression)
     {
         return leftExpression.Contains("IsSuccess") && rightExpression == "true"

README.md

@@ -77,4 +77,4 @@ Contributions are welcome! Please feel free to open an issue or submit a pull re
 
 ## License
 
-MIT
+MIT

build/Build.cs

@@ -55,7 +55,7 @@ class Build : NukeBuild
 
     static readonly string PackageContentType = "application/octet-stream";
     static string ChangeLogFile => RootDirectory / "CHANGELOG.md";
-    
+
     string BranchSpec => GitHubActions?.Ref;
     bool IsTag => BranchSpec != null && BranchSpec.Contains("refs/tags", StringComparison.OrdinalIgnoreCase);
 
@@ -73,7 +73,7 @@ class Build : NukeBuild
         });
 
     Target Restore => _ => _
-        .DependsOn(Clean)       
+        .DependsOn(Clean)
         .Executes(() =>
         {
             DotNetRestore(s => s.SetProjectFile(SourceDirectory));
@@ -130,7 +130,8 @@ class Build : NukeBuild
     Target PublishToGithub => _ => _
         .Description($"Publishing to Github for Development only.")
         .Triggers(CreateRelease)
-        .OnlyWhenStatic(() => Repository.IsOnDevelopBranch() && GitHubActions is not null || GitHubActions?.IsPullRequest == true)
+        .OnlyWhenStatic(() =>
+            (Repository.IsOnDevelopBranch() || Repository.IsOnReleaseBranch()) && GitHubActions is not null || GitHubActions?.IsPullRequest == true)
         .Executes(() =>
         {
             ArtifactsDirectory.GlobFiles(ArtifactsType)
@@ -149,8 +150,7 @@ class Build : NukeBuild
         .DependsOn(Pack)
         .Requires(() => NugetApiUrl)
         .Requires(() => NugetApiKey)
-        .OnlyWhenDynamic(() => IsTag, "No Tag added to commit")
-        .Triggers(CreateRelease)
+        .OnlyWhenDynamic(() => IsTag|| Repository.IsOnReleaseBranch(), "No Tag added to commit")
         .Executes(() =>
         {
             DotNetNuGetPush(s =>
@@ -162,10 +162,11 @@ class Build : NukeBuild
                     )
             );
         });
-    
+
     Target CreateRelease => _ => _
         .Description($"Creating release for the publishable version.")
-        .OnlyWhenStatic(() => Repository.IsOnMainOrMasterBranch() || Repository.IsOnReleaseBranch())
+        //Create release is not working just turn it off for now.
+        .OnlyWhenStatic(() => false  && (Repository.IsOnMainOrMasterBranch() || Repository.IsOnReleaseBranch()))
         .Executes(async () =>
         {
             var credentials = new Credentials(GitHubActions.Token);
@@ -225,7 +226,7 @@ static async Task UploadReleaseAssetToGithub(Release release, string asset)
 
             Log.Information("Https URL = {Value}", Repository.HttpsUrl);
             Log.Information("SSH URL = {Value}", Repository.SshUrl);
-            
+
             Log.Information("ChangeLogFile = {Value}", ChangeLogFile);
             Log.Information("SourceDirectory = {Value}", SourceDirectory);
             Log.Information("RootDirectory = {Value}", RootDirectory);

changelog.md

@@ -14,3 +14,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - Enhanced Roslyn analyzer to support Switch Expression Syntax. The analyzer now correctly identifies and validates 
  usage of `IsSuccess` and `Value` within C# switch expressions, providing a more comprehensive code quality check.
+## 1.4.1 / 2024-03-22
+- Addressed multiple issues related to pattern matching and complex conditional logic that were impacting code analysis accuracy.