Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temporary workaround for IfPartPattern in reviewer and other fixes #362

Closed
wants to merge 52 commits into from

Conversation

rkm
Copy link
Member

@rkm rkm commented Jul 25, 2023

closes #144
closes #312

@rkm rkm changed the title Tempoary workaround for IfPartPattern in reviewer Tempoary workaround for IfPartPattern in reviewer and other fixes Jul 25, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2023

Codecov Report

Attention: 232 lines in your changes are missing coverage. Please review.

Comparison is base (6ad3891) 41.46% compared to head (6db3433) 38.56%.
Report is 59 commits behind head on main.

Files Patch % Lines
...entifiable/Reporting/Reports/FailureStoreReport.cs 45.69% 75 Missing and 7 partials ⚠️
ii/MainWindow.cs 0.00% 41 Missing ⚠️
ii/Program.cs 0.00% 28 Missing ⚠️
IsIdentifiable/Rules/PartPatternFilterRule.cs 63.79% 15 Missing and 6 partials ⚠️
ii/Views/RulesView.cs 0.00% 19 Missing ⚠️
ii/Views/FailureView.cs 0.00% 16 Missing ⚠️
ii/Views/TreeNodeWithCount.cs 0.00% 10 Missing ⚠️
...le/Options/IsIdentifiableReportValidatorOptions.cs 0.00% 9 Missing ⚠️
IsIdentifiable/Redacting/ReportReader.cs 40.00% 3 Missing ⚠️
IsIdentifiable/Failures/FailurePart.cs 0.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #362      +/-   ##
==========================================
- Coverage   41.46%   38.56%   -2.91%     
==========================================
  Files          67       69       +2     
  Lines        3803     4071     +268     
  Branches        0      554     +554     
==========================================
- Hits         1577     1570       -7     
- Misses       2226     2395     +169     
- Partials        0      106     +106     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rkm rkm changed the title Tempoary workaround for IfPartPattern in reviewer and other fixes Temporary workaround for IfPartPattern in reviewer and other fixes Jul 25, 2023
@rkm rkm force-pushed the hotfix/ifpartpattern branch 2 times, most recently from 1bd3912 to 492b2d8 Compare August 8, 2023 11:02
Comment on lines +81 to +83
matchesBefore && string.IsNullOrWhiteSpace(WordAfter) ||
matchesAfter && string.IsNullOrWhiteSpace(WordBefore) ||
(matchesBefore && matchesAfter)

Check notice

Code scanning / CodeQL

Complex condition Note

Complex condition: too many logical operations in this expression.
private Regex? _wordAfterRegex;

private int _usedCount = 0;
private object _usedCountLock = new();

Check notice

Code scanning / CodeQL

Missed 'readonly' opportunity Note

Field '_usedCountLock' can be 'readonly'.

while (r.Read())
int totalProcessed = 0;
var localTokenSource = new CancellationTokenSource();

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable Warning

Disposable 'CancellationTokenSource' is created but not disposed.
ii/Program.cs Outdated
}

var report = new FailureStoreReport("", 0, fileSystem);
var failures = FailureStoreReport.Deserialize(fileSystem.FileInfo.New(opts.FailuresCsv), (_) => { }, new CancellationTokenSource().Token, partRules: null, runParallel: false).ToArray();

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable Warning

Disposable 'CancellationTokenSource' is created but not disposed.
return 1;
}

var report = new FailureStoreReport("", 0, fileSystem);

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
report
is useless, since its value is never read.
ii/Program.cs Fixed Show fixed Hide fixed
Comment on lines +276 to +286
foreach (var partRule in partRules)
{
if (!string.IsNullOrWhiteSpace(partRule.IfColumn) && !string.Equals(partRule.IfColumn, row.ProblemField, StringComparison.InvariantCultureIgnoreCase))
continue;

foreach (var part in parts.Where(x => partRule.Covers(x, row.ProblemValue)))
{
toRemove.Add(part);
partRule.IncrementUsed();
}
}

Check notice

Code scanning / CodeQL

Missed opportunity to use Where Note

This foreach loop
implicitly filters its target sequence
- consider filtering the sequence explicitly using '.Where(...)'.
}

var report = new FailureStoreReport("", 0, fileSystem);
var failures = FailureStoreReport.Deserialize(fileSystem.FileInfo.New(opts.FailuresCsv), (_) => { }, new CancellationTokenSource().Token, partRules: null, runParallel: false, opts.StopAtFirstError).ToArray();

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
failures
is useless, since its value is never read.
if (row.ProblemValue.Substring(part.Offset, part.Word.Length) == part.Word)
continue;
}
catch (ArgumentOutOfRangeException) { }

Check notice

Code scanning / CodeQL

Poor error handling: empty catch block Note

Poor error handling: empty catch block.
@rkm
Copy link
Member Author

rkm commented Dec 8, 2023

needs reworked and rebased - separate PR

@rkm rkm closed this Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants