fix: consolidate SonarCloud cleanup into main#83
Conversation
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
…load fix: resolve SonarCloud issues in PayloadGenerator
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
Qodana Community for .NETIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2026.1.3
with:
upload-result: trueContact Qodana teamContact us at qodana-support@jetbrains.com
|
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| for (var y = 0; y < x; y++) | ||
| { | ||
| for (var y = 0; y < x; y++) | ||
| if (!IsBlocked(new SKRectI(x, y, 1, 1), blockedModules)) | ||
| { | ||
| if (!IsBlocked(new SKRectI(x, y, 1, 1), blockedModules)) | ||
| { | ||
| qrCode.ModuleMatrix[y][x] ^= methods[selectedPattern.Value](x, y); | ||
| qrCode.ModuleMatrix[x][y] ^= methods[selectedPattern.Value](y, x); | ||
| } | ||
| qrCode.ModuleMatrix[y][x] ^= pattern(x, y); | ||
| qrCode.ModuleMatrix[x][y] ^= pattern(y, x); | ||
| } |
There was a problem hiding this comment.
📝 Info: ApplyMask shares a single IsBlocked check for two symmetric module positions
The extracted ApplyMask method at QRCoder.Core/Generators/QRCodeGenerator.cs:476-493 faithfully preserves a pre-existing behavior: when y < x, a single IsBlocked(new SKRectI(x, y, 1, 1), blockedModules) check gates XOR operations on both ModuleMatrix[y][x] and ModuleMatrix[x][y]. This means position (y, x) (the transposed cell) is never independently checked against blockedModules. If (x, y) is blocked but (y, x) is not, neither gets masked. If (x, y) is unblocked but (y, x) is blocked, the blocked cell still gets masked. This was identical in the old code and may be intentional (the blocked regions are symmetric in valid QR codes), but it's worth being aware of if the blocking logic ever changes.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for (var y = yStart; y != yEnd && datawords.Count > 0; y += yStep) | ||
| { | ||
| int y; | ||
| if (up) | ||
| { | ||
| y = size - yMod; | ||
| if (datawords.Count > 0 && !IsBlocked(new SKRectI(x, y, 1, 1), blockedModules)) | ||
| qrCode.ModuleMatrix[y][x] = datawords.Dequeue(); | ||
| if (datawords.Count > 0 && x > 0 && !IsBlocked(new SKRectI(x - 1, y, 1, 1), blockedModules)) | ||
| qrCode.ModuleMatrix[y][x - 1] = datawords.Dequeue(); | ||
| } | ||
| else | ||
| TryPlaceDataWord(qrCode, x, y, datawords, blockedModules); | ||
| if (x > 0) | ||
| { | ||
| y = yMod - 1; | ||
| if (datawords.Count > 0 && !IsBlocked(new SKRectI(x, y, 1, 1), blockedModules)) | ||
| qrCode.ModuleMatrix[y][x] = datawords.Dequeue(); | ||
| if (datawords.Count > 0 && x > 0 && !IsBlocked(new SKRectI(x - 1, y, 1, 1), blockedModules)) | ||
| qrCode.ModuleMatrix[y][x - 1] = datawords.Dequeue(); | ||
| TryPlaceDataWord(qrCode, x - 1, y, datawords, blockedModules); | ||
| } | ||
| } | ||
|
|
||
| up = !up; | ||
| x -= 2; | ||
| } |
There was a problem hiding this comment.
📝 Info: PlaceDataWords inner loop now exits early when data is exhausted
The refactored PlaceDataWords adds datawords.Count > 0 to the inner for-loop condition at line 516. The old code always iterated all y values for every x column pair, relying on the datawords.Count > 0 guard inside each placement call. The new code exits the y-loop as soon as data runs out. This is functionally equivalent (the old code did nothing with empty data), but it's a minor performance improvement — the loop no longer iterates over remaining y positions once all data bits are placed. The outer x-loop still runs to completion, toggling up and decrementing x, which has no effect since the inner loop will immediately exit.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
| else | ||
| { | ||
| var resPoly = MultiplyGeneratorPolynomByLeadterm(generatorPolynom, ConvertToAlphaNotation(leadTermSource).PolyItems[0], i); | ||
| var resPoly = MultiplyGeneratorPolynomByLeadterm(generatorPolynom, ConvertToAlphaNotation(leadTermSource).PolyItems[0], iteration); | ||
| resPoly = ConvertToDecNotation(resPoly); | ||
| resPoly = XORPolynoms(leadTermSource, resPoly); | ||
| leadTermSource = resPoly; | ||
| } | ||
|
|
||
| iteration++; | ||
| } |
There was a problem hiding this comment.
📝 Info: CalculateECCWords iteration counter increments on both branches, matching original for-loop
The conversion from for (var i = 0; ...; i++) to a while loop with a separate iteration variable at QRCoder.Core/Generators/QRCodeGenerator.cs:1042-1060 is correct but has a subtle aspect worth noting: iteration increments on every loop pass (line 1059), including when the if branch (coefficient == 0) is taken and MultiplyGeneratorPolynomByLeadterm is not called. This matches the original for loop where i++ ran unconditionally. The iteration value is only consumed in the else branch as the lowerExponentBy parameter, so skipped iterations effectively increase the exponent reduction on the next non-zero coefficient — the same behavior as before.
(Refers to lines 1042-1060)
Was this helpful? React with 👍 or 👎 to provide feedback.
| protected virtual void Dispose(bool disposing) | ||
| { | ||
| if (_disposedValue) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _disposedValue = true; | ||
| } |
There was a problem hiding this comment.
📝 Info: Dispose pattern added but no resources are actually released
The new Dispose(bool disposing) method at lines 1589-1597 implements the standard dispose pattern but the body only sets _disposedValue = true without releasing any resources. The disposing parameter is not even checked. Compare this with QRCodeData.Dispose(bool) at QRCoder.Core/Models/QRCodeData.cs:214-221 which actually nulls out ModuleMatrix. The QRCodeGenerator class holds no instance state (all fields are static), so there's nothing to dispose — this is purely for pattern compliance. However, the disposing parameter should arguably still be checked per the standard pattern, even if the body is empty, to be consistent with QRCodeData's implementation.
Was this helpful? React with 👍 or 👎 to provide feedback.
| private static QRCodeData CopyMatrix(QRCodeData source, int size) | ||
| { | ||
| var copy = new QRCodeData(source.Version); | ||
| for (var y = 0; y < size; y++) | ||
| { | ||
| for (var x = 0; x < size; x++) | ||
| { | ||
| copy.ModuleMatrix[y][x] = source.ModuleMatrix[y][x]; | ||
| } | ||
| } | ||
| return copy; | ||
| } |
There was a problem hiding this comment.
📝 Info: CopyMatrix uses source.Version instead of the size-derived version parameter
The CopyMatrix helper at line 463-474 constructs the copy via new QRCodeData(source.Version) rather than accepting a separate version parameter. The old inline code in MaskCode used the version parameter passed to MaskCode. These are always equal in practice because PlaceModules creates QRCodeData(version) and passes the same version to MaskCode. Using source.Version is arguably more robust since it's self-consistent with the source data, but it's a semantic change worth noting — if someone ever called MaskCode with a mismatched version, the behavior would differ from the old code.
Was this helpful? React with 👍 or 👎 to provide feedback.
…derers-core fix: resolve SonarCloud issues in core renderers and extensions
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
…derers-others fix: resolve SonarCloud issues in remaining renderers
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
…pose-fix fix: dispose SKPaint and SKBitmap in renderer icon/dot helpers
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
…l-guard fix: add null guard in HexSKColorToByteArray to resolve S2259
Resize was creating an empty scaledImage and returning it without drawing the source image, so the background image was invisible. Use image.Resize() to scale and dispose the temporary bitmap, and dispose the resized result in RenderGraphicCore. Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
…t ratios Ensure scaled dimensions are at least 1 pixel before calling image.Resize(). Create the output bitmap only after a successful resize to prevent leaking it if the resize operation fails. Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
…ize-fix fix(ArtQRCode): scale and dispose background image
devin-ai-integration
Bot
changed the title
- Cast SKRect/DrawBitmap multiplications to float before evaluation to avoid int overflow. - Guard nullable ImageAttributes in SvgQRCode.IsBlockedByLogo. - Update renderer tests to use non-obsolete options overloads. - Correct PayloadGeneratorTests pragma (CS0612 -> CS0618). Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
| PixelsPerModule = 10, | ||
| DarkSKColor = SKColors.Black, | ||
| LightSKColor = SKColors.Transparent, | ||
| Icon = SKBitmap.Decode(System.IO.Path.Combine(HelperFunctions.GetAssemblyPath(), "assets", "noun_software-engineer_2909346.png")) |
| PixelsPerModule = 10, | ||
| DarkSKColor = SKColors.Black, | ||
| LightSKColor = SKColors.White, | ||
| Icon = SKBitmap.Decode(System.IO.Path.Combine(HelperFunctions.GetAssemblyPath(), "assets", "noun_software-engineer_2909346.png")) |
The workflow expects a qodana.sarif.json baseline; without it Qodana compares against an empty report and reports all 1225 pre-existing issues as NEW. This baseline was generated with Qodana Community for .NET 2026.1 against the integration branch and will be used by PRs targeting main after merge. Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
|
2 participants
All Submissions:
Changes to Core Features:
Summary
This is the consolidation PR from the
feature/devin-20260712-sonar-qualityintegration branch intomain. It contains the SonarCloud cleanup work executed in phases during this session.Phases merged into the integration branch:
#81)#82)#84)#85)#86)#87)#88)Verification:
dotnet build QRCoder.Core.sln --configuration Releasepasses for all target frameworks.dotnet test QRCoder.Core.Tests/ -f net10.0 --configuration Releasepasses (502 tests).dotnet test QRCoder.Core.Tests/ -f net8.0 --configuration Releasepasses (502 tests).net48tests do not run on Linux due to missingmono.Public APIs are preserved; obsolete overloads are kept with
[Obsolete]and[SuppressMessage]where needed.Link to Devin session: https://app.devin.ai/sessions/20c92a1bad724f63b8be89a9e195e664
Requested by: @afonsoft