Skip to content

Commit 8974637

Browse files
shimatclaude
andcommitted
Address CodeRabbit review: tighten analyzer edge-case handling
MatExprChainAnalyzer (OCVS005): unwrap ParenthesizedExpressionSyntax when walking up to the parent binary expression so (a + b) + c is also detected. RowColInLoopBodyAnalyzer (OCVS003): restrict IsInsideLoopBody to only return true when the node is inside the loop's Statement (body), not in the for-loop initializer, condition, or iterator expressions. Uses FullSpan.Contains. RowColNotDisposedAnalyzer (OCVS004): narrow the member-access chain exemption from the blanket MemberAccessExpressionSyntax check to only At<T> calls (which OCVS001 already covers). mat.Row(i).Clone() now correctly reports OCVS004 since the Row() submatrix still leaks. Tests added for each edge case (27 total, all passing). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 601d388 commit 8974637

6 files changed

Lines changed: 88 additions & 10 deletions

File tree

src/OpenCvSharp.Analyzers/MatExprChainAnalyzer.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,11 @@ private static void AnalyzeBinaryExpression(SyntaxNodeAnalysisContext context)
6565

6666
// Flag only when this MatExpr is consumed immediately by another binary expression
6767
// without being assigned to a variable — that's the intermediate-temporary case.
68+
// Unwrap any parentheses: (a + b) + c has a ParenthesizedExpressionSyntax between the
69+
// inner binary and the outer one, but the semantics are identical.
6870
var parent = binary.Parent;
71+
while (parent is ParenthesizedExpressionSyntax)
72+
parent = parent.Parent;
6973
if (parent is BinaryExpressionSyntax or AssignmentExpressionSyntax { Left: not IdentifierNameSyntax })
7074
{
7175
var operatorToken = binary.OperatorToken.ToString();

src/OpenCvSharp.Analyzers/RowColInLoopBodyAnalyzer.cs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,18 @@ or LambdaExpressionSyntax
7373
or AnonymousMethodExpressionSyntax)
7474
return false;
7575

76-
// Loop constructs
77-
if (current is ForStatementSyntax
78-
or WhileStatementSyntax
79-
or DoStatementSyntax
80-
or ForEachStatementSyntax)
81-
return true;
76+
// Loop constructs: only flag when node is inside the loop body (Statement),
77+
// not in the initializer, condition, or iterator of a for/foreach.
78+
StatementSyntax? loopBody = current switch
79+
{
80+
ForStatementSyntax f => f.Statement,
81+
WhileStatementSyntax w => w.Statement,
82+
DoStatementSyntax d => d.Statement,
83+
ForEachStatementSyntax fe => fe.Statement,
84+
_ => null,
85+
};
86+
if (loopBody is not null)
87+
return loopBody.FullSpan.Contains(node.FullSpan);
8288
}
8389
return false;
8490
}

src/OpenCvSharp.Analyzers/RowColNotDisposedAnalyzer.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,12 @@ private static bool IsDisposed(InvocationExpressionSyntax invocation)
8484
if (parent is UsingStatementSyntax)
8585
return true;
8686

87-
// mat.Row(i).SomeMethod() — member access chain; OCVS001 handles the At<T> case,
88-
// and a chained call generally means the caller is using the result.
89-
// Skip to avoid double-warning.
90-
if (parent is MemberAccessExpressionSyntax)
87+
// mat.Row(i).At<T>(col) — OCVS001 already flags this exact pattern; skip here
88+
// to avoid a double warning. Only At<T> is exempted; other chains like .Clone()
89+
// still leak the submatrix and should be reported.
90+
if (parent is MemberAccessExpressionSyntax chainedMa
91+
&& chainedMa.Parent is InvocationExpressionSyntax
92+
&& chainedMa.Name.Identifier.Text == "At")
9193
return true;
9294

9395
// Passed as argument — caller is responsible

test/OpenCvSharp.Analyzers.Tests/MatExprChainAnalyzerTests.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,20 @@ void M(OpenCvSharp.Mat a, OpenCvSharp.Mat b)
6363
}
6464
""");
6565

66+
[Fact]
67+
public Task ParenthesizedChain_ReportsWarning() => Verify(
68+
"""
69+
class Test
70+
{
71+
void M(OpenCvSharp.Mat a, OpenCvSharp.Mat b, OpenCvSharp.Mat c)
72+
{
73+
OpenCvSharp.Mat result = ({|#0:a + b|}) + c;
74+
}
75+
}
76+
""",
77+
DiagnosticResult.CompilerWarning(MatExprChainAnalyzer.DiagnosticId)
78+
.WithLocation(0).WithArguments("+"));
79+
6680
[Fact]
6781
public Task UsingMatExprVariable_NoWarning() => Verify(
6882
"""

test/OpenCvSharp.Analyzers.Tests/RowColInLoopBodyAnalyzerTests.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,30 @@ void M(OpenCvSharp.Mat mat)
7878
}
7979
""");
8080

81+
[Fact]
82+
public Task RowInForLoopCondition_NoWarning() => Verify(
83+
"""
84+
class Test
85+
{
86+
void M(OpenCvSharp.Mat mat)
87+
{
88+
for (int i = 0; i < mat.Row(0).Rows; i++) { }
89+
}
90+
}
91+
""");
92+
93+
[Fact]
94+
public Task RowInForLoopInitializer_NoWarning() => Verify(
95+
"""
96+
class Test
97+
{
98+
void M(OpenCvSharp.Mat mat)
99+
{
100+
for (var r = mat.Row(0); ; ) { }
101+
}
102+
}
103+
""");
104+
81105
[Fact]
82106
public Task RowInNestedLambda_NoWarning() => Verify(
83107
"""

test/OpenCvSharp.Analyzers.Tests/RowColNotDisposedAnalyzerTests.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ public class Mat : System.IDisposable
1717
public Mat Col(int x) => this;
1818
public Mat RowRange(int start, int end) => this;
1919
public Mat ColRange(int start, int end) => this;
20+
public T At<T>(int i0) where T : struct => default;
21+
public Mat Clone() => new Mat();
2022
public void Dispose() { }
2123
}
2224
}
@@ -83,6 +85,32 @@ void M(OpenCvSharp.Mat mat)
8385
}
8486
""");
8587

88+
[Fact]
89+
public Task ChainedAtCall_NoWarning() => Verify(
90+
"""
91+
class Test
92+
{
93+
void M(OpenCvSharp.Mat mat)
94+
{
95+
int v = mat.Row(0).At<int>(3);
96+
}
97+
}
98+
""");
99+
100+
[Fact]
101+
public Task ChainedCloneCall_ReportsWarning() => Verify(
102+
"""
103+
class Test
104+
{
105+
void M(OpenCvSharp.Mat mat)
106+
{
107+
var copy = {|#0:mat.Row(0)|}.Clone();
108+
}
109+
}
110+
""",
111+
DiagnosticResult.CompilerWarning(RowColNotDisposedAnalyzer.DiagnosticId)
112+
.WithLocation(0).WithArguments("Row"));
113+
86114
[Fact]
87115
public Task ColRangeWithoutUsing_ReportsWarning() => Verify(
88116
"""

0 commit comments

Comments
 (0)