Skip to content

Fix SonarCloud and CodeQL code-quality issues - #73

Merged
Malcolmnixon merged 1 commit into
mainfrom
fix/code-quality-sonarcloud-codeql-issues
Aug 31, 2026
Merged

Fix SonarCloud and CodeQL code-quality issues#73
Malcolmnixon merged 1 commit into
mainfrom
fix/code-quality-sonarcloud-codeql-issues

Conversation

@Malcolmnixon

Copy link
Copy Markdown
Member

This pull request refactors how file-type assertion units are managed in FileAssertFile by grouping them into a single struct, simplifying the constructor and related code. Additionally, it updates various tests to use more concise and expressive assertions, and makes minor improvements to resource handling and type safety.

Refactoring and code simplification:

  • The FileAssertFile constructor now takes a single FileTypeAsserts struct containing all optional file-type assert units (text, pdf, xml, html, yaml, json, zip) instead of passing each one as a separate parameter. This change is reflected in the constructor, its documentation, and the static Create method. (src/DemaConsulting.FileAssert/Modeling/FileAssertFile.cs) [1] [2] [3]

Test improvements:

  • Test assertions throughout the codebase are updated to use Assert.Single and variable assignment for clarity and to avoid duplicate index-based access. (test/DemaConsulting.FileAssert.Tests/Cli/ContextNewPropertiesTests.cs, test/DemaConsulting.FileAssert.Tests/Configuration/ConfigurationTests.cs, test/DemaConsulting.FileAssert.Tests/Configuration/FileAssertConfigTests.cs, test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertHtmlAssertTests.cs, test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertJsonAssertTests.cs, test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertRuleTests.cs, test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertYamlAssertTests.cs, test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertZipAssertTests.cs) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]

Resource management and type safety:

  • The PDF test now uses a using statement for PdfDocumentBuilder to ensure proper disposal. (src/DemaConsulting.FileAssert/SelfTest/ValidationPdf.cs)
  • The BuildPageText method signature is updated to accept a List<Page> instead of IReadOnlyList<Page>, matching usage. (src/DemaConsulting.FileAssert/Modeling/FileAssertPdfAssert.cs)
  • In RunDocumentAssertions, a null-forgiving operator is used for _pages after a guard, improving null-safety. (src/DemaConsulting.FileAssert/Modeling/FileAssertPdfAssert.cs)

Minor improvements:

  • Adds a missing using System.Linq; directive for LINQ usage in zip file test helpers. (test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertZipAssertTests.cs)
  • Refactors the zip file test helper to avoid a double-nested loop and ensure proper resource disposal. (test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertZipAssertTests.cs)

- Dispose PdfDocumentBuilder in ValidationPdf self-test (cs/local-not-disposed)
- Replace foreach+re-open-entry loop with LINQ Select in
  FileAssertZipAssertTests to satisfy cs/linq/missed-select
- Remove unreachable null-check on _pages in FileAssertPdfAssert (S2589)
- Narrow BuildPageText parameter to List<Page> for CA1859
- Consolidate FileAssertFile's 13-parameter constructor into a
  FileTypeAsserts record struct to satisfy S107
- Use the return value of Assert.Single/Assert.IsType instead of
  re-deriving it via indexing/casting across 8 test files (xUnit2033)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 31, 2026 00:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request addresses code-quality findings by refactoring the core file-assert modeling API to reduce constructor parameter sprawl, tightening resource handling in PDF self-tests, and simplifying test assertions for clarity and maintainability.

Changes:

  • Refactored FileAssertFile to accept a single FileTypeAsserts value grouping all optional file-type assert units.
  • Improved PDF-related resource handling and adjusted internal PDF helper typing for consistency.
  • Updated multiple tests to use Assert.Single(...) and local variables to avoid repeated index-based access.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/DemaConsulting.FileAssert/Modeling/FileAssertFile.cs Groups optional file-type asserts into a single struct passed to the constructor and Create method.
src/DemaConsulting.FileAssert/Modeling/FileAssertPdfAssert.cs Improves null-safety around _pages usage and updates BuildPageText parameter type to match call sites.
src/DemaConsulting.FileAssert/SelfTest/ValidationPdf.cs Ensures PdfDocumentBuilder is disposed via using var.
test/DemaConsulting.FileAssert.Tests/Cli/ContextNewPropertiesTests.cs Replaces index-based assertions with Assert.Single and a local variable.
test/DemaConsulting.FileAssert.Tests/Configuration/ConfigurationTests.cs Simplifies hierarchy assertions using Assert.Single for collections.
test/DemaConsulting.FileAssert.Tests/Configuration/FileAssertConfigTests.cs Uses Assert.Single and locals to make parsed-config expectations clearer.
test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertHtmlAssertTests.cs Uses Assert.Single to validate a single error message more directly.
test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertJsonAssertTests.cs Uses Assert.Single for error collection assertions in parse/IO failure tests.
test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertRuleTests.cs Avoids repeated casts by capturing the typed rule via Assert.IsType<T>.
test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertYamlAssertTests.cs Uses Assert.Single and a local variable for parse-failure error verification.
test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertZipAssertTests.cs Adds missing LINQ using; refactors zip helper disposal and simplifies single-item assertions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Malcolmnixon
Malcolmnixon merged commit 601e720 into main Aug 31, 2026
16 checks passed
@Malcolmnixon
Malcolmnixon deleted the fix/code-quality-sonarcloud-codeql-issues branch August 31, 2026 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants