Skip to content

Commit

Permalink
fix: Fix bugs and update packages (#42)
Browse files Browse the repository at this point in the history
* Update package versions

Updated CSharpFunctionalExtensions to 2.41.0, xunit to 2.7.0, and Roslynator.Testing.CSharp.Xunit to 4.12.0 across all projects to incorporate the latest fixes and features.

* Refactor condition checks and improve pattern matching

Remove unnecessary braces in conditional structures to simplify code and add handling cases for pattern matching that involve checking values directly on the result. This improves the robustness of the analyzer when dealing with more complex pattern matching scenarios.

* Remove deprecated analyzer class

The `UseResultValueWithoutCheck_older` class has been deleted as it was deprecated and replaced by newer implementations. This removal cleans up the codebase and reduces complexity.
  • Loading branch information
AlmarAubel authored Mar 22, 2024
1 parent a152710 commit 4405bad
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 304 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
OutputItemType="Analyzer" ReferenceOutputAssembly="false"/>
</ItemGroup>
<ItemGroup>
<PackageReference Include="CSharpFunctionalExtensions" Version="2.40.3" />
<PackageReference Include="CSharpFunctionalExtensions" Version="2.41.0" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
<PackageReference Include="xunit" Version="2.6.4" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.5.6">
<PackageReference Include="xunit" Version="2.7.0" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.5.3">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="coverlet.collector" Version="6.0.0">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="CSharpFunctionalExtensions" Version="2.40.3" />
<PackageReference Include="CSharpFunctionalExtensions" Version="2.41.0" />
<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"/>
<PackageReference Include="Roslynator.Testing.CSharp.Xunit" Version="4.6.2" />
<PackageReference Include="Roslynator.Testing.CSharp.Xunit" Version="4.12.0" />
</ItemGroup>


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ public class UseResultValueWithoutCheckPatternMatchingTests
[Theory]
[InlineData(" { IsSuccess: true } => result.Value")]
[InlineData(" { IsFailure: false } => result.Value")]
[InlineData(" { IsSuccess: true, Value: 1 } => result.Value")]
[InlineData(" { IsSuccess: true, Value: > 1 } => result.Value")]
public async Task TestNoDiagnostics_AccessValueAfterCheck(string source)
{
await VerifyNoDiagnosticAsync(AddPatternMatchingContext(source), options: CSharpTestOptions());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ await VerifyNoDiagnosticAsync(
[InlineData("if(result.IsSuccess) Console.WriteLine(result.Value);")]
[InlineData("if(result.IsSuccess == true) Console.WriteLine(result.Value);")]
[InlineData("if(result.IsSuccess && new Random().Next() > 1) Console.WriteLine(result.Value);")]
[InlineData("if(result is { IsSuccess: true, Value: > 1 }) Console.WriteLine(result.Value);")]
[InlineData("var x = result.IsSuccess ? result.Value: 0;")]
public async Task TestNoDiagnostic_AccesValueOnResultObject_WithCheckIsSuccess(string source)
{
Expand All @@ -59,6 +60,8 @@ public async Task TestNoDiagnostic_AccesValueOnResultObject_WithCheckIsSuccess(s
[InlineData("if(result.IsFailure == false) Console.WriteLine(result.Value);")]
[InlineData("if(!result.IsFailure && new Random().Next() > 1) Console.WriteLine(result.Value);")]
[InlineData("""if(result.IsFailure || result.Value > 1) Console.WriteLine("foo");""")]
[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;")]
public async Task TestNoDiagnostic_AccesValueOnResultObject_WithCheckIsFailure(string source)
{
Expand All @@ -82,8 +85,8 @@ await VerifyDiagnosticAsync(

[Theory]
[InlineData("if(result.IsFailure) Console.WriteLine([|result.Value|]);")]
[InlineData(" var x= result.IsFailure ? [|result.Value|]: 0;")]
[InlineData(" var x= result.IsSuccess ? 0: [|result.Value|];")]
[InlineData(" var x = result.IsFailure ? [|result.Value|]: 0;")]
[InlineData(" var x = result.IsSuccess ? 0: [|result.Value|];")]
public async Task Test_AccessValueAfterCheckForFailure(string source)
{
await VerifyDiagnosticAsync(AddContext(source), options: CSharpTestOptions());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="CSharpFunctionalExtensions" Version="2.40.3" >
<PackageReference Include="CSharpFunctionalExtensions" Version="2.41.0">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,16 @@ private void NodeWalkerInternal(SyntaxNode node)
{
//Fix if(result.IsFailure || result.Value > 1) Console.WriteLine("foo"); is marked als inccorrect usage
case IfStatementSyntax ifStatement:
_result.CheckResult = DetermineCheckResult(ifStatement.Condition);
NodeWalkerInternal(ifStatement);
if (_result.CorrectUsage) return;
if (_result is { CheckResult: CheckResult.CheckedSuccess, Terminated: true, AccessedValue: false })
{
_result.CheckResult = DetermineCheckResult(ifStatement.Condition);
NodeWalkerInternal(ifStatement);
if (_result.CorrectUsage) return;
if (_result is { CheckResult: CheckResult.CheckedSuccess, Terminated: true, AccessedValue: false })
{
_result.CheckResult = CheckResult.Unchecked;
}

_result.Terminated = false;
break;
_result.CheckResult = CheckResult.Unchecked;
}

_result.Terminated = false;
break;
case ReturnStatementSyntax or ThrowStatementSyntax:
_result.Terminated = true;
NodeWalkerInternal(child);
Expand Down Expand Up @@ -105,13 +103,13 @@ private CheckResult BinaryExpressionSyntax(BinaryExpressionSyntax binaryExpressi

if (rightResult == CheckResult.AccesedValue && leftResult == CheckResult.CheckedFailure)
return CheckResult.CheckedSuccess;

if (leftResult is CheckResult.Unchecked or CheckResult.CheckedFailure)
return leftResult;

if (rightResult == CheckResult.Unchecked)
return rightResult;


// If both sides are the same, return either; otherwise, it's ambiguous so return Unchecked.
return leftResult == rightResult ? leftResult : CheckResult.Unchecked;
Expand All @@ -126,7 +124,9 @@ private CheckResult BinaryExpressionSyntax(BinaryExpressionSyntax binaryExpressi

if (IsFailure(leftExpression, rightExpression))
return CheckResult.CheckedFailure;

if (DetermineCheckResult(binaryExpression.Left) == CheckResult.AccesedValue ||
DetermineCheckResult(binaryExpression.Right) == CheckResult.AccesedValue)
return CheckResult.AccesedValue;
break;
}
case SyntaxKind.ExclamationEqualsToken:
Expand Down Expand Up @@ -177,6 +177,14 @@ private CheckResult DetermineCheckResult(ExpressionSyntax condition)
return CheckResult.CheckedSuccess; // This means we found a !IsFailure, so it's equivalent to IsSuccess.
case ConditionalExpressionSyntax ternary:
return DetermineCheckResult(ternary.Condition);
case IsPatternExpressionSyntax isPatternExpressionSyntax:
return isPatternExpressionSyntax.Pattern switch
{
RecursivePatternSyntax recursivePatternSyntax => CheckedRecusivePattern(recursivePatternSyntax),
_ => throw new ArgumentOutOfRangeException()
};

break;
case SwitchExpressionSyntax switchExpressionSyntax:
throw new NotImplementedException();
}
Expand All @@ -200,7 +208,11 @@ private static CheckResult CheckSwitchExpressionArm(SwitchExpressionArmSyntax sw
{
var pattern = switchExpressionArm.Pattern;
// Todo check if switchExpression is on the result object
if (pattern is not RecursivePatternSyntax recursivePattern) return CheckResult.Unchecked;
return pattern is not RecursivePatternSyntax recursivePattern ? CheckResult.Unchecked : CheckedRecusivePattern(recursivePattern);
}

private static CheckResult CheckedRecusivePattern(RecursivePatternSyntax recursivePattern)
{
foreach (var propPattern in recursivePattern.PropertyPatternClause?.Subpatterns)
{
var name = (propPattern.Pattern as ConstantPatternSyntax)?.Expression.ToString();
Expand Down
Loading

0 comments on commit 4405bad

Please sign in to comment.