Merge pull request #1874 from shimat/claude/exciting-jackson-eaf946

Fix Row()/At<T> misuse and add Span-based pixel access APIs (fixes #1775)

Author: shimat

src/OpenCvSharp.Analyzers/AnalyzerReleases.Shipped.md

@@ -0,0 +1,2 @@
+; Shipped analyzer releases
+; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md

src/OpenCvSharp.Analyzers/AnalyzerReleases.Unshipped.md

@@ -0,0 +1,11 @@
+; Unshipped analyzer releases
+
+### New Rules
+
+Rule ID | Category | Severity | Notes
+--------|----------|----------|-------
+OCVS001 | Correctness | Warning  | At<T>(int) called on a Mat row submatrix
+OCVS002 | Performance | Warning  | Mat property (Rows/Cols/Dims/Width/Height) in loop condition
+OCVS003 | Performance | Warning  | Mat.Row() / Mat.Col() called inside a loop body
+OCVS004 | Reliability | Warning  | Mat submatrix (Row/Col/RowRange/ColRange) not disposed
+OCVS005 | Reliability | Warning  | Intermediate MatExpr not disposed in chained Mat arithmetic

src/OpenCvSharp.Analyzers/MatExprChainAnalyzer.cs

@@ -0,0 +1,79 @@
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace OpenCvSharp.Analyzers;
+
+/// <summary>
+/// Detects chained arithmetic on <c>Mat</c> (e.g. <c>a + b + c</c>) where intermediate
+/// <c>MatExpr</c> objects are never disposed.
+/// </summary>
+/// <remarks>
+/// <para>
+/// <c>Mat</c> arithmetic operators return <c>MatExpr</c> (an <see cref="System.IDisposable"/> wrapper
+/// around a native lazy-evaluation expression). In a chain such as <c>a + b + c</c>, the
+/// <c>MatExpr</c> produced by <c>a + b</c> is consumed by the second <c>+</c> and never disposed,
+/// leaving the native object to the GC finalizer.
+/// </para>
+/// <para>
+/// Prefer <c>Cv2.Add</c> / <c>Cv2.Subtract</c> etc., or assign each step to a <c>using</c>
+/// variable.
+/// </para>
+/// </remarks>
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class MatExprChainAnalyzer : DiagnosticAnalyzer
+{
+    public const string DiagnosticId = "OCVS005";
+
+    private static readonly DiagnosticDescriptor Rule = new(
+        id: DiagnosticId,
+        title: "Intermediate MatExpr not disposed in chained Mat arithmetic",
+        messageFormat: "The intermediate MatExpr from '{0}' is never disposed. " +
+                       "Use Cv2.Add/Subtract/Multiply/Divide, or assign each step to a 'using' variable.",
+        category: "Reliability",
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        description: "Mat arithmetic operators return MatExpr, an IDisposable wrapping a native " +
+                     "lazy-evaluation object. In a chain like a + b + c, the MatExpr for a + b " +
+                     "is consumed without being disposed, leaking the native resource until " +
+                     "the GC finalizer runs.",
+        helpLinkUri: "https://github.com/shimat/opencvsharp/issues/1775");
+
+    private const string MatExprFullName = "OpenCvSharp.MatExpr";
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
+        => ImmutableArray.Create(Rule);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+        context.RegisterSyntaxNodeAction(AnalyzeBinaryExpression, SyntaxKind.AddExpression,
+            SyntaxKind.SubtractExpression, SyntaxKind.MultiplyExpression, SyntaxKind.DivideExpression);
+    }
+
+    private static void AnalyzeBinaryExpression(SyntaxNodeAnalysisContext context)
+    {
+        var binary = (BinaryExpressionSyntax)context.Node;
+
+        // The result of this expression must be MatExpr
+        var resultType = context.SemanticModel.GetTypeInfo(binary).Type;
+        if (resultType?.ToDisplayString() != MatExprFullName)
+            return;
+
+        // Flag only when this MatExpr is consumed immediately by another binary expression
+        // without being assigned to a variable — that's the intermediate-temporary case.
+        // Unwrap any parentheses: (a + b) + c has a ParenthesizedExpressionSyntax between the
+        // inner binary and the outer one, but the semantics are identical.
+        var parent = binary.Parent;
+        while (parent is ParenthesizedExpressionSyntax)
+            parent = parent.Parent;
+        if (parent is BinaryExpressionSyntax or AssignmentExpressionSyntax { Left: not IdentifierNameSyntax })
+        {
+            var operatorToken = binary.OperatorToken.ToString();
+            context.ReportDiagnostic(Diagnostic.Create(Rule, binary.GetLocation(), operatorToken));
+        }
+    }
+}

src/OpenCvSharp.Analyzers/MatPropertyInLoopConditionAnalyzer.cs

@@ -0,0 +1,85 @@
+using System;
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace OpenCvSharp.Analyzers;
+
+/// <summary>
+/// Detects <c>mat.Rows</c>, <c>mat.Cols</c>, <c>mat.Dims</c>, <c>mat.Width</c>, or <c>mat.Height</c>
+/// used in loop conditions, where each access is a P/Invoke call evaluated on every iteration.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class MatPropertyInLoopConditionAnalyzer : DiagnosticAnalyzer
+{
+    public const string DiagnosticId = "OCVS002";
+
+    private static readonly DiagnosticDescriptor Rule = new(
+        id: DiagnosticId,
+        title: "Mat property accessed in loop condition",
+        messageFormat: "'{0}' is a P/Invoke call on every iteration; cache it before the loop as 'int {1} = mat.{0};'",
+        category: "Performance",
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        description: "Mat.Rows, Mat.Cols, Mat.Dims, Mat.Width, and Mat.Height each invoke a native " +
+                     "P/Invoke call. Placing them in a loop condition causes one P/Invoke call per " +
+                     "iteration. Cache the value in a local variable before the loop.",
+        helpLinkUri: "https://github.com/shimat/opencvsharp/issues/1775");
+
+    // Properties on OpenCvSharp.Mat that are P/Invoke calls
+    private static readonly ImmutableHashSet<string> ExpensiveProperties =
+        ImmutableHashSet.Create(StringComparer.Ordinal, "Rows", "Cols", "Dims", "Width", "Height");
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
+        => ImmutableArray.Create(Rule);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+        context.RegisterSyntaxNodeAction(AnalyzeFor,     SyntaxKind.ForStatement);
+        context.RegisterSyntaxNodeAction(AnalyzeWhile,   SyntaxKind.WhileStatement);
+        context.RegisterSyntaxNodeAction(AnalyzeDo,      SyntaxKind.DoStatement);
+    }
+
+    private static void AnalyzeFor(SyntaxNodeAnalysisContext context)
+    {
+        var forLoop = (ForStatementSyntax)context.Node;
+        if (forLoop.Condition is not null)
+            CheckExpression(forLoop.Condition, context);
+    }
+
+    private static void AnalyzeWhile(SyntaxNodeAnalysisContext context)
+    {
+        var whileLoop = (WhileStatementSyntax)context.Node;
+        CheckExpression(whileLoop.Condition, context);
+    }
+
+    private static void AnalyzeDo(SyntaxNodeAnalysisContext context)
+    {
+        var doLoop = (DoStatementSyntax)context.Node;
+        CheckExpression(doLoop.Condition, context);
+    }
+
+    private static void CheckExpression(ExpressionSyntax condition, SyntaxNodeAnalysisContext context)
+    {
+        foreach (var node in condition.DescendantNodesAndSelf())
+        {
+            if (node is not MemberAccessExpressionSyntax ma)
+                continue;
+
+            var propName = ma.Name.Identifier.ValueText;
+            if (!ExpensiveProperties.Contains(propName))
+                continue;
+
+            var symbol = context.SemanticModel.GetSymbolInfo(ma).Symbol as IPropertySymbol;
+            if (symbol?.ContainingType?.ToDisplayString() != "OpenCvSharp.Mat")
+                continue;
+
+            var suggestedName = char.ToLowerInvariant(propName[0]) + propName.Substring(1);
+            context.ReportDiagnostic(Diagnostic.Create(Rule, ma.GetLocation(), propName, suggestedName));
+        }
+    }
+}

src/OpenCvSharp.Analyzers/OpenCvSharp.Analyzers.csproj

@@ -0,0 +1,34 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>netstandard2.0</TargetFramework>
+    <LangVersion>12</LangVersion>
+    <Nullable>enable</Nullable>
+    <RootNamespace>OpenCvSharp.Analyzers</RootNamespace>
+    <AssemblyName>OpenCvSharp.Analyzers</AssemblyName>
+    <!-- AssemblyVersion must be a valid major.minor.build.revision (each ≤ 65535).
+         The repo uses date-based Version (e.g. 4.13.0.20260524) which exceeds that limit,
+         so we pin the assembly version independently. -->
+    <AssemblyVersion>1.0.0.0</AssemblyVersion>
+    <FileVersion>1.0.0.0</FileVersion>
+    <IsRoslynComponent>true</IsRoslynComponent>
+    <!-- EnforceExtendedAnalyzerRules ensures the analyzer follows Roslyn analyzer best practices -->
+    <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>
+    <!-- Suppress build output from the package; it goes into analyzers/ folder only -->
+    <IncludeBuildOutput>false</IncludeBuildOutput>
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
+    <!-- DiagnosticId, SupportedDiagnostics, Initialize are required overrides; XML comments not needed -->
+    <NoWarn>CS1591</NoWarn>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.8.0" PrivateAssets="all" />
+    <PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4" PrivateAssets="all" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <AdditionalFiles Include="AnalyzerReleases.Shipped.md" />
+    <AdditionalFiles Include="AnalyzerReleases.Unshipped.md" />
+  </ItemGroup>
+
+</Project>

src/OpenCvSharp.Analyzers/RowAtAnalyzer.cs

@@ -0,0 +1,128 @@
+using System.Collections.Immutable;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace OpenCvSharp.Analyzers;
+
+/// <summary>
+/// Detects the misuse pattern <c>mat.Row(i).At&lt;T&gt;(col)</c> where the column index is
+/// silently interpreted as a row index, causing out-of-bounds memory access.
+/// </summary>
+/// <remarks>
+/// <para>
+/// <c>Mat.Row(i)</c> returns a 1×N submatrix. <c>At&lt;T&gt;(int i0)</c> on a 2D matrix
+/// treats <c>i0</c> as the row index (dimension 0), not a column index. The resulting
+/// address is <c>data + step * col</c> — jumping entire rows past the end of the submatrix.
+/// </para>
+/// <para>
+/// Use <c>mat.At&lt;T&gt;(row, col)</c> or <c>mat.AsRows&lt;T&gt;()</c> instead.
+/// See https://github.com/shimat/opencvsharp/issues/1775
+/// </para>
+/// </remarks>
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class RowAtAnalyzer : DiagnosticAnalyzer
+{
+    public const string DiagnosticId = "OCVS001";
+
+    private static readonly DiagnosticDescriptor Rule = new(
+        id: DiagnosticId,
+        title: "At<T>(int) called on a Mat row submatrix",
+        messageFormat: "At<T>(int) on the 1-row Mat returned by Row() treats the argument as a row index, " +
+                       "not a column index, silently producing wrong results or out-of-bounds access. " +
+                       "Use mat.At<T>(row, col) or mat.AsRows<T>() instead.",
+        category: "Correctness",
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        description: "Mat.Row(i) returns a 2D 1×N submatrix. At<T>(int i0) interprets i0 as a row " +
+                     "index (dimension 0), not a column index. Access the parent matrix directly: " +
+                     "mat.At<T>(row, col), or use mat.AsRows<T>() for high-performance loops.",
+        helpLinkUri: "https://github.com/shimat/opencvsharp/issues/1775");
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
+        => ImmutableArray.Create(Rule);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+        context.RegisterSyntaxNodeAction(AnalyzeInvocation, SyntaxKind.InvocationExpression);
+    }
+
+    private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
+    {
+        var atInvocation = (InvocationExpressionSyntax)context.Node;
+
+        if (!IsMatAtIntCall(atInvocation, context.SemanticModel))
+            return;
+
+        if (atInvocation.Expression is not MemberAccessExpressionSyntax memberAccess)
+            return;
+
+        var receiver = memberAccess.Expression;
+
+        // Case 1: direct chain — mat.Row(i).At<T>(c)
+        if (IsMatRowCall(receiver, context.SemanticModel))
+        {
+            context.ReportDiagnostic(Diagnostic.Create(Rule, atInvocation.GetLocation()));
+            return;
+        }
+
+        // Case 2: local variable — var row = mat.Row(i); row.At<T>(c)
+        if (receiver is IdentifierNameSyntax identifier &&
+            context.SemanticModel.GetSymbolInfo(identifier).Symbol is ILocalSymbol local &&
+            IsLocalDeclaredFromRowCall(local, context.SemanticModel))
+        {
+            context.ReportDiagnostic(Diagnostic.Create(Rule, atInvocation.GetLocation()));
+        }
+    }
+
+    /// <summary>
+    /// Returns true when <paramref name="expr"/> is a call to <c>OpenCvSharp.Mat.At&lt;T&gt;(int)</c>.
+    /// </summary>
+    private static bool IsMatAtIntCall(InvocationExpressionSyntax expr, SemanticModel model)
+    {
+        if (expr.Expression is not MemberAccessExpressionSyntax ma)
+            return false;
+        if (ma.Name.Identifier.ValueText != "At")
+            return false;
+        if (expr.ArgumentList.Arguments.Count != 1)
+            return false;
+
+        var sym = model.GetSymbolInfo(expr).Symbol as IMethodSymbol;
+        return sym is { Parameters.Length: 1 } &&
+               sym.ContainingType?.ToDisplayString() == "OpenCvSharp.Mat" &&
+               sym.Parameters[0].Type.SpecialType == SpecialType.System_Int32;
+    }
+
+    /// <summary>
+    /// Returns true when <paramref name="expr"/> is a call to <c>OpenCvSharp.Mat.Row(int)</c>.
+    /// </summary>
+    private static bool IsMatRowCall(ExpressionSyntax expr, SemanticModel model)
+    {
+        if (expr is not InvocationExpressionSyntax inv)
+            return false;
+        if (inv.Expression is not MemberAccessExpressionSyntax ma)
+            return false;
+        if (ma.Name.Identifier.ValueText != "Row")
+            return false;
+
+        var sym = model.GetSymbolInfo(inv).Symbol as IMethodSymbol;
+        return sym?.ContainingType?.ToDisplayString() == "OpenCvSharp.Mat";
+    }
+
+    /// <summary>
+    /// Returns true when the local variable's declaration initializer is a call to <c>Mat.Row(int)</c>.
+    /// Covers: <c>var row = mat.Row(i);</c>
+    /// </summary>
+    private static bool IsLocalDeclaredFromRowCall(ILocalSymbol local, SemanticModel model)
+    {
+        var syntaxRef = local.DeclaringSyntaxReferences.FirstOrDefault();
+        if (syntaxRef?.GetSyntax() is not VariableDeclaratorSyntax { Initializer.Value: { } initializer })
+            return false;
+
+        return IsMatRowCall(initializer, model);
+    }
+}