Feature/devin 20260712 sonar quality#80
Conversation
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>
…kflows-plus fix(ci): corrigir workflows legados
…plexity-generators fix: reduzir complexidade do gerador e payload
|
Qodana Community for .NET1251 new problems were found
💡 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
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| resPoly = XORPolynoms(leadTermSource, resPoly); | ||
| leadTermSource = resPoly; | ||
| var blackModules = qrCode.ModuleMatrix.Sum(row => row.Cast<bool>().Count(x => x)); | ||
| var percent = ((double)blackModules / (size * size)) * 100; |
| } | ||
|
|
||
| private static bool IsInRange(char c, char min, char max) | ||
| { | ||
| return (uint)(c - min) <= (uint)(max - min); | ||
| } | ||
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| for (var x = size - 1; x >= 0; x = x - 2) | ||
| { | ||
| var currentX = x == 6 ? 5 : x; | ||
| if (x == 6) | ||
| x = 5; | ||
| for (var yMod = 1; yMod <= size; yMod++) | ||
| { | ||
| int y; | ||
| if (up) | ||
| { | ||
| y = size - yMod; | ||
| if (datawords.Count > 0 && !IsBlocked(new SKRectI(currentX, y, 1, 1), blockedModules)) | ||
| qrCode.ModuleMatrix[y][currentX] = datawords.Dequeue(); | ||
| if (datawords.Count > 0 && currentX > 0 && !IsBlocked(new SKRectI(currentX - 1, y, 1, 1), blockedModules)) | ||
| qrCode.ModuleMatrix[y][currentX - 1] = datawords.Dequeue(); | ||
| 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 | ||
| { | ||
| y = yMod - 1; | ||
| if (datawords.Count > 0 && !IsBlocked(new SKRectI(currentX, y, 1, 1), blockedModules)) | ||
| qrCode.ModuleMatrix[y][currentX] = datawords.Dequeue(); | ||
| if (datawords.Count > 0 && currentX > 0 && !IsBlocked(new SKRectI(currentX - 1, y, 1, 1), blockedModules)) | ||
| qrCode.ModuleMatrix[y][currentX - 1] = datawords.Dequeue(); | ||
| 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(); | ||
| } | ||
| } | ||
| up = !up; | ||
| x = currentX - 2; | ||
| } |
There was a problem hiding this comment.
📝 Info: PlaceDataWords loop refactoring requires careful equivalence verification
The old code used a while loop with a currentX variable to skip column 6 (the timing pattern column). When x == 6, currentX was set to 5, and then x = currentX - 2 = 3. The new code at QRCoder.Core/Generators/QRCodeGenerator.cs:495-519 uses a for loop with x = x - 2 in the update expression and an if (x == 6) x = 5 guard at the top of the loop body. Both produce the same sequence of column values (e.g., ..., 8, 5, 3, 1) because when x reaches 6, it's immediately adjusted to 5, and then the loop body uses 5. The for-loop update then sets x = 5 - 2 = 3. This is mechanically equivalent but the reasoning is non-obvious — worth a second look to confirm confidence.
Was this helpful? React with 👍 or 👎 to provide feedback.
| private static int ScoreFinderPatterns(QRCodeData qrCode, int size) | ||
| { | ||
| var score = 0; | ||
| for (var y = 0; y < size; y++) | ||
| { | ||
| for (var x = 0; x < size - 10; x++) | ||
| { | ||
| if ((qrCode.ModuleMatrix[y][x] && | ||
| !qrCode.ModuleMatrix[y][x + 1] && | ||
| qrCode.ModuleMatrix[y][x + 2] && | ||
| qrCode.ModuleMatrix[y][x + 3] && | ||
| qrCode.ModuleMatrix[y][x + 4] && | ||
| !qrCode.ModuleMatrix[y][x + 5] && | ||
| qrCode.ModuleMatrix[y][x + 6] && | ||
| !qrCode.ModuleMatrix[y][x + 7] && | ||
| !qrCode.ModuleMatrix[y][x + 8] && | ||
| !qrCode.ModuleMatrix[y][x + 9] && | ||
| !qrCode.ModuleMatrix[y][x + 10]) || | ||
| (!qrCode.ModuleMatrix[y][x] && | ||
| !qrCode.ModuleMatrix[y][x + 1] && | ||
| !qrCode.ModuleMatrix[y][x + 2] && | ||
| !qrCode.ModuleMatrix[y][x + 3] && | ||
| qrCode.ModuleMatrix[y][x + 4] && | ||
| !qrCode.ModuleMatrix[y][x + 5] && | ||
| qrCode.ModuleMatrix[y][x + 6] && | ||
| qrCode.ModuleMatrix[y][x + 7] && | ||
| qrCode.ModuleMatrix[y][x + 8] && | ||
| !qrCode.ModuleMatrix[y][x + 9] && | ||
| qrCode.ModuleMatrix[y][x + 10])) | ||
| { | ||
| score3 += 40; | ||
| } | ||
|
|
||
| if ((qrCode.ModuleMatrix[x][y] && | ||
| !qrCode.ModuleMatrix[x + 1][y] && | ||
| qrCode.ModuleMatrix[x + 2][y] && | ||
| qrCode.ModuleMatrix[x + 3][y] && | ||
| qrCode.ModuleMatrix[x + 4][y] && | ||
| !qrCode.ModuleMatrix[x + 5][y] && | ||
| qrCode.ModuleMatrix[x + 6][y] && | ||
| !qrCode.ModuleMatrix[x + 7][y] && | ||
| !qrCode.ModuleMatrix[x + 8][y] && | ||
| !qrCode.ModuleMatrix[x + 9][y] && | ||
| !qrCode.ModuleMatrix[x + 10][y]) || | ||
| (!qrCode.ModuleMatrix[x][y] && | ||
| !qrCode.ModuleMatrix[x + 1][y] && | ||
| !qrCode.ModuleMatrix[x + 2][y] && | ||
| !qrCode.ModuleMatrix[x + 3][y] && | ||
| qrCode.ModuleMatrix[x + 4][y] && | ||
| !qrCode.ModuleMatrix[x + 5][y] && | ||
| qrCode.ModuleMatrix[x + 6][y] && | ||
| qrCode.ModuleMatrix[x + 7][y] && | ||
| qrCode.ModuleMatrix[x + 8][y] && | ||
| !qrCode.ModuleMatrix[x + 9][y] && | ||
| qrCode.ModuleMatrix[x + 10][y])) | ||
| { | ||
| score3 += 40; | ||
| } | ||
| if (MatchesFinderPattern(qrCode, x, y)) | ||
| score += 40; | ||
| } | ||
| } | ||
|
|
||
| //Penalty 4 | ||
| double blackModules = 0; | ||
| foreach (var row in qrCode.ModuleMatrix) | ||
| foreach (bool bit in row) | ||
| if (bit) | ||
| blackModules++; | ||
|
|
||
| var percent = (blackModules / (qrCode.ModuleMatrix.Count * qrCode.ModuleMatrix.Count)) * 100; | ||
| var prevMultipleOf5 = Math.Abs((int)Math.Floor(percent / 5) * 5 - 50) / 5; | ||
| var nextMultipleOf5 = Math.Abs((int)Math.Floor(percent / 5) * 5 - 45) / 5; | ||
| score4 = Math.Min(prevMultipleOf5, nextMultipleOf5) * 10; | ||
| for (var x = 0; x < size - 10; x++) | ||
| { | ||
| for (var y = 0; y < size; y++) | ||
| { | ||
| if (MatchesFinderPattern(qrCode, x, y, vertical: true)) | ||
| score += 40; | ||
| } | ||
| } | ||
|
|
||
| return score1 + score2 + score3 + score4; | ||
| return score; |
There was a problem hiding this comment.
📝 Info: ScoreFinderPatterns splits single loop into two separate loops
The old code checked both horizontal and vertical finder-like patterns in a single nested loop (y: 0..size, x: 0..size-10). The new code at QRCoder.Core/Generators/QRCodeGenerator.cs:725-746 splits this into two separate double-loops: one for horizontal (y outer, x inner with x < size-10) and one for vertical (x outer with x < size-10, y inner with y < size). Both cover the same set of (x, y) pairs, so the total score is identical. The split improves readability but the reviewer should note that the iteration order change is intentional and correct — the score is a simple sum so order doesn't matter.
Was this helpful? React with 👍 or 👎 to provide feedback.
| AppendParameter(payload, "separeference", Uri.EscapeDataString(sepaReference)); | ||
| if (authority == AuthorityType.singledirectdebitsepa) | ||
| { | ||
| bezahlCodePayload += $"account={account}&"; | ||
| bezahlCodePayload += $"bnc={bnc}&"; | ||
| if (postingKey > 0) | ||
| bezahlCodePayload += $"postingkey={postingKey}&"; | ||
| AppendParameter(payload, "creditorid", Uri.EscapeDataString(creditorId)); | ||
| AppendParameter(payload, "mandateid", Uri.EscapeDataString(mandateId)); |
There was a problem hiding this comment.
📝 Info: Null guards removed before Uri.EscapeDataString calls in BezahlCode
The old code explicitly checked !string.IsNullOrEmpty(sepaReference), !string.IsNullOrEmpty(creditorId), and !string.IsNullOrEmpty(mandateId) before calling Uri.EscapeDataString(). The new code at QRCoder.Core/Generators/PayloadGenerator.cs:2040-2044 calls Uri.EscapeDataString() unconditionally and relies on AppendParameter to filter empty results. This is safe because: (1) these parameters default to "" in the constructor, (2) the SEPA branch validates them (e.g., sepaReference.Length would NPE on null), and (3) Uri.EscapeDataString("") returns "" which AppendParameter filters out. However, the implicit contract has changed — the old code was defensive against null, while the new code assumes non-null.
Was this helpful? React with 👍 or 👎 to provide feedback.
| private static string ReverseString(string inp) | ||
| { | ||
| var sb = new StringBuilder(inp.Length); | ||
| for (var i = inp.Length - 1; i >= 0; i--) | ||
| sb.Append(inp[i]); | ||
| return sb.ToString(); |
There was a problem hiding this comment.
🔍 Several methods moved outside ModulePlacer nested class with inconsistent indentation
The methods ReverseString (QRCoder.Core/Generators/QRCodeGenerator.cs:353-358), PlaceAlignmentPatterns (QRCoder.Core/Generators/QRCodeGenerator.cs:581-601), and IsBlocked (QRCoder.Core/Generators/QRCodeGenerator.cs:625-628) appear to have been moved from inside the ModulePlacer nested static class to the enclosing QRCodeGenerator class scope (based on their reduced indentation level). However, other methods like PlaceVersion, PlaceFormat, PlaceFinderPatterns, etc. remain inside ModulePlacer with their original indentation. This creates an inconsistent code organization where some module-placement helpers are inside ModulePlacer and others are outside it. The code still compiles and works correctly since nested classes can access private members of enclosing classes, but the inconsistency may cause confusion.
Was this helpful? React with 👍 or 👎 to provide feedback.
| public void Dispose() | ||
| { | ||
| if (this.disposed) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| this.disposed = true; | ||
| GC.SuppressFinalize(this); | ||
| } |
There was a problem hiding this comment.
📝 Info: Dispose pattern changed from idempotent guard to GC.SuppressFinalize without finalizer
The old Dispose() method used a disposed boolean field to guard against double-disposal. The new implementation at QRCoder.Core/Generators/QRCodeGenerator.cs:1550-1553 replaces this with a single GC.SuppressFinalize(this) call. Since QRCodeGenerator has no finalizer and no unmanaged resources, calling GC.SuppressFinalize is unnecessary (but harmless). The class implements IDisposable purely for backward compatibility as noted in the XML comment. The disposed field was also removed from the class. This is a minor style concern — GC.SuppressFinalize without a finalizer is a no-op that may confuse readers into thinking there's a finalizer somewhere.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
All Submissions:
New Feature Submissions:
Changes to Core Features: