feat: Add --lint support for validating definition files

[Copilot]

Adds a --lint command that reads the .reviewmark.yaml configuration file and reports all structural and semantic issues in a single pass, with non-zero exit code on failure. Lint output follows the standard [location]: [severity]: [issue] format used by common linting tools, making it easy to integrate with editors and CI pipelines.

Description

Context.cs

• Added Lint boolean property and --lint flag parsing (mirrors --validate pattern)

Program.cs

• Added --lint to help text
• Added Priority 4 lint block in Run() (between self-validation and main tool logic)
• Added RunLintLogic that calls ReviewMarkConfiguration.Lint and reports every returned error using the standard linting format; non-zero exit code on any error
• On success, outputs [definitionFile]: No issues found

ReviewMarkConfiguration.cs

• Refactored into a new file-local ReviewMarkConfigurationHelpers class containing IsSupportedEvidenceSourceType, DeserializeRaw, and BuildConfiguration methods, allowing file-local YAML types to appear in method signatures
• Load now embeds the filename in all exceptions:
  • YAML syntax errors: "Failed to parse 'filename' at line X, column Y: <message>"
  • Semantic validation errors: "Invalid configuration in 'filename': <message>"
  • File-read errors: "Failed to read configuration file 'filename': <message>"
• BuildConfiguration now validates the evidence-source type is url or fileshare, so Load produces a clear error for unsupported types rather than a later failure in ReviewIndex.Load
• Added Lint(filePath) static method that accumulates all detectable errors in one pass without throwing, returning them in the standard [location]: [severity]: [issue] format:
  • YAML syntax errors use file:line:col: error: <message> (includes precise position)
  • All other errors use file: error: <message>
  • Validates evidence-source block, each review set, and detects duplicate review set IDs

Validation.cs

• Added ReviewMark_Lint self-validation test (checks for No issues found in output)

TestDirectory.cs (new)

• Added TestDirectory — an IDisposable helper class for the test project that creates and auto-deletes a temporary directory, replacing the repetitive try/finally pattern across tests
• Dispose() wraps deletion in try/catch for IOException and UnauthorizedAccessException so transient file-lock failures on Windows do not mask real assertion failures

Tests

• ContextTests: Context_Create_LintFlag_SetsLintTrue, Context_Create_NoArguments_LintIsFalse
• ProgramTests: refactored elaborate and lint tests to use TestDirectory; all lint tests dispose Context before reading the log file to release the Windows file handle; updated all error tests to assert "error:" (lowercase, matching the new format) plus specific content (filename, error type); added:
  • Program_Run_WithLintFlag_CorruptedYaml_ReportsError
  • Program_Run_WithLintFlag_MissingEvidenceSource_ReportsError
  • Program_Run_WithLintFlag_MultipleErrors_ReportsAll
• ReviewMarkConfigurationTests: added three new tests covering filename-in-error, line-in-error, and all-errors-in-one-pass behaviour

requirements.yaml

• Added ReviewMark-Cmd-Lint requirement linked to all 13 test cases
• Added ReviewMark_Lint to all platform requirements (Windows/Linux/macOS, .NET 8/9/10)

README.md

• Added --lint to the Features list, Usage examples, and Command-Line Options table
• Added ReviewMark_Lint to the Self Validation test report example and descriptions (totals updated: 8 → 9)

docs/guide/guide.md

• Added a full ## Lint Definition File section covering purpose, running lint, what lint checks, and lint error message format with examples in the standard [location]: [severity]: [issue] style
• Added --lint to the Command-Line Options table
• Added ReviewMark_Lint to the validation report example and test descriptions (totals updated: 8 → 9)
• Added Example 4: Lint a Definition File (with basic, --definition, and silent/log variants); subsequent examples renumbered

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Code quality improvement

Pre-Submission Checklist

Before submitting this pull request, ensure you have completed the following:

Build and Test

• Code builds successfully: dotnet build --configuration Release
• All unit tests pass: dotnet test --configuration Release
• Self-validation tests pass:
  dotnet run --project src/DemaConsulting.ReviewMark --configuration Release --framework net10.0
--no-build -- --validate
• Code produces zero warnings

Code Quality

• Code formatting is correct: dotnet format --verify-no-changes
• New code has appropriate XML documentation comments
• Static analyzer warnings have been addressed

Quality Checks

Please run the following checks before submitting:

• Spell checker passes: cspell "**/*.{md,cs}"
• Markdown linter passes: markdownlint "**/*.md"
• YAML linter passes: yamllint .

Testing

• Added unit tests for new functionality
• Updated existing tests if behavior changed
• All tests follow the AAA (Arrange, Act, Assert) pattern
• Test coverage is maintained or improved

Documentation

• Updated README.md (if applicable)
• Updated ARCHITECTURE.md (if applicable)
• Added code examples for new features (if applicable)
• Updated requirements.yaml (if applicable)

Additional Notes

The lint check reports all detectable issues rather than stopping at the first error, so users can fix multiple problems in one pass. Output follows the standard [location]: [severity]: [issue] convention used by common linting tools:

• YAML syntax errors include the precise position: definition.yaml:3:5: error: (parse details)
• Semantic errors use file-level location: definition.yaml: error: Configuration is missing required 'evidence-source' block.
• Clean files report: definition.yaml: No issues found

The severity token is error for all current findings. The format is designed to accept warning alongside error if warnings are added in future. The shared IsSupportedEvidenceSourceType helper ensures the url/fileshare type check is consistent between BuildConfiguration (used by Load) and Lint.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[Feature]: Add --lint support</issue_title>
<issue_description>### Problem Statement

ReviewMark should have a lint option to lint the configuration file and report issues.

Proposed Solution

Add a --lint command which reads the configuration file and checks if it's valid. The result should be written using the Context.WriteLine or Context.WriteError so lint-failures also result in a non-zero exit code.

Alternatives Considered

No response

Usage Examples

Benefits

This provides the ability to check the configuration file is valid.

Priority

Medium - Would improve my workflow

Willingness to Contribute

Yes, I can submit a pull request

Additional Context

Lint errors should be clear about the cause and location so that the issues can be easily fixed.

Checklist

• I have searched existing issues to ensure this is not a duplicate
• I have provided a clear description of the feature
• I have explained why this feature would be useful</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes [Feature]: Add --lint support #26

---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

[Malcolmnixon]

@copilot Whups, you're right - reviewmark doesn't need includes - it's just in a single file.

[Copilot]

@copilot Whups, you're right - reviewmark doesn't need includes - it's just in a single file.

Removed in commit cf4b385 — the includes: field, all includes processing in Load, the 3 includes-related ReviewMarkConfigurationTests, the IncludedFileCorrupted program test, and the corresponding requirements.yaml entries. The improved error messages (filename + line number) are retained.

[Copilot]

Pull request overview

Adds a new --lint command to ReviewMark to validate .reviewmark.yaml definition files, improving configuration feedback quality (including filename and YAML line/column reporting) and extending self-validation/test coverage around the new behavior.

Changes:

• Added --lint flag parsing (Context) and a new lint execution path in Program.Run().
• Refactored ReviewMarkConfiguration.Load/Parse to centralize YAML deserialization/validation and improve exception messages with filename (+ line/column for YAML syntax errors).
• Added/updated unit tests and self-validation coverage; introduced a TestDirectory helper to reduce temp-dir boilerplate; updated requirements.yaml accordingly.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/DemaConsulting.ReviewMark/Context.cs	Adds Lint flag and --lint argument parsing.
src/DemaConsulting.ReviewMark/Program.cs	Adds --lint help text and lint execution path (RunLintLogic).
src/DemaConsulting.ReviewMark/ReviewMarkConfiguration.cs	Refactors YAML parsing/validation helpers; improves Load exception messages with filename/line/column.
src/DemaConsulting.ReviewMark/Validation.cs	Adds ReviewMark_Lint self-validation test.
test/DemaConsulting.ReviewMark.Tests/TestDirectory.cs	New disposable temp-directory helper for tests.
test/DemaConsulting.ReviewMark.Tests/ContextTests.cs	Adds tests validating --lint flag parsing behavior.
test/DemaConsulting.ReviewMark.Tests/ProgramTests.cs	Adds lint-focused tests and refactors temp-dir usage to TestDirectory.
test/DemaConsulting.ReviewMark.Tests/ReviewMarkConfigurationTests.cs	Adds tests asserting improved Load() error message quality.
requirements.yaml	Adds a --lint requirement and links it to relevant tests across platforms/runtimes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Malcolmnixon]

@copilot apply changes based on the comments in this thread

Also the test is getting errors on windows - I don't think a file is being closed before being read:

  Failed Program_Run_WithLintFlag_ValidConfig_ReportsSuccess [18 ms]
  Error Message:
   Test method DemaConsulting.ReviewMark.Tests.ProgramTests.Program_Run_WithLintFlag_ValidConfig_ReportsSuccess threw exception:
System.IO.IOException: The process cannot access the file 'C:\Users\runneradmin\AppData\Local\Temp\reviewmark_test_831d1631-e51f-4b8f-9c34-ad2560832226\lint.log' because it is being used by another process.

[Malcolmnixon]

@copilot And remember the README.md and User Guide should be expanded to include the lint option, usage examples, and possibly needs updates to the self-validation for the new tests.

[Copilot]

@copilot apply changes based on the comments in this thread

Also the test is getting errors on windows - I don't...

Applied in commit 9db09b0:

Windows file lock fix: All lint tests that used using var context = ... were reading the log file while Context still held the StreamWriter file handle. Changed to using (var context = ...) { ... } block style so the context is disposed (and the handle released) before File.ReadAllText is called.

Consistent error messages: Both missing-filePath lint errors fixed — unknown evidence-source type and duplicate review ID now include "Invalid configuration in '{filePath}':" prefix.

BuildConfiguration type validation: Added url/fileshare check so ReviewMarkConfiguration.Load also gives a clear error for unsupported types (previously the failure would surface later in ReviewIndex.Load without the config filename). The type check is extracted into ReviewMarkConfigurationHelpers.IsSupportedEvidenceSourceType so both BuildConfiguration and Lint share a single implementation.

TestDirectory.Dispose safety: Wrapped Directory.Delete in try/catch for IOException and UnauthorizedAccessException so transient cleanup failures don't mask real assertion failures.

[Copilot]

@copilot And remember the README.md and User Guide should be expanded to include the lint option, usage examples, and possibly needs updates to the self-validation for the new tests.

Done in commit 7147118:

README.md: added --lint to the Features list, Usage examples (reviewmark --lint, reviewmark --lint --definition path/to/definition.yaml), the Command-Line Options table, and the Self Validation section (ReviewMark_Lint now appears in the example report and descriptions; totals updated to 9).

docs/guide/guide.md: added a full ## Lint Definition File section covering purpose, Running Lint, What Lint Checks, and Lint Error Messages with example output; added --lint to the Command-Line Options table; updated the validation report example and test descriptions with ReviewMark_Lint; added Example 4: Lint a Definition File (with basic, --definition, and silent/log variants) and renumbered the subsequent examples.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/1108766/chrome-linux.zip
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node install.js (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Malcolmnixon]

@copilot Linting failed - English vs American spelling.

[Copilot]

@copilot Linting failed - English vs American spelling.

Fixed in commit 3cce5e8 — changed recognised to recognized in the XML doc comment for IsSupportedEvidenceSourceType. Spell check now reports 0 issues.

[Malcolmnixon]

I think the linting output should follow the standard for common linting reports. Something like "[location]: [issue]" and if there are no issues then "[definitionFile]: No issues found".

[Copilot]

I think the linting output should follow the standard for common linting reports. Something like "[location]: [issue]" and if there are no issues then "[definitionFile]: No issues found".

Done in commit 0a40cb6. Lint output now follows [location]: [severity]: [issue] format:

• YAML syntax errors include the line and column: definition.yaml:3:5: error: (parse details)
• Semantic errors use file-only location: definition.yaml: error: Configuration is missing required 'evidence-source' block.
• Success: definition.yaml: No issues found

The severity token is error for all current findings. The format is designed to accept warning alongside error if/when warnings are added later. All tests and the self-validation test updated accordingly.