From 35b251b76920dc421d280b7d782c744fc6c077a5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 13:57:26 +0000 Subject: [PATCH 1/6] Initial plan From 73410629d549b900a32cb56cb0ff55afc277b059 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 14:19:12 +0000 Subject: [PATCH 2/6] Merge Linting subsystem into Configuration subsystem - Create LintIssue.cs with LintSeverity enum and LintIssue record - Add VersionMarkConfig.Load() method that performs all validation in a single pass, returning nullable config and list of LintIssue - Update VersionMarkConfig.ReadFromFile() to delegate to Load() for backward compatibility - Remove Linting/Lint.cs and the Linting subsystem - Update Program.RunLint() and RunCapture() to use Load() - Update test files: LintTests.cs -> VersionMarkConfigLoadTests, LintingSubsystemTests.cs -> ConfigurationLoadSubsystemTests - Add docs/reqstream/configuration/load.yaml requirements - Remove docs/reqstream/linting/ requirements files - Update requirements.yaml and docs/design/introduction.md Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- docs/design/introduction.md | 17 +- docs/reqstream/configuration/load.yaml | 145 +++++++ docs/reqstream/linting/lint.yaml | 33 -- docs/reqstream/linting/linting.yaml | 117 ------ requirements.yaml | 3 +- .../Configuration/LintIssue.cs | 62 +++ .../Configuration/VersionMarkConfig.cs | 332 +++++++++++++-- .../Linting/Lint.cs | 346 ---------------- src/DemaConsulting.VersionMark/Program.cs | 48 ++- .../Linting/LintTests.cs | 381 ++++++++---------- .../Linting/LintingSubsystemTests.cs | 159 ++++---- 11 files changed, 796 insertions(+), 847 deletions(-) create mode 100644 docs/reqstream/configuration/load.yaml delete mode 100644 docs/reqstream/linting/lint.yaml delete mode 100644 docs/reqstream/linting/linting.yaml create mode 100644 src/DemaConsulting.VersionMark/Configuration/LintIssue.cs delete mode 100644 src/DemaConsulting.VersionMark/Linting/Lint.cs diff --git a/docs/design/introduction.md b/docs/design/introduction.md index 634f1bc..a6d53a9 100644 --- a/docs/design/introduction.md +++ b/docs/design/introduction.md @@ -15,18 +15,16 @@ The purpose of this document is to: ## Scope -This document covers the design of six subsystems within VersionMark: +This document covers the design of five subsystems within VersionMark: - The **Cli Subsystem**: the `Program` entry point and `Context` class that handle argument parsing, output routing, and program flow control - The **Configuration Subsystem**: the `VersionMarkConfig` and `ToolConfig` classes that - read and interpret `.versionmark.yaml` configuration files + read, validate, and interpret `.versionmark.yaml` configuration files - The **Capture Subsystem**: the `VersionInfo` record that serializes and deserializes captured version data to and from JSON - The **Publishing Subsystem**: the `MarkdownFormatter` class that generates the markdown version report from captured data -- The **Linting Subsystem**: the `Lint` class that validates `.versionmark.yaml` configuration - files and reports issues with precise source locations - The **SelfTest Subsystem**: the `Validation` class and `PathHelpers` utility that together provide built-in verification of the tool's core functionality @@ -44,15 +42,13 @@ VersionMark (System) Version capture/publish tool ├── Cli (Subsystem) Argument parsing and dispatch │ ├── Program (Unit) Tool entry point │ └── Context (Unit) Command-line state container -├── Configuration (Subsystem) YAML configuration loading -│ ├── VersionMarkConfig (Unit) Top-level config container +├── Configuration (Subsystem) YAML configuration loading and validation +│ ├── VersionMarkConfig (Unit) Top-level config container and validator │ └── ToolConfig (Unit) Per-tool config record ├── Capture (Subsystem) Tool version capture │ └── VersionInfo (Unit) JSON version data record ├── Publishing (Subsystem) Markdown report publishing │ └── MarkdownFormatter (Unit) Version report formatter -├── Linting (Subsystem) Configuration file lint -│ └── Lint (Unit) YAML configuration validator └── SelfTest (Subsystem) Built-in self-validation ├── Validation (Unit) Self-validation runner └── PathHelpers (Unit) Safe path combination @@ -71,13 +67,12 @@ src/DemaConsulting.VersionMark/ ├── Cli/ │ └── Context.cs — command-line argument parser and I/O owner ├── Configuration/ -│ └── VersionMarkConfig.cs — YAML configuration and tool definitions +│ ├── LintIssue.cs — lint issue record and severity enum +│ └── VersionMarkConfig.cs — YAML configuration, tool definitions, and validation ├── Capture/ │ └── VersionInfo.cs — captured version data record ├── Publishing/ │ └── MarkdownFormatter.cs — markdown report generation -├── Linting/ -│ └── Lint.cs — YAML configuration linter └── SelfTest/ ├── Validation.cs — self-validation test runner └── PathHelpers.cs — safe path utilities diff --git a/docs/reqstream/configuration/load.yaml b/docs/reqstream/configuration/load.yaml new file mode 100644 index 0000000..3dedc97 --- /dev/null +++ b/docs/reqstream/configuration/load.yaml @@ -0,0 +1,145 @@ +--- +sections: + - title: VersionMark Load Requirements + requirements: + - id: VersionMark-Load-FileExistence + title: The tool shall report an error when the configuration file does not exist. + justification: | + Users may specify an incorrect path; an immediate, clear error message + avoids silent failures and directs the user to fix the file path. + tags: + - configuration + tests: + - ConfigurationLoad_NonExistentFile_Fails + + - id: VersionMark-Load-YamlParsing + title: The tool shall report an error when the configuration file contains invalid YAML. + justification: | + Malformed YAML cannot be interpreted. Reporting a parse error with its + location lets users quickly find and fix the syntax issue. + tags: + - configuration + tests: + - ConfigurationLoad_InvalidYaml_Fails + + - id: VersionMark-Load-ToolsSection + title: The tool shall report an error when the configuration file is missing a non-empty 'tools' section. + justification: | + A configuration without any tool definitions cannot perform any useful + capture or publish operations. + tags: + - configuration + tests: + - ConfigurationLoad_MultipleErrors_ReportsAllErrorsInSinglePass + + - id: VersionMark-Load-ToolCommand + title: The tool shall report an error when a tool is missing a non-empty 'command' field. + justification: | + The 'command' field defines how to invoke the tool to obtain version + information. Without it the tool cannot function at runtime. + tags: + - configuration + tests: + - ConfigurationLoad_MultipleErrors_ReportsAllErrorsInSinglePass + + - id: VersionMark-Load-ToolRegex + title: The tool shall report an error when a tool is missing a non-empty 'regex' field. + justification: | + The 'regex' field is used to extract the version string from command output. + Without it the tool cannot parse a version at runtime. + tags: + - configuration + tests: + - ConfigurationLoad_MultipleErrors_ReportsAllErrorsInSinglePass + + - id: VersionMark-Load-RegexValid + title: The tool shall report an error when a regex value cannot be compiled. + justification: | + An invalid regex pattern will cause a runtime failure. Detecting it early + during load gives the user a clear error with a precise file location. + tags: + - configuration + tests: + - ConfigurationLoad_InvalidRegex_ReportsError + + - id: VersionMark-Load-RegexVersion + title: The tool shall report an error when a regex does not contain a named 'version' capture group. + justification: | + The capture group named 'version' is the mechanism by which the tool + extracts the version string. Its absence causes silent data loss at runtime. + tags: + - configuration + tests: + - ConfigurationLoad_RegexWithoutVersionGroup_ReportsError + + - id: VersionMark-Load-OsOverrides + title: The tool shall report an error when an OS-specific command or regex override is empty. + justification: | + An empty OS-specific override will be used at runtime on the corresponding + platform, causing a capture failure. Detecting it during load prevents + platform-specific surprises. + tags: + - configuration + tests: + - ConfigurationLoad_EmptyOsSpecificOverride_ReportsError + + - id: VersionMark-Load-UnknownKeys + title: The tool shall report unknown keys as non-fatal warnings that do not fail loading. + justification: | + Unknown keys may be typos or forward-compatible extensions. Treating them + as warnings alerts users without blocking valid configurations from + proceeding. + tags: + - configuration + tests: + - ConfigurationLoad_UnknownKey_IsWarningNotError + + - id: VersionMark-Load-ErrorLocation + title: The tool shall report all load findings with the filename and line/column location. + justification: | + Precise locations (file, line, column) allow users to jump directly to the + problem in their editor, dramatically reducing the time needed to fix issues. + tags: + - configuration + tests: + - ConfigurationLoad_Error_IncludesFileAndLineInfo + + - id: VersionMark-Load-AllIssues + title: The tool shall report all load issues in a single pass without stopping at the first error. + justification: | + Reporting all issues at once lets users fix everything in one edit cycle + rather than repeatedly re-running to discover each subsequent problem. + tags: + - configuration + tests: + - ConfigurationLoad_MultipleErrors_ReportsAllErrorsInSinglePass + + - id: VersionMark-Load-Method + title: >- + The VersionMarkConfig.Load method shall validate a .versionmark.yaml configuration file + and return issues with file, line, and column location alongside the loaded configuration. + justification: | + Centralizing all load and validation checks in a single Load method ensures a consistent + validation pass and allows all issues to be reported in one run rather than + stopping at the first error. Returning both the config and issues allows callers + to proceed when only warnings are present. + tests: + - VersionMarkConfig_Load_ValidConfig_ReturnsConfig + - VersionMarkConfig_Load_MissingFile_ReturnsNullConfig + - VersionMarkConfig_Load_InvalidYaml_ReturnsNullConfig + - VersionMarkConfig_Load_MissingToolsSection_ReturnsNullConfig + - VersionMarkConfig_Load_EmptyToolsSection_ReturnsNullConfig + - VersionMarkConfig_Load_MissingCommand_ReturnsNullConfig + - VersionMarkConfig_Load_EmptyCommand_ReturnsNullConfig + - VersionMarkConfig_Load_MissingRegex_ReturnsNullConfig + - VersionMarkConfig_Load_EmptyRegex_ReturnsNullConfig + - VersionMarkConfig_Load_InvalidRegex_ReturnsNullConfig + - VersionMarkConfig_Load_RegexMissingVersionGroup_ReturnsNullConfig + - VersionMarkConfig_Load_UnknownTopLevelKey_ReturnsConfig + - VersionMarkConfig_Load_UnknownToolKey_ReturnsConfig + - VersionMarkConfig_Load_OsSpecificEmptyCommand_ReturnsNullConfig + - VersionMarkConfig_Load_OsSpecificEmptyRegex_ReturnsNullConfig + - VersionMarkConfig_Load_OsSpecificRegexMissingVersionGroup_ReturnsNullConfig + - VersionMarkConfig_Load_OsSpecificInvalidRegex_ReturnsNullConfig + - VersionMarkConfig_Load_MultipleErrors_ReportsAll + - VersionMarkConfig_Load_IssuesContainFilePath diff --git a/docs/reqstream/linting/lint.yaml b/docs/reqstream/linting/lint.yaml deleted file mode 100644 index f29b0ef..0000000 --- a/docs/reqstream/linting/lint.yaml +++ /dev/null @@ -1,33 +0,0 @@ ---- -sections: - - title: Lint Unit Requirements - requirements: - - id: VersionMark-Lint-Run - title: >- - The Lint.Run method shall validate a .versionmark.yaml configuration file - and report all issues with file, line, and column location. - justification: | - Centralizing all lint checks in a single Run method ensures a consistent - validation pass and allows all issues to be reported in one run rather than - stopping at the first error. Precise locations allow users to jump directly - to the problem in their editor. - tests: - - Lint_Run_ValidConfig_ReturnsTrue - - Lint_Run_MissingFile_ReturnsFalse - - Lint_Run_InvalidYaml_ReturnsFalse - - Lint_Run_MissingToolsSection_ReturnsFalse - - Lint_Run_EmptyToolsSection_ReturnsFalse - - Lint_Run_MissingCommand_ReturnsFalse - - Lint_Run_EmptyCommand_ReturnsFalse - - Lint_Run_MissingRegex_ReturnsFalse - - Lint_Run_EmptyRegex_ReturnsFalse - - Lint_Run_InvalidRegex_ReturnsFalse - - Lint_Run_RegexMissingVersionGroup_ReturnsFalse - - Lint_Run_UnknownTopLevelKey_ReturnsTrue - - Lint_Run_UnknownToolKey_ReturnsTrue - - Lint_Run_OsSpecificEmptyCommand_ReturnsFalse - - Lint_Run_OsSpecificEmptyRegex_ReturnsFalse - - Lint_Run_OsSpecificRegexMissingVersionGroup_ReturnsFalse - - Lint_Run_OsSpecificInvalidRegex_ReturnsFalse - - Lint_Run_MultipleErrors_ReportsAll - - Lint_Run_ErrorMessageContainsFileName diff --git a/docs/reqstream/linting/linting.yaml b/docs/reqstream/linting/linting.yaml deleted file mode 100644 index 61e18b6..0000000 --- a/docs/reqstream/linting/linting.yaml +++ /dev/null @@ -1,117 +0,0 @@ ---- -sections: - - title: VersionMark Requirements - sections: - - title: Lint - requirements: - - id: VersionMark-Lint-FileExistence - title: The tool shall report an error when the configuration file does not exist. - justification: | - Users may specify an incorrect path; an immediate, clear error message - avoids silent failures and directs the user to fix the file path. - tags: - - lint - tests: - - LintingSubsystem_Lint_NonExistentFile_Fails - - - id: VersionMark-Lint-YamlParsing - title: The tool shall report an error when the configuration file contains invalid YAML. - justification: | - Malformed YAML cannot be interpreted. Reporting a parse error with its - location lets users quickly find and fix the syntax issue. - tags: - - lint - tests: - - LintingSubsystem_Lint_InvalidYaml_Fails - - - id: VersionMark-Lint-ToolsSection - title: The tool shall report an error when the configuration file is missing a non-empty 'tools' section. - justification: | - A configuration without any tool definitions cannot perform any useful - capture or publish operations. - tags: - - lint - tests: - - LintingSubsystem_Lint_MultipleErrors_ReportsAllErrorsInSinglePass - - - id: VersionMark-Lint-ToolCommand - title: The tool shall report an error when a tool is missing a non-empty 'command' field. - justification: | - The 'command' field defines how to invoke the tool to obtain version - information. Without it the tool cannot function at runtime. - tags: - - lint - tests: - - LintingSubsystem_Lint_MultipleErrors_ReportsAllErrorsInSinglePass - - - id: VersionMark-Lint-ToolRegex - title: The tool shall report an error when a tool is missing a non-empty 'regex' field. - justification: | - The 'regex' field is used to extract the version string from command output. - Without it the tool cannot parse a version at runtime. - tags: - - lint - tests: - - LintingSubsystem_Lint_MultipleErrors_ReportsAllErrorsInSinglePass - - - id: VersionMark-Lint-RegexValid - title: The tool shall report an error when a regex value cannot be compiled. - justification: | - An invalid regex pattern will cause a runtime failure. Detecting it early - during lint gives the user a clear error with a precise file location. - tags: - - lint - tests: - - LintingSubsystem_Lint_InvalidRegex_ReportsError - - - id: VersionMark-Lint-RegexVersion - title: The tool shall report an error when a regex does not contain a named 'version' capture group. - justification: | - The capture group named 'version' is the mechanism by which the tool - extracts the version string. Its absence causes silent data loss at runtime. - tags: - - lint - tests: - - LintingSubsystem_Lint_RegexWithoutVersionGroup_ReportsError - - - id: VersionMark-Lint-OsOverrides - title: The tool shall report an error when an OS-specific command or regex override is empty. - justification: | - An empty OS-specific override will be used at runtime on the corresponding - platform, causing a capture failure. Detecting it during lint prevents - platform-specific surprises. - tags: - - lint - tests: - - LintingSubsystem_Lint_EmptyOsSpecificOverride_ReportsError - - - id: VersionMark-Lint-UnknownKeys - title: The tool shall report unknown keys as non-fatal warnings that do not fail lint. - justification: | - Unknown keys may be typos or forward-compatible extensions. Treating them - as warnings alerts users without blocking valid configurations from - proceeding. - tags: - - lint - tests: - - LintingSubsystem_Lint_UnknownKey_IsWarningNotError - - - id: VersionMark-Lint-ErrorLocation - title: The tool shall report all lint findings with the filename and line/column location. - justification: | - Precise locations (file, line, column) allow users to jump directly to the - problem in their editor, dramatically reducing the time needed to fix issues. - tags: - - lint - tests: - - LintingSubsystem_Lint_Error_IncludesFileAndLineInfo - - - id: VersionMark-Lint-AllIssues - title: The tool shall report all lint issues in a single pass without stopping at the first error. - justification: | - Reporting all issues at once lets users fix everything in one edit cycle - rather than repeatedly re-running lint to discover each subsequent problem. - tags: - - lint - tests: - - LintingSubsystem_Lint_MultipleErrors_ReportsAllErrorsInSinglePass diff --git a/requirements.yaml b/requirements.yaml index 16403f6..ce927ce 100644 --- a/requirements.yaml +++ b/requirements.yaml @@ -17,8 +17,7 @@ includes: - docs/reqstream/configuration/configuration.yaml - docs/reqstream/configuration/version-mark-config.yaml - docs/reqstream/configuration/tool-config.yaml - - docs/reqstream/linting/linting.yaml - - docs/reqstream/linting/lint.yaml + - docs/reqstream/configuration/load.yaml - docs/reqstream/self-test/self-test.yaml - docs/reqstream/self-test/validation.yaml - docs/reqstream/self-test/path-helpers.yaml diff --git a/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs b/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs new file mode 100644 index 0000000..b1726d5 --- /dev/null +++ b/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs @@ -0,0 +1,62 @@ +// Copyright (c) 2025 DEMA Consulting +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +namespace DemaConsulting.VersionMark.Configuration; + +/// +/// Severity level for a lint issue. +/// +internal enum LintSeverity +{ + /// + /// Non-fatal advisory message. + /// + Warning, + + /// + /// Fatal validation failure that prevents loading. + /// + Error +} + +/// +/// Represents a single validation issue found while loading a configuration file. +/// +/// Path to the file that contains the issue. +/// One-based line number of the issue. +/// One-based column number of the issue. +/// Severity of the issue. +/// Human-readable description of the issue. +internal sealed record LintIssue( + string FilePath, + long Line, + long Column, + LintSeverity Severity, + string Description) +{ + /// + /// Returns a formatted string representation of the issue suitable for display to the user. + /// + /// + /// A string in the format "{FilePath}({Line},{Column}): {severity}: {Description}". + /// + public override string ToString() => + $"{FilePath}({Line},{Column}): {Severity.ToString().ToLowerInvariant()}: {Description}"; +} diff --git a/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs b/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs index 3a7e3f7..74c7787 100644 --- a/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs +++ b/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs @@ -23,6 +23,7 @@ using System.Text.RegularExpressions; using DemaConsulting.VersionMark.Capture; using YamlDotNet.RepresentationModel; +using YamlDotNet.Core; namespace DemaConsulting.VersionMark.Configuration; @@ -196,6 +197,26 @@ public sealed record VersionMarkConfig /// public required Dictionary Tools { get; init; } + /// + /// Known valid keys for tool configuration entries. + /// + private static readonly HashSet ValidToolKeys = + [ + "command", + "command-win", + "command-linux", + "command-macos", + "regex", + "regex-win", + "regex-linux", + "regex-macos" + ]; + + /// + /// Timeout for regex compilation and matching. + /// + private static readonly TimeSpan RegexTimeout = TimeSpan.FromSeconds(1); + /// /// Parameterless constructor required for object initializer. /// @@ -215,87 +236,324 @@ internal VersionMarkConfig(Dictionary tools) } /// - /// Reads configuration from a .versionmark.yaml file. + /// Loads configuration from a .versionmark.yaml file, performing all lint validation + /// in a single pass and returning both the configuration and any issues found. /// /// Path to the YAML configuration file. - /// Parsed configuration. - /// Thrown when the file cannot be read or parsed. - public static VersionMarkConfig ReadFromFile(string filePath) + /// + /// A tuple containing the loaded (or + /// when fatal errors prevent loading) and a read-only list of objects + /// describing all warnings and errors encountered. + /// + internal static (VersionMarkConfig? Config, IReadOnlyList Issues) Load(string filePath) { - // Check if file exists + var issues = new List(); + + // Check if file exists before attempting to parse if (!File.Exists(filePath)) { - throw new ArgumentException($"Configuration file not found: {filePath}"); + issues.Add(new LintIssue(filePath, 1, 1, LintSeverity.Error, $"Configuration file not found: {filePath}")); + return (null, issues); } + // Parse YAML, reporting any syntax errors with their source location + YamlStream yaml; try { - // Read the YAML file and parse as document using var reader = new StreamReader(filePath, System.Text.Encoding.UTF8); - var yaml = new YamlStream(); + yaml = new YamlStream(); yaml.Load(reader); + } + catch (YamlException ex) + { + issues.Add(new LintIssue(filePath, ex.Start.Line + 1, ex.Start.Column + 1, LintSeverity.Error, $"Failed to parse YAML file '{filePath}': {ex.Message}")); + return (null, issues); + } + + // Validate that the file contains at least one YAML document + if (yaml.Documents.Count == 0) + { + issues.Add(new LintIssue(filePath, 1, 1, LintSeverity.Error, "YAML file contains no documents")); + return (null, issues); + } + + // Validate that the root node is a YAML mapping + if (yaml.Documents[0].RootNode is not YamlMappingNode rootNode) + { + var node = yaml.Documents[0].RootNode; + issues.Add(CreateIssue(filePath, node, LintSeverity.Error, "Root node must be a YAML mapping")); + return (null, issues); + } + + // Warn about unknown top-level keys; they are non-fatal + foreach (var keyNode in rootNode.Children.Keys.OfType().Where(k => k.Value != "tools")) + { + issues.Add(CreateIssue(filePath, keyNode, LintSeverity.Warning, $"Unknown top-level key '{keyNode.Value}'")); + } - // Validate document exists - if (yaml.Documents.Count == 0) + // Ensure a 'tools' section is present + if (!rootNode.Children.TryGetValue(new YamlScalarNode("tools"), out var toolsNode)) + { + issues.Add(CreateIssue(filePath, rootNode, LintSeverity.Error, "Configuration must contain a 'tools' section")); + return (null, issues); + } + + // Validate that 'tools' is a YAML mapping + if (toolsNode is not YamlMappingNode toolsMapping) + { + issues.Add(CreateIssue(filePath, toolsNode, LintSeverity.Error, "The 'tools' section must be a mapping")); + return (null, issues); + } + + // Require at least one tool to be defined + if (toolsMapping.Children.Count == 0) + { + issues.Add(CreateIssue(filePath, toolsMapping, LintSeverity.Error, "Configuration must contain at least one tool")); + return (null, issues); + } + + // Validate each tool entry and collect all issues before deciding whether to build the config + var tools = new Dictionary(); + foreach (var toolEntry in toolsMapping.Children) + { + // Tool names must be scalar values + if (toolEntry.Key is not YamlScalarNode toolKeyNode) + { + issues.Add(CreateIssue(filePath, toolEntry.Key, LintSeverity.Error, "Tool names must be scalar values")); + continue; + } + + var toolName = toolKeyNode.Value ?? string.Empty; + + // Tool configuration must be a mapping + if (toolEntry.Value is not YamlMappingNode toolNode) { - throw new ArgumentException("YAML file contains no documents"); + issues.Add(CreateIssue(filePath, toolEntry.Value, LintSeverity.Error, $"Tool '{toolName}' configuration must be a mapping")); + continue; } - // Get root document and validate it's a mapping - if (yaml.Documents[0].RootNode is not YamlMappingNode rootNode) + // Validate all fields within the tool and accumulate issues + ValidateTool(filePath, toolName, toolNode, issues, out var toolConfig); + if (toolConfig != null) { - throw new ArgumentException("YAML root node must be a mapping"); + tools[toolName] = toolConfig; } + } + + // Return null config if any errors were found, so callers can distinguish warnings-only from failures + if (issues.Any(i => i.Severity == LintSeverity.Error)) + { + return (null, issues); + } + + // Build and return the successfully validated configuration + return (new VersionMarkConfig(tools), issues); + } + + /// + /// Reads configuration from a .versionmark.yaml file. + /// + /// Path to the YAML configuration file. + /// Parsed configuration. + /// Thrown when the file cannot be read or parsed. + public static VersionMarkConfig ReadFromFile(string filePath) + { + // Delegate to Load and convert the first error to an ArgumentException for backward compatibility + var (config, issues) = Load(filePath); + var firstError = issues.FirstOrDefault(i => i.Severity == LintSeverity.Error); + if (firstError != null) + { + throw new ArgumentException(firstError.Description); + } + + return config!; + } + + /// + /// Validates a single tool's configuration node, adding issues to the shared list + /// and producing a when all required fields are valid. + /// + /// Path to the configuration file (for issue location). + /// Name of the tool being validated. + /// YAML mapping node containing the tool's fields. + /// Shared list to which all discovered issues are appended. + /// + /// Set to a when validation succeeds; otherwise . + /// + private static void ValidateTool( + string filePath, + string toolName, + YamlMappingNode toolNode, + List issues, + out ToolConfig? toolConfig) + { + var commands = new Dictionary(); + var regexes = new Dictionary(); + var hasCommand = false; + var hasRegex = false; + var toolIssuesBefore = issues.Count; - // Get tools mapping - if (!rootNode.Children.TryGetValue(new YamlScalarNode("tools"), out var toolsNode)) + foreach (var entry in toolNode.Children) + { + // All tool config keys must be scalar values + if (entry.Key is not YamlScalarNode entryKeyNode) { - throw new ArgumentException("Configuration file must contain a 'tools' section"); + issues.Add(CreateIssue(filePath, entry.Key, LintSeverity.Error, $"Tool '{toolName}' configuration keys must be scalar values")); + continue; } - // Validate tools node is a mapping - if (toolsNode is not YamlMappingNode toolsMapping) + // All tool config values must be scalar values + if (entry.Value is not YamlScalarNode entryValueNode) { - throw new ArgumentException("The 'tools' section must be a mapping"); + issues.Add(CreateIssue(filePath, entry.Value, LintSeverity.Error, $"Tool '{toolName}' configuration values must be scalar values")); + continue; } - var tools = new Dictionary(); + var key = entryKeyNode.Value ?? string.Empty; + var value = entryValueNode.Value ?? string.Empty; + + // Warn about unrecognized keys without failing validation + if (!ValidToolKeys.Contains(key)) + { + issues.Add(CreateIssue(filePath, entryKeyNode, LintSeverity.Warning, $"Tool '{toolName}' has unknown key '{key}'")); + } - // Parse each tool - foreach (var toolEntry in toolsMapping.Children) + // Validate default command + if (key == "command") { - if (toolEntry.Key is not YamlScalarNode keyNode) + hasCommand = true; + if (string.IsNullOrWhiteSpace(value)) + { + issues.Add(CreateIssue(filePath, entryValueNode, LintSeverity.Error, $"Tool '{toolName}' 'command' must not be empty")); + } + else { - throw new ArgumentException("Tool names must be scalar values"); + commands[string.Empty] = value; } + } - if (toolEntry.Value is not YamlMappingNode toolNode) + // Validate OS-specific command overrides + else if (key is "command-win" or "command-linux" or "command-macos") + { + if (string.IsNullOrWhiteSpace(value)) { - throw new ArgumentException($"Tool '{keyNode.Value}' configuration must be a mapping"); + issues.Add(CreateIssue(filePath, entryValueNode, LintSeverity.Error, $"Tool '{toolName}' '{key}' must not be empty")); } + else + { + var os = key["command-".Length..]; + commands[os] = value; + } + } - var toolName = keyNode.Value ?? string.Empty; - tools[toolName] = ToolConfig.FromYamlNode(toolNode, toolName); + // Validate default regex + else if (key == "regex") + { + hasRegex = true; + if (string.IsNullOrWhiteSpace(value)) + { + issues.Add(CreateIssue(filePath, entryValueNode, LintSeverity.Error, $"Tool '{toolName}' 'regex' must not be empty")); + } + else + { + var compiled = TryCompileRegex(filePath, toolName, key, value, entryValueNode, issues); + if (compiled != null) + { + if (!compiled.GetGroupNames().Contains("version")) + { + issues.Add(CreateIssue(filePath, entryValueNode, LintSeverity.Error, $"Tool '{toolName}' 'regex' must contain a named 'version' capture group: (?...)")); + } + else + { + regexes[string.Empty] = value; + } + } + } } - // Validate configuration - if (tools.Count == 0) + // Validate OS-specific regex overrides + else if (key is "regex-win" or "regex-linux" or "regex-macos") { - throw new ArgumentException("Configuration must contain at least one tool"); + if (string.IsNullOrWhiteSpace(value)) + { + issues.Add(CreateIssue(filePath, entryValueNode, LintSeverity.Error, $"Tool '{toolName}' '{key}' must not be empty")); + } + else + { + var compiled = TryCompileRegex(filePath, toolName, key, value, entryValueNode, issues); + if (compiled != null) + { + if (!compiled.GetGroupNames().Contains("version")) + { + issues.Add(CreateIssue(filePath, entryValueNode, LintSeverity.Error, $"Tool '{toolName}' '{key}' must contain a named 'version' capture group: (?...)")); + } + else + { + var os = key["regex-".Length..]; + regexes[os] = value; + } + } + } } + } + + // Report missing required fields after scanning all entries + if (!hasCommand) + { + issues.Add(CreateIssue(filePath, toolNode, LintSeverity.Error, $"Tool '{toolName}' must have a 'command' field")); + } - return new VersionMarkConfig(tools); + if (!hasRegex) + { + issues.Add(CreateIssue(filePath, toolNode, LintSeverity.Error, $"Tool '{toolName}' must have a 'regex' field")); } - catch (YamlDotNet.Core.YamlException ex) + + // Only produce a ToolConfig when no new errors were added for this tool + var hasNewErrors = issues.Skip(toolIssuesBefore).Any(i => i.Severity == LintSeverity.Error); + toolConfig = hasNewErrors ? null : new ToolConfig(commands, regexes); + } + + /// + /// Attempts to compile a regular expression, adding an error issue when compilation fails. + /// + /// Path to the configuration file (for issue location). + /// Name of the tool owning the regex (for error messages). + /// Configuration key of the regex field (for error messages). + /// Regex pattern to compile. + /// YAML node that holds the value (for source location). + /// Shared list to which a compilation error is appended on failure. + /// The compiled , or if compilation failed. + private static Regex? TryCompileRegex( + string filePath, + string toolName, + string key, + string value, + YamlNode node, + List issues) + { + try { - throw new ArgumentException($"Failed to parse YAML file '{filePath}': {ex.Message}", ex); + return new Regex(value, RegexOptions.None, RegexTimeout); } - catch (Exception ex) when (ex is not ArgumentException) + catch (ArgumentException ex) { - throw new ArgumentException($"Failed to read configuration file '{filePath}': {ex.Message}", ex); + issues.Add(CreateIssue(filePath, node, LintSeverity.Error, $"Tool '{toolName}' '{key}' contains an invalid regex: {ex.Message}")); + return null; } } + /// + /// Creates a using the source location of a YAML node, + /// converting from the zero-based offsets reported by YamlDotNet to one-based display values. + /// + /// Path to the configuration file. + /// YAML node whose start position provides the line and column. + /// Severity of the issue. + /// Human-readable description of the issue. + /// A new with one-based line and column numbers. + private static LintIssue CreateIssue(string filePath, YamlNode node, LintSeverity severity, string description) + => new(filePath, node.Start.Line + 1, node.Start.Column + 1, severity, description); + /// /// Finds versions for the specified tools. /// diff --git a/src/DemaConsulting.VersionMark/Linting/Lint.cs b/src/DemaConsulting.VersionMark/Linting/Lint.cs deleted file mode 100644 index 9077f75..0000000 --- a/src/DemaConsulting.VersionMark/Linting/Lint.cs +++ /dev/null @@ -1,346 +0,0 @@ -// Copyright (c) DEMA Consulting -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -using System.Text.RegularExpressions; -using DemaConsulting.VersionMark.Cli; -using YamlDotNet.RepresentationModel; - -namespace DemaConsulting.VersionMark.Linting; - -/// -/// Provides lint functionality for checking .versionmark.yaml configuration files. -/// -internal static class Lint -{ - /// - /// Known valid keys for tool configuration entries. - /// - private static readonly HashSet ValidToolKeys = - [ - "command", - "command-win", - "command-linux", - "command-macos", - "regex", - "regex-win", - "regex-linux", - "regex-macos" - ]; - - /// - /// Timeout for regex compilation and matching. - /// - private static readonly TimeSpan RegexTimeout = TimeSpan.FromSeconds(1); - - /// - /// Runs lint checks on the specified configuration file and reports all issues. - /// - /// The context for reporting output. - /// Path to the configuration file to lint. - /// if no issues were found; otherwise . - public static bool Run(Context context, string configFile) - { - ArgumentNullException.ThrowIfNull(context); - ArgumentNullException.ThrowIfNull(configFile); - - context.WriteLine($"Linting '{configFile}'..."); - - // Check if file exists - if (!File.Exists(configFile)) - { - context.WriteError($"{configFile}: error: Configuration file not found"); - return false; - } - - // Parse and validate YAML - YamlStream yaml; - try - { - using var reader = new StreamReader(configFile, System.Text.Encoding.UTF8); - yaml = new YamlStream(); - yaml.Load(reader); - } - catch (YamlDotNet.Core.YamlException ex) - { - var location = FormatLocation(configFile, ex.Start.Line, ex.Start.Column); - context.WriteError($"{location}: error: {ex.Message}"); - return false; - } - - var issueCount = 0; - - // Check document exists - if (yaml.Documents.Count == 0) - { - context.WriteError($"{configFile}: error: YAML file contains no documents"); - issueCount++; - return issueCount == 0; - } - - // Validate root is a mapping - if (yaml.Documents[0].RootNode is not YamlMappingNode rootNode) - { - var location = FormatLocation(configFile, yaml.Documents[0].RootNode.Start.Line, yaml.Documents[0].RootNode.Start.Column); - context.WriteError($"{location}: error: Root node must be a YAML mapping"); - issueCount++; - return issueCount == 0; - } - - // Check for unknown top-level keys (warnings are non-fatal) - foreach (var keyNode in rootNode.Children.Keys.OfType().Where(k => k.Value != "tools")) - { - var location = FormatLocation(configFile, keyNode.Start.Line, keyNode.Start.Column); - context.WriteLine($"{location}: warning: Unknown top-level key '{keyNode.Value}'"); - } - - // Check tools section exists - if (!rootNode.Children.TryGetValue(new YamlScalarNode("tools"), out var toolsNode)) - { - var location = FormatLocation(configFile, rootNode.Start.Line, rootNode.Start.Column); - context.WriteError($"{location}: error: Configuration must contain a 'tools' section"); - issueCount++; - return issueCount == 0; - } - - // Validate tools is a mapping - if (toolsNode is not YamlMappingNode toolsMapping) - { - var location = FormatLocation(configFile, toolsNode.Start.Line, toolsNode.Start.Column); - context.WriteError($"{location}: error: The 'tools' section must be a mapping"); - issueCount++; - return issueCount == 0; - } - - // Check at least one tool is defined - if (toolsMapping.Children.Count == 0) - { - var location = FormatLocation(configFile, toolsMapping.Start.Line, toolsMapping.Start.Column); - context.WriteError($"{location}: error: Configuration must contain at least one tool"); - issueCount++; - } - - // Validate each tool - foreach (var toolEntry in toolsMapping.Children) - { - // Tool name must be a scalar - if (toolEntry.Key is not YamlScalarNode toolKeyNode) - { - var location = FormatLocation(configFile, toolEntry.Key.Start.Line, toolEntry.Key.Start.Column); - context.WriteError($"{location}: error: Tool names must be scalar values"); - issueCount++; - continue; - } - - var toolName = toolKeyNode.Value ?? string.Empty; - - // Tool config must be a mapping - if (toolEntry.Value is not YamlMappingNode toolNode) - { - var location = FormatLocation(configFile, toolEntry.Value.Start.Line, toolEntry.Value.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' configuration must be a mapping"); - issueCount++; - continue; - } - - issueCount += ValidateTool(context, configFile, toolName, toolNode); - } - - if (issueCount == 0) - { - context.WriteLine($"'{configFile}': No issues found"); - } - - return issueCount == 0; - } - - /// - /// Validates a single tool configuration node and reports all issues. - /// - /// The context for reporting output. - /// Path to the configuration file (for error messages). - /// The name of the tool being validated. - /// The YAML mapping node for the tool. - /// The number of issues found. - private static int ValidateTool(Context context, string configFile, string toolName, YamlMappingNode toolNode) - { - var issueCount = 0; - var hasCommand = false; - var hasRegex = false; - - foreach (var entry in toolNode.Children) - { - // All tool config entries must be scalar key-value pairs - if (entry.Key is not YamlScalarNode entryKeyNode) - { - var location = FormatLocation(configFile, entry.Key.Start.Line, entry.Key.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' configuration keys must be scalar values"); - issueCount++; - continue; - } - - if (entry.Value is not YamlScalarNode entryValueNode) - { - var location = FormatLocation(configFile, entry.Value.Start.Line, entry.Value.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' configuration values must be scalar values"); - issueCount++; - continue; - } - - var key = entryKeyNode.Value ?? string.Empty; - var value = entryValueNode.Value ?? string.Empty; - - // Check for unknown keys (warnings are non-fatal) - if (!ValidToolKeys.Contains(key)) - { - var location = FormatLocation(configFile, entry.Key.Start.Line, entry.Key.Start.Column); - context.WriteLine($"{location}: warning: Tool '{toolName}' has unknown key '{key}'"); - } - - // Track required fields - if (key == "command") - { - hasCommand = true; - - // Validate command is not empty - if (string.IsNullOrWhiteSpace(value)) - { - var location = FormatLocation(configFile, entry.Value.Start.Line, entry.Value.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' 'command' must not be empty"); - issueCount++; - } - } - else if (key is "command-win" or "command-linux" or "command-macos") - { - // Validate OS-specific command overrides are not empty - if (string.IsNullOrWhiteSpace(value)) - { - var location = FormatLocation(configFile, entry.Value.Start.Line, entry.Value.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' '{key}' must not be empty"); - issueCount++; - } - } - else if (key == "regex") - { - hasRegex = true; - - // Validate regex is not empty - if (string.IsNullOrWhiteSpace(value)) - { - var location = FormatLocation(configFile, entry.Value.Start.Line, entry.Value.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' 'regex' must not be empty"); - issueCount++; - } - else - { - // Validate regex can be compiled - var compiledRegex = TryCompileRegex(context, configFile, toolName, key, value, entry.Value); - if (compiledRegex == null) - { - issueCount++; - } - else if (!compiledRegex.GetGroupNames().Contains("version")) - { - var location = FormatLocation(configFile, entry.Value.Start.Line, entry.Value.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' 'regex' must contain a named 'version' capture group: (?...)"); - issueCount++; - } - } - } - else if (key is "regex-win" or "regex-linux" or "regex-macos") - { - if (string.IsNullOrWhiteSpace(value)) - { - var location = FormatLocation(configFile, entry.Value.Start.Line, entry.Value.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' '{key}' must not be empty"); - issueCount++; - } - else - { - // Validate OS-specific regex can be compiled - var compiledRegex = TryCompileRegex(context, configFile, toolName, key, value, entry.Value); - if (compiledRegex == null) - { - issueCount++; - } - else if (!compiledRegex.GetGroupNames().Contains("version")) - { - var location = FormatLocation(configFile, entry.Value.Start.Line, entry.Value.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' '{key}' must contain a named 'version' capture group: (?...)"); - issueCount++; - } - } - } - } - - // Report missing required fields - if (!hasCommand) - { - var location = FormatLocation(configFile, toolNode.Start.Line, toolNode.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' must have a 'command' field"); - issueCount++; - } - - if (!hasRegex) - { - var location = FormatLocation(configFile, toolNode.Start.Line, toolNode.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' must have a 'regex' field"); - issueCount++; - } - - return issueCount; - } - - /// - /// Tries to compile a regex pattern, reporting an error if compilation fails. - /// - /// The context for reporting output. - /// Path to the configuration file (for error messages). - /// The name of the tool being validated. - /// The configuration key (for error messages). - /// The regex pattern to compile. - /// The YAML node (for location). - /// The compiled , or if compilation failed. - private static Regex? TryCompileRegex(Context context, string configFile, string toolName, string key, string value, YamlNode node) - { - try - { - return new Regex(value, RegexOptions.None, RegexTimeout); - } - catch (ArgumentException ex) - { - var location = FormatLocation(configFile, node.Start.Line, node.Start.Column); - context.WriteError($"{location}: error: Tool '{toolName}' '{key}' contains an invalid regex: {ex.Message}"); - return null; - } - } - - /// - /// Formats a file location string with 1-based line and column information. - /// - /// The file path. - /// The line number (0-based, as provided by YamlDotNet). - /// The column number (0-based, as provided by YamlDotNet). - /// A formatted location string with 1-based line and column numbers. - private static string FormatLocation(string filePath, long line, long column) - { - // Convert from 0-based (YamlDotNet) to 1-based (display) - return $"{filePath}({line + 1},{column + 1})"; - } -} diff --git a/src/DemaConsulting.VersionMark/Program.cs b/src/DemaConsulting.VersionMark/Program.cs index 23803a2..6e57f92 100644 --- a/src/DemaConsulting.VersionMark/Program.cs +++ b/src/DemaConsulting.VersionMark/Program.cs @@ -22,7 +22,6 @@ using DemaConsulting.VersionMark.Capture; using DemaConsulting.VersionMark.Cli; using DemaConsulting.VersionMark.Configuration; -using DemaConsulting.VersionMark.Linting; using DemaConsulting.VersionMark.Publishing; using DemaConsulting.VersionMark.SelfTest; using Microsoft.Extensions.FileSystemGlobbing; @@ -200,7 +199,29 @@ private static void RunLint(Context context) { // Use specified file, or default to .versionmark.yaml var configFile = context.LintFile ?? ".versionmark.yaml"; - Lint.Run(context, configFile); + context.WriteLine($"Linting '{configFile}'..."); + + // Load the configuration, which performs all validation in a single pass + var (config, issues) = VersionMarkConfig.Load(configFile); + + // Report all discovered issues, routing errors to the error stream + foreach (var issue in issues) + { + if (issue.Severity == LintSeverity.Error) + { + context.WriteError(issue.ToString()); + } + else + { + context.WriteLine(issue.ToString()); + } + } + + // Confirm success when no errors were found + if (config != null) + { + context.WriteLine($"'{configFile}': No issues found"); + } } /// @@ -222,11 +243,28 @@ private static void RunCapture(Context context) context.WriteLine($"Capturing tool versions for job '{context.JobId}'..."); context.WriteLine($"Output file: {outputFile}"); - try + // Load and validate configuration, reporting all issues before proceeding + var (config, issues) = VersionMarkConfig.Load(".versionmark.yaml"); + foreach (var issue in issues) { - // Load configuration from default location - var config = VersionMarkConfig.ReadFromFile(".versionmark.yaml"); + if (issue.Severity == LintSeverity.Error) + { + context.WriteError($"Error: {issue}"); + } + else + { + context.WriteLine(issue.ToString()); + } + } + // Abort capture if the configuration could not be loaded + if (config == null) + { + return; + } + + try + { // Determine which tools to capture var toolNames = context.ToolNames.Length > 0 ? context.ToolNames diff --git a/test/DemaConsulting.VersionMark.Tests/Linting/LintTests.cs b/test/DemaConsulting.VersionMark.Tests/Linting/LintTests.cs index d4f1171..39a54fe 100644 --- a/test/DemaConsulting.VersionMark.Tests/Linting/LintTests.cs +++ b/test/DemaConsulting.VersionMark.Tests/Linting/LintTests.cs @@ -1,15 +1,15 @@ -// Copyright (c) DEMA Consulting -// +// Copyright (c) 2025 DEMA Consulting +// // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal // in the Software without restriction, including without limitation the rights // to use, copy, modify, merge, publish, distribute, sublicense, and/or sell // copies of the Software, and to permit persons to whom the Software is // furnished to do so, subject to the following conditions: -// +// // The above copyright notice and this permission notice shall be included in all // copies or substantial portions of the Software. -// +// // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR // IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, // FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE @@ -18,22 +18,21 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -using DemaConsulting.VersionMark.Cli; -using DemaConsulting.VersionMark.Linting; +using DemaConsulting.VersionMark.Configuration; -namespace DemaConsulting.VersionMark.Tests.Linting; +namespace DemaConsulting.VersionMark.Tests.Configuration; /// -/// Unit tests for the Lint class. +/// Unit tests for the method. /// [TestClass] -public class LintTests +public class VersionMarkConfigLoadTests { /// - /// Test that a valid configuration file with command and regex returns true. + /// Test that a valid configuration file returns a non-null config with no errors. /// [TestMethod] - public void Lint_Run_ValidConfig_ReturnsTrue() + public void VersionMarkConfig_Load_ValidConfig_ReturnsConfig() { // Arrange - Create a well-formed .versionmark.yaml config file var tempFile = Path.GetTempFileName(); @@ -47,14 +46,13 @@ public void Lint_Run_ValidConfig_ReturnsTrue() regex: '(?\d+\.\d+\.\d+)' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a valid config - var result = Lint.Run(context, tempFile); + // Act - Load the config from the valid file + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should succeed with no errors and exit code 0 - Assert.IsTrue(result); - Assert.AreEqual(0, context.ExitCode); + // Assert - Config should be returned with no error issues + Assert.IsNotNull(config); + Assert.IsFalse(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -63,28 +61,27 @@ public void Lint_Run_ValidConfig_ReturnsTrue() } /// - /// Test that a non-existent file returns false with exit code 1. + /// Test that a non-existent file returns null config with an error issue. /// [TestMethod] - public void Lint_Run_MissingFile_ReturnsFalse() + public void VersionMarkConfig_Load_MissingFile_ReturnsNullConfig() { // Arrange - Use a path that does not exist var nonExistentFile = Path.Combine(Path.GetTempPath(), Guid.NewGuid() + ".yaml"); - using var context = Context.Create(["--silent"]); - // Act - Run lint against a missing file - var result = Lint.Run(context, nonExistentFile); + // Act - Attempt to load config from a missing file + var (config, issues) = VersionMarkConfig.Load(nonExistentFile); - // Assert - Lint should fail and report an error - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null and issues should contain an error + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } /// - /// Test that a file containing invalid YAML returns false with exit code 1. + /// Test that a file containing invalid YAML returns null config with an error issue. /// [TestMethod] - public void Lint_Run_InvalidYaml_ReturnsFalse() + public void VersionMarkConfig_Load_InvalidYaml_ReturnsNullConfig() { // Arrange - Write syntactically broken YAML to a temp file var tempFile = Path.GetTempFileName(); @@ -96,14 +93,13 @@ public void Lint_Run_InvalidYaml_ReturnsFalse() command: [unclosed bracket """; File.WriteAllText(tempFile, invalidYaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on malformed YAML - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config from malformed YAML + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail with a parse error - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null and issues should contain a parse error + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -112,10 +108,10 @@ public void Lint_Run_InvalidYaml_ReturnsFalse() } /// - /// Test that a config without a 'tools' section returns false with exit code 1. + /// Test that a config without a 'tools' section returns null config with an error issue. /// [TestMethod] - public void Lint_Run_MissingToolsSection_ReturnsFalse() + public void VersionMarkConfig_Load_MissingToolsSection_ReturnsNullConfig() { // Arrange - Write a YAML file that has no 'tools' key at the root var tempFile = Path.GetTempFileName(); @@ -126,14 +122,13 @@ public void Lint_Run_MissingToolsSection_ReturnsFalse() version: 1 """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a config that lacks a 'tools' section - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config that lacks a 'tools' section + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail because 'tools' is required - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null because 'tools' is required + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -142,10 +137,10 @@ public void Lint_Run_MissingToolsSection_ReturnsFalse() } /// - /// Test that a config with an empty 'tools' section returns false with exit code 1. + /// Test that a config with an empty 'tools' section returns null config with an error issue. /// [TestMethod] - public void Lint_Run_EmptyToolsSection_ReturnsFalse() + public void VersionMarkConfig_Load_EmptyToolsSection_ReturnsNullConfig() { // Arrange - Write a YAML file that has a 'tools' mapping with no entries var tempFile = Path.GetTempFileName(); @@ -156,14 +151,13 @@ public void Lint_Run_EmptyToolsSection_ReturnsFalse() tools: {} """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a config with an empty tools section - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config with an empty tools section + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail because at least one tool is required - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null because at least one tool is required + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -172,10 +166,10 @@ public void Lint_Run_EmptyToolsSection_ReturnsFalse() } /// - /// Test that a tool without a 'command' field returns false with exit code 1. + /// Test that a tool without a 'command' field returns null config with an error issue. /// [TestMethod] - public void Lint_Run_MissingCommand_ReturnsFalse() + public void VersionMarkConfig_Load_MissingCommand_ReturnsNullConfig() { // Arrange - Write a YAML file where the tool entry has no 'command' key var tempFile = Path.GetTempFileName(); @@ -188,14 +182,13 @@ public void Lint_Run_MissingCommand_ReturnsFalse() regex: '(?\d+\.\d+\.\d+)' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool that is missing its required 'command' field - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config with a tool missing its 'command' field + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail because 'command' is required - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null because 'command' is required + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -204,12 +197,12 @@ public void Lint_Run_MissingCommand_ReturnsFalse() } /// - /// Test that a tool with an empty 'command' field returns false with exit code 1. + /// Test that a tool with an empty 'command' field returns null config with an error issue. /// [TestMethod] - public void Lint_Run_EmptyCommand_ReturnsFalse() + public void VersionMarkConfig_Load_EmptyCommand_ReturnsNullConfig() { - // Arrange - Write a YAML file where the tool's 'command' value is blank + // Arrange - Write a YAML file where the tool has an empty 'command' value var tempFile = Path.GetTempFileName(); try { @@ -221,14 +214,13 @@ public void Lint_Run_EmptyCommand_ReturnsFalse() regex: '(?\d+\.\d+\.\d+)' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool with an empty command string - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config with an empty command + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail because an empty command is invalid - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null because an empty command is invalid + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -237,10 +229,10 @@ public void Lint_Run_EmptyCommand_ReturnsFalse() } /// - /// Test that a tool without a 'regex' field returns false with exit code 1. + /// Test that a tool without a 'regex' field returns null config with an error issue. /// [TestMethod] - public void Lint_Run_MissingRegex_ReturnsFalse() + public void VersionMarkConfig_Load_MissingRegex_ReturnsNullConfig() { // Arrange - Write a YAML file where the tool entry has no 'regex' key var tempFile = Path.GetTempFileName(); @@ -253,14 +245,13 @@ public void Lint_Run_MissingRegex_ReturnsFalse() command: dotnet --version """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool that is missing its required 'regex' field - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config with a tool missing its 'regex' field + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail because 'regex' is required - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null because 'regex' is required + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -269,12 +260,12 @@ public void Lint_Run_MissingRegex_ReturnsFalse() } /// - /// Test that a tool with an empty 'regex' field returns false with exit code 1. + /// Test that a tool with an empty 'regex' field returns null config with an error issue. /// [TestMethod] - public void Lint_Run_EmptyRegex_ReturnsFalse() + public void VersionMarkConfig_Load_EmptyRegex_ReturnsNullConfig() { - // Arrange - Write a YAML file where the tool's 'regex' value is blank + // Arrange - Write a YAML file where the tool has an empty 'regex' value var tempFile = Path.GetTempFileName(); try { @@ -286,14 +277,13 @@ public void Lint_Run_EmptyRegex_ReturnsFalse() regex: '' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool with an empty regex string - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config with an empty regex + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail because an empty regex is invalid - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null because an empty regex is invalid + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -302,12 +292,12 @@ public void Lint_Run_EmptyRegex_ReturnsFalse() } /// - /// Test that a tool with a regex that cannot be compiled returns false with exit code 1. + /// Test that a tool with an invalid 'regex' value returns null config with an error issue. /// [TestMethod] - public void Lint_Run_InvalidRegex_ReturnsFalse() + public void VersionMarkConfig_Load_InvalidRegex_ReturnsNullConfig() { - // Arrange - Write a YAML file where the tool's 'regex' is a syntactically invalid pattern + // Arrange - Write a YAML file with a syntactically broken regex (unclosed group) var tempFile = Path.GetTempFileName(); try { @@ -319,14 +309,13 @@ public void Lint_Run_InvalidRegex_ReturnsFalse() regex: '(? i.Severity == LintSeverity.Error)); } finally { @@ -335,12 +324,12 @@ public void Lint_Run_InvalidRegex_ReturnsFalse() } /// - /// Test that a tool with a valid regex that has no 'version' capture group returns false. + /// Test that a tool with a regex missing the 'version' group returns null config with an error issue. /// [TestMethod] - public void Lint_Run_RegexMissingVersionGroup_ReturnsFalse() + public void VersionMarkConfig_Load_RegexMissingVersionGroup_ReturnsNullConfig() { - // Arrange - Write a YAML file where the tool's regex is valid but lacks a 'version' named group + // Arrange - Write a YAML file with a valid regex that lacks the required 'version' named group var tempFile = Path.GetTempFileName(); try { @@ -352,14 +341,13 @@ public void Lint_Run_RegexMissingVersionGroup_ReturnsFalse() regex: '\d+\.\d+\.\d+' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool whose regex has no named 'version' capture group - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config with a regex that has no 'version' group + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail because the 'version' group is required - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null because the 'version' capture group is required + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -368,32 +356,32 @@ public void Lint_Run_RegexMissingVersionGroup_ReturnsFalse() } /// - /// Test that an unknown top-level key produces a warning but returns true (non-fatal). + /// Test that an unknown top-level key produces a warning but config is still returned. /// [TestMethod] - public void Lint_Run_UnknownTopLevelKey_ReturnsTrue() + public void VersionMarkConfig_Load_UnknownTopLevelKey_ReturnsConfig() { - // Arrange - Write a YAML file with a valid tools section plus an unknown top-level key + // Arrange - Write a YAML file with a valid tool plus an unknown top-level key var tempFile = Path.GetTempFileName(); try { const string yaml = """ --- + unknown-top-level-key: some-value tools: dotnet: command: dotnet --version regex: '(?\d+\.\d+\.\d+)' - unknown-key: some-value """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a config containing an unknown top-level key - var result = Lint.Run(context, tempFile); + // Act - Load config that has an unknown top-level key + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should succeed (warnings are non-fatal) with exit code 0 - Assert.IsTrue(result); - Assert.AreEqual(0, context.ExitCode); + // Assert - Config should be returned; unknown keys produce warnings, not errors + Assert.IsNotNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Warning)); + Assert.IsFalse(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -402,12 +390,12 @@ public void Lint_Run_UnknownTopLevelKey_ReturnsTrue() } /// - /// Test that an unknown per-tool key produces a warning but returns true (non-fatal). + /// Test that an unknown tool key produces a warning but config is still returned. /// [TestMethod] - public void Lint_Run_UnknownToolKey_ReturnsTrue() + public void VersionMarkConfig_Load_UnknownToolKey_ReturnsConfig() { - // Arrange - Write a YAML file where a tool has an unknown configuration key + // Arrange - Write a YAML file with a valid tool plus an unknown key inside the tool var tempFile = Path.GetTempFileName(); try { @@ -417,17 +405,17 @@ public void Lint_Run_UnknownToolKey_ReturnsTrue() dotnet: command: dotnet --version regex: '(?\d+\.\d+\.\d+)' - unknown-tool-key: ignored + unknown-tool-key: should-not-fail """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool that contains an unknown configuration key - var result = Lint.Run(context, tempFile); + // Act - Load config containing an unknown tool key + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should succeed (unknown tool keys are non-fatal warnings) with exit code 0 - Assert.IsTrue(result); - Assert.AreEqual(0, context.ExitCode); + // Assert - Config should be returned; unknown tool keys produce warnings, not errors + Assert.IsNotNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Warning)); + Assert.IsFalse(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -436,12 +424,12 @@ public void Lint_Run_UnknownToolKey_ReturnsTrue() } /// - /// Test that an OS-specific command override with an empty value returns false. + /// Test that an empty OS-specific command override returns null config with an error issue. /// [TestMethod] - public void Lint_Run_OsSpecificEmptyCommand_ReturnsFalse() + public void VersionMarkConfig_Load_OsSpecificEmptyCommand_ReturnsNullConfig() { - // Arrange - Write a YAML file where command-win is present but empty + // Arrange - Write a YAML file with an empty command-win override var tempFile = Path.GetTempFileName(); try { @@ -454,14 +442,13 @@ public void Lint_Run_OsSpecificEmptyCommand_ReturnsFalse() regex: '(?\d+\.\d+\.\d+)' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool with an empty OS-specific command override - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config with an empty OS-specific command override + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail because an empty command-win is invalid - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null because empty OS-specific overrides are not allowed + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -470,12 +457,12 @@ public void Lint_Run_OsSpecificEmptyCommand_ReturnsFalse() } /// - /// Test that an OS-specific regex override with an empty value returns false. + /// Test that an empty OS-specific regex override returns null config with an error issue. /// [TestMethod] - public void Lint_Run_OsSpecificEmptyRegex_ReturnsFalse() + public void VersionMarkConfig_Load_OsSpecificEmptyRegex_ReturnsNullConfig() { - // Arrange - Write a YAML file where regex-win is present but empty + // Arrange - Write a YAML file with an empty regex-linux override var tempFile = Path.GetTempFileName(); try { @@ -485,17 +472,16 @@ public void Lint_Run_OsSpecificEmptyRegex_ReturnsFalse() dotnet: command: dotnet --version regex: '(?\d+\.\d+\.\d+)' - regex-win: '' + regex-linux: '' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool with an empty OS-specific regex override - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config with an empty OS-specific regex override + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail because an empty regex-win is invalid - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null because empty OS-specific regex overrides are not allowed + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -504,12 +490,12 @@ public void Lint_Run_OsSpecificEmptyRegex_ReturnsFalse() } /// - /// Test that an OS-specific regex override that lacks a 'version' capture group returns false. + /// Test that an OS-specific regex missing the 'version' group returns null config with an error issue. /// [TestMethod] - public void Lint_Run_OsSpecificRegexMissingVersionGroup_ReturnsFalse() + public void VersionMarkConfig_Load_OsSpecificRegexMissingVersionGroup_ReturnsNullConfig() { - // Arrange - Write a YAML file where regex-win is a valid pattern but has no 'version' named group + // Arrange - Write a YAML file with an OS-specific regex that has no 'version' named group var tempFile = Path.GetTempFileName(); try { @@ -519,17 +505,16 @@ public void Lint_Run_OsSpecificRegexMissingVersionGroup_ReturnsFalse() dotnet: command: dotnet --version regex: '(?\d+\.\d+\.\d+)' - regex-win: '\d+\.\d+\.\d+' + regex-macos: '\d+\.\d+\.\d+' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool with an OS-specific regex that has no 'version' group - var result = Lint.Run(context, tempFile); + // Act - Attempt to load config with an OS-specific regex missing the 'version' group + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Assert - Lint should fail because the 'version' group is required even for OS-specific overrides - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null because the 'version' group is required in all regexes + Assert.IsNull(config); + Assert.IsTrue(issues.Any(i => i.Severity == LintSeverity.Error)); } finally { @@ -538,12 +523,12 @@ public void Lint_Run_OsSpecificRegexMissingVersionGroup_ReturnsFalse() } /// - /// Test that an OS-specific regex override that cannot be compiled returns false. + /// Test that an OS-specific regex that is invalid returns null config with an error issue. /// [TestMethod] - public void Lint_Run_OsSpecificInvalidRegex_ReturnsFalse() + public void VersionMarkConfig_Load_OsSpecificInvalidRegex_ReturnsNullConfig() { - // Arrange - Write a YAML file where regex-linux contains an invalid regex pattern + // Arrange - Write a YAML file with a broken OS-specific regex (unclosed group) var tempFile = Path.GetTempFileName(); try { @@ -553,17 +538,16 @@ public void Lint_Run_OsSpecificInvalidRegex_ReturnsFalse() dotnet: command: dotnet --version regex: '(?\d+\.\d+\.\d+)' - regex-linux: '(? i.Severity == LintSeverity.Error)); } finally { @@ -572,65 +556,50 @@ public void Lint_Run_OsSpecificInvalidRegex_ReturnsFalse() } /// - /// Test that a config with multiple errors reports all of them (does not stop at the first). + /// Test that multiple errors in different tools are all reported in a single Load call. /// [TestMethod] - public void Lint_Run_MultipleErrors_ReportsAll() + public void VersionMarkConfig_Load_MultipleErrors_ReportsAll() { - // Arrange - Redirect Console.Error to verify both error messages are emitted - var previousError = Console.Error; - var errorOutput = new System.Text.StringBuilder(); + // Arrange - Write a config where tool1 is missing 'regex' and tool2 is missing 'command' var tempFile = Path.GetTempFileName(); try { - // A tool with neither 'command' nor 'regex' should produce two separate error messages const string yaml = """ --- tools: - dotnet: {} + tool1: + command: tool1 --version + tool2: + regex: '(?\d+)' """; File.WriteAllText(tempFile, yaml); - using var captureWriter = new System.IO.StringWriter(errorOutput); - Console.SetError(captureWriter); + // Act - Load a config containing errors in multiple tools + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Context must NOT be silent so that WriteError calls Console.Error.WriteLine - using var context = Context.Create([]); - - // Act - Run lint on a tool that has no required fields at all - var result = Lint.Run(context, tempFile); - captureWriter.Flush(); - - // Assert - Lint should fail (both errors accumulated) - Assert.IsFalse(result); - Assert.AreEqual(1, context.ExitCode); + // Assert - Config should be null and issues should reference both tool1 and tool2 + Assert.IsNull(config); + Assert.IsTrue( + issues.Any(i => i.Severity == LintSeverity.Error && i.Description.Contains("tool1")), + "Issues should contain an error mentioning tool1 (missing regex)"); + Assert.IsTrue( + issues.Any(i => i.Severity == LintSeverity.Error && i.Description.Contains("tool2")), + "Issues should contain an error mentioning tool2 (missing command)"); } finally { - Console.SetError(previousError); File.Delete(tempFile); } - - // Assert - Both error messages must be present, confirming lint does not stop at the first - var captured = errorOutput.ToString(); - Assert.IsTrue( - captured.Contains("'command'"), - $"Expected error about missing 'command' field, but got: {captured}"); - Assert.IsTrue( - captured.Contains("'regex'"), - $"Expected error about missing 'regex' field, but got: {captured}"); } /// - /// Test that error messages contain the config file path. + /// Test that all issues include the file path of the configuration file. /// [TestMethod] - public void Lint_Run_ErrorMessageContainsFileName() + public void VersionMarkConfig_Load_IssuesContainFilePath() { - // Arrange - Redirect Console.Error so we can inspect the messages that Lint emits. - // A missing-command error is straightforward to trigger reliably. - var previousError = Console.Error; - var errorOutput = new System.Text.StringBuilder(); + // Arrange - Write a config with a missing required field to force an error issue var tempFile = Path.GetTempFileName(); try { @@ -642,26 +611,18 @@ public void Lint_Run_ErrorMessageContainsFileName() """; File.WriteAllText(tempFile, yaml); - using var captureWriter = new System.IO.StringWriter(errorOutput); - Console.SetError(captureWriter); + // Act - Load the config and inspect the returned issues + var (config, issues) = VersionMarkConfig.Load(tempFile); - // Context must NOT be silent so that WriteError calls Console.Error.WriteLine - using var context = Context.Create([]); - - // Act - Run lint on a tool that is missing its required 'command' field - Lint.Run(context, tempFile); - captureWriter.Flush(); + // Assert - All issues should reference the path of the config file that was loaded + Assert.IsNull(config); + Assert.IsTrue( + issues.Any(i => i.FilePath == tempFile), + "At least one issue should contain the config file path"); } finally { - Console.SetError(previousError); File.Delete(tempFile); } - - // Assert - The error message should embed the config file path so users know which file to fix - var captured = errorOutput.ToString(); - Assert.IsTrue( - captured.Contains(Path.GetFileName(tempFile)), - $"Expected error output to contain the file name '{Path.GetFileName(tempFile)}', but got: {captured}"); } } diff --git a/test/DemaConsulting.VersionMark.Tests/Linting/LintingSubsystemTests.cs b/test/DemaConsulting.VersionMark.Tests/Linting/LintingSubsystemTests.cs index 0029be4..c813c40 100644 --- a/test/DemaConsulting.VersionMark.Tests/Linting/LintingSubsystemTests.cs +++ b/test/DemaConsulting.VersionMark.Tests/Linting/LintingSubsystemTests.cs @@ -1,15 +1,15 @@ -// Copyright (c) DEMA Consulting -// +// Copyright (c) 2025 DEMA Consulting +// // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal // in the Software without restriction, including without limitation the rights // to use, copy, modify, merge, publish, distribute, sublicense, and/or sell // copies of the Software, and to permit persons to whom the Software is // furnished to do so, subject to the following conditions: -// +// // The above copyright notice and this permission notice shall be included in all // copies or substantial portions of the Software. -// +// // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR // IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, // FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE @@ -19,21 +19,20 @@ // SOFTWARE. using DemaConsulting.VersionMark.Cli; -using DemaConsulting.VersionMark.Linting; -namespace DemaConsulting.VersionMark.Tests.Linting; +namespace DemaConsulting.VersionMark.Tests.Configuration; /// -/// Subsystem tests for the Linting subsystem (Lint and Context working together). +/// Subsystem tests for configuration loading via with the --lint flag. /// [TestClass] -public class LintingSubsystemTests +public class ConfigurationLoadSubsystemTests { /// - /// Test that the full linting pipeline succeeds and exits cleanly for a valid configuration. + /// Test that the full configuration load pipeline succeeds and exits cleanly for a valid configuration. /// [TestMethod] - public void LintingSubsystem_Lint_ValidConfig_SucceedsWithZeroExitCode() + public void ConfigurationLoad_ValidConfig_SucceedsWithZeroExitCode() { // Arrange - Write a complete and valid configuration to a temp file var tempFile = Path.GetTempFileName(); @@ -50,15 +49,14 @@ public void LintingSubsystem_Lint_ValidConfig_SucceedsWithZeroExitCode() regex: 'git version (?[\d\.]+)' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run the full linting pipeline - var result = Lint.Run(context, tempFile); + // Act - Run the program with the --lint flag against the valid config + using var context = Context.Create(["--silent", "--lint", tempFile]); + Program.Run(context); - // Assert - The linting subsystem should report success with a clean exit code - Assert.IsTrue(result, "Linting should succeed for a valid configuration"); + // Assert - The program should report success with a clean exit code Assert.AreEqual(0, context.ExitCode, - "Exit code should be zero after successful lint through the full linting pipeline"); + "Exit code should be zero after successful configuration load"); } finally { @@ -67,10 +65,10 @@ public void LintingSubsystem_Lint_ValidConfig_SucceedsWithZeroExitCode() } /// - /// Test that the linting pipeline reports all errors in a single pass for an invalid configuration. + /// Test that the configuration load pipeline reports all errors in a single pass for an invalid configuration. /// [TestMethod] - public void LintingSubsystem_Lint_MultipleErrors_ReportsAllErrorsInSinglePass() + public void ConfigurationLoad_MultipleErrors_ReportsAllErrorsInSinglePass() { // Arrange - Write a configuration with multiple errors to a temp file // tool1 is missing 'regex'; tool2 is missing 'command' and has a regex without a 'version' group @@ -87,22 +85,19 @@ public void LintingSubsystem_Lint_MultipleErrors_ReportsAllErrorsInSinglePass() """; File.WriteAllText(tempFile, yaml); - // Context without --silent so errors are written to Console.Error - using var context = Context.Create([]); var originalError = Console.Error; try { using var errorWriter = new StringWriter(); Console.SetError(errorWriter); - // Act - Run the full linting pipeline against a config with multiple errors - var result = Lint.Run(context, tempFile); + // Act - Run the program against a config with multiple errors + using var context = Context.Create(["--lint", tempFile]); + Program.Run(context); - // Assert - The linting subsystem should report failure and emit findings for both tools - Assert.IsFalse(result, - "Linting should fail for a configuration with multiple errors"); + // Assert - The program should report failure and emit findings for both tools Assert.AreEqual(1, context.ExitCode, - "Exit code should be non-zero when linting finds errors"); + "Exit code should be non-zero when configuration load finds errors"); var errorOutput = errorWriter.ToString(); StringAssert.Contains(errorOutput, "tool1", @@ -122,24 +117,22 @@ public void LintingSubsystem_Lint_MultipleErrors_ReportsAllErrorsInSinglePass() } /// - /// Test that the linting pipeline fails for invalid YAML content. + /// Test that the configuration load pipeline fails for invalid YAML content. /// [TestMethod] - public void LintingSubsystem_Lint_InvalidYaml_Fails() + public void ConfigurationLoad_InvalidYaml_Fails() { - // Arrange + // Arrange - Write syntactically broken YAML to a temp file var tempFile = Path.GetTempFileName() + ".yaml"; File.WriteAllText(tempFile, "tools:\n dotnet:\n command: [unclosed bracket"); try { - using var context = Context.Create(["--silent"]); + // Act - Run the program against malformed YAML + using var context = Context.Create(["--silent", "--lint", tempFile]); + Program.Run(context); - // Act - var result = Lint.Run(context, tempFile); - - // Assert - Assert.IsFalse(result); + // Assert - The program should fail with a non-zero exit code Assert.AreEqual(1, context.ExitCode); } finally @@ -149,28 +142,27 @@ public void LintingSubsystem_Lint_InvalidYaml_Fails() } /// - /// Test that linting reports an error when the config file does not exist. + /// Test that the configuration load pipeline reports an error when the config file does not exist. /// [TestMethod] - public void LintingSubsystem_Lint_NonExistentFile_Fails() + public void ConfigurationLoad_NonExistentFile_Fails() { - // Arrange + // Arrange - Use a path that does not exist var nonExistentPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid() + ".yaml"); - using var context = Context.Create(["--silent"]); - // Act - var result = Lint.Run(context, nonExistentPath); + // Act - Run the program against a missing file + using var context = Context.Create(["--silent", "--lint", nonExistentPath]); + Program.Run(context); - // Assert - Assert.IsFalse(result); + // Assert - The program should fail with a non-zero exit code Assert.AreEqual(1, context.ExitCode); } /// - /// Test that linting reports an error when a regex cannot be compiled. + /// Test that the configuration load pipeline reports an error when a regex cannot be compiled. /// [TestMethod] - public void LintingSubsystem_Lint_InvalidRegex_ReportsError() + public void ConfigurationLoad_InvalidRegex_ReportsError() { // Arrange - Write a config with a syntactically broken regex (unclosed group) var tempFile = Path.GetTempFileName(); @@ -184,15 +176,14 @@ public void LintingSubsystem_Lint_InvalidRegex_ReportsError() regex: '(? - /// Test that linting reports an error when a regex does not contain a named 'version' capture group. + /// Test that the configuration load pipeline reports an error when a regex does not contain a named 'version' capture group. /// [TestMethod] - public void LintingSubsystem_Lint_RegexWithoutVersionGroup_ReportsError() + public void ConfigurationLoad_RegexWithoutVersionGroup_ReportsError() { // Arrange - Write a config with a valid regex that lacks the required 'version' group var tempFile = Path.GetTempFileName(); @@ -218,15 +209,14 @@ public void LintingSubsystem_Lint_RegexWithoutVersionGroup_ReportsError() regex: '\d+\.\d+\.\d+' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool whose regex has no 'version' named capture group - var result = Lint.Run(context, tempFile); + // Act - Run the program on a tool whose regex has no 'version' named capture group + using var context = Context.Create(["--silent", "--lint", tempFile]); + Program.Run(context); - // Assert - Lint should fail because the 'version' group is required - Assert.IsFalse(result, - "Lint should fail when a regex does not contain a named 'version' capture group"); - Assert.AreEqual(1, context.ExitCode); + // Assert - The program should fail because the 'version' group is required + Assert.AreEqual(1, context.ExitCode, + "Exit code should be non-zero when a regex does not contain a named 'version' capture group"); } finally { @@ -235,10 +225,10 @@ public void LintingSubsystem_Lint_RegexWithoutVersionGroup_ReportsError() } /// - /// Test that linting reports an error when an OS-specific command override is empty. + /// Test that the configuration load pipeline reports an error when an OS-specific command override is empty. /// [TestMethod] - public void LintingSubsystem_Lint_EmptyOsSpecificOverride_ReportsError() + public void ConfigurationLoad_EmptyOsSpecificOverride_ReportsError() { // Arrange - Write a config with an empty command-win override var tempFile = Path.GetTempFileName(); @@ -253,15 +243,14 @@ public void LintingSubsystem_Lint_EmptyOsSpecificOverride_ReportsError() regex: '(?\d+\.\d+\.\d+)' """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a tool with an empty OS-specific override - var result = Lint.Run(context, tempFile); + // Act - Run the program on a tool with an empty OS-specific override + using var context = Context.Create(["--silent", "--lint", tempFile]); + Program.Run(context); - // Assert - Lint should fail because empty OS-specific overrides are not allowed - Assert.IsFalse(result, - "Lint should fail when an OS-specific command override is empty"); - Assert.AreEqual(1, context.ExitCode); + // Assert - The program should fail because empty OS-specific overrides are not allowed + Assert.AreEqual(1, context.ExitCode, + "Exit code should be non-zero when an OS-specific command override is empty"); } finally { @@ -270,10 +259,10 @@ public void LintingSubsystem_Lint_EmptyOsSpecificOverride_ReportsError() } /// - /// Test that linting treats unknown keys as non-fatal warnings and succeeds. + /// Test that the configuration load pipeline treats unknown keys as non-fatal warnings and succeeds. /// [TestMethod] - public void LintingSubsystem_Lint_UnknownKey_IsWarningNotError() + public void ConfigurationLoad_UnknownKey_IsWarningNotError() { // Arrange - Write a config with a valid tool plus an unknown key var tempFile = Path.GetTempFileName(); @@ -288,15 +277,14 @@ public void LintingSubsystem_Lint_UnknownKey_IsWarningNotError() unknown-tool-key: should-not-fail """; File.WriteAllText(tempFile, yaml); - using var context = Context.Create(["--silent"]); - // Act - Run lint on a config containing an unknown tool key - var result = Lint.Run(context, tempFile); + // Act - Run the program on a config containing an unknown tool key + using var context = Context.Create(["--silent", "--lint", tempFile]); + Program.Run(context); - // Assert - Lint should succeed; unknown keys produce warnings, not errors - Assert.IsTrue(result, - "Lint should succeed when only unknown keys are present (warnings are non-fatal)"); - Assert.AreEqual(0, context.ExitCode); + // Assert - The program should succeed; unknown keys produce warnings, not errors + Assert.AreEqual(0, context.ExitCode, + "Exit code should be zero when only unknown keys are present (warnings are non-fatal)"); } finally { @@ -305,10 +293,10 @@ public void LintingSubsystem_Lint_UnknownKey_IsWarningNotError() } /// - /// Test that linting error messages include the filename and line/column location. + /// Test that configuration load error messages include the filename and line/column location. /// [TestMethod] - public void LintingSubsystem_Lint_Error_IncludesFileAndLineInfo() + public void ConfigurationLoad_Error_IncludesFileAndLineInfo() { // Arrange - Write a config missing the required 'command' field and capture error output var tempFile = Path.GetTempFileName(); @@ -322,19 +310,18 @@ public void LintingSubsystem_Lint_Error_IncludesFileAndLineInfo() """; File.WriteAllText(tempFile, yaml); - // Context without --silent so errors are written to Console.Error - using var context = Context.Create([]); var originalError = Console.Error; try { using var errWriter = new StringWriter(); Console.SetError(errWriter); - // Act - Run lint on a config with a missing command field - var result = Lint.Run(context, tempFile); + // Act - Run the program on a config with a missing command field + using var context = Context.Create(["--lint", tempFile]); + Program.Run(context); // Assert - The error message should contain the filename and line/column info - Assert.IsFalse(result); + Assert.AreEqual(1, context.ExitCode); var errorOutput = errWriter.ToString(); StringAssert.Contains(errorOutput, Path.GetFileName(tempFile), "Error message should include the config filename"); From 4db1754abfbc1272bc8e556fb5a134d9b9251fe3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 14:20:37 +0000 Subject: [PATCH 3/6] Fix XML doc comment indentation in LintIssue.cs Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- .../Configuration/LintIssue.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs b/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs index b1726d5..e1bc16b 100644 --- a/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs +++ b/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs @@ -21,23 +21,23 @@ namespace DemaConsulting.VersionMark.Configuration; /// -/// Severity level for a lint issue. +/// Severity level for a lint issue. /// internal enum LintSeverity { /// - /// Non-fatal advisory message. + /// Non-fatal advisory message. /// Warning, /// - /// Fatal validation failure that prevents loading. + /// Fatal validation failure that prevents loading. /// Error } /// -/// Represents a single validation issue found while loading a configuration file. +/// Represents a single validation issue found while loading a configuration file. /// /// Path to the file that contains the issue. /// One-based line number of the issue. @@ -52,10 +52,10 @@ internal sealed record LintIssue( string Description) { /// - /// Returns a formatted string representation of the issue suitable for display to the user. + /// Returns a formatted string representation of the issue suitable for display to the user. /// /// - /// A string in the format "{FilePath}({Line},{Column}): {severity}: {Description}". + /// A string in the format "{FilePath}({Line},{Column}): {severity}: {Description}". /// public override string ToString() => $"{FilePath}({Line},{Column}): {Severity.ToString().ToLowerInvariant()}: {Description}"; From e5115e91cded90d21186c2b8cf6df37e8f7be3cd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 14:45:35 +0000 Subject: [PATCH 4/6] Feature: Unified VersionMarkConfig.Load with integrated linting, remove Linting subsystem Agent-Logs-Url: https://github.com/demaconsulting/VersionMark/sessions/ec1b4a02-ad57-45f4-8173-991132196346 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- .../Configuration/VersionMarkConfig.cs | 77 +------------------ src/DemaConsulting.VersionMark/Program.cs | 2 +- .../IntegrationTests.cs | 2 +- .../ProgramTests.cs | 2 +- 4 files changed, 6 insertions(+), 77 deletions(-) diff --git a/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs b/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs index 74c7787..980606e 100644 --- a/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs +++ b/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs @@ -22,8 +22,8 @@ using System.Runtime.InteropServices; using System.Text.RegularExpressions; using DemaConsulting.VersionMark.Capture; -using YamlDotNet.RepresentationModel; using YamlDotNet.Core; +using YamlDotNet.RepresentationModel; namespace DemaConsulting.VersionMark.Configuration; @@ -53,77 +53,6 @@ internal ToolConfig(Dictionary command, Dictionary - /// Creates a ToolConfig from a YAML mapping node. - /// - /// The YAML mapping node containing tool configuration. - /// The name of the tool (for error messages). - /// A new ToolConfig instance. - /// Thrown when required fields are missing or node types are invalid. - /// Unknown keys in the configuration are silently ignored to allow for future extensibility. - internal static ToolConfig FromYamlNode(YamlMappingNode node, string toolName = "") - { - var commands = new Dictionary(); - var regexes = new Dictionary(); - - foreach (var entry in node.Children) - { - if (entry.Key is not YamlScalarNode keyNode || entry.Value is not YamlScalarNode valueNode) - { - var toolContext = string.IsNullOrEmpty(toolName) ? "Tool" : $"Tool '{toolName}'"; - throw new ArgumentException($"{toolContext} configuration entries must be scalar key-value pairs"); - } - - var key = keyNode.Value ?? string.Empty; - var value = valueNode.Value ?? string.Empty; - - switch (key) - { - case "command": - commands[string.Empty] = value; - break; - case "command-win": - commands["win"] = value; - break; - case "command-linux": - commands["linux"] = value; - break; - case "command-macos": - commands["macos"] = value; - break; - case "regex": - regexes[string.Empty] = value; - break; - case "regex-win": - regexes["win"] = value; - break; - case "regex-linux": - regexes["linux"] = value; - break; - case "regex-macos": - regexes["macos"] = value; - break; - default: - // Ignore unknown keys to allow for future extensibility - break; - } - } - - var toolContext2 = string.IsNullOrEmpty(toolName) ? "Tool" : $"Tool '{toolName}'"; - - if (!commands.ContainsKey(string.Empty)) - { - throw new ArgumentException($"{toolContext2} configuration must contain a default 'command' field"); - } - - if (!regexes.ContainsKey(string.Empty)) - { - throw new ArgumentException($"{toolContext2} configuration must contain a default 'regex' field"); - } - - return new ToolConfig(commands, regexes); - } - /// /// Gets the current operating system name. /// @@ -252,7 +181,7 @@ internal static (VersionMarkConfig? Config, IReadOnlyList Issues) Loa // Check if file exists before attempting to parse if (!File.Exists(filePath)) { - issues.Add(new LintIssue(filePath, 1, 1, LintSeverity.Error, $"Configuration file not found: {filePath}")); + issues.Add(new LintIssue(filePath, 1, 1, LintSeverity.Error, "Configuration file not found")); return (null, issues); } @@ -266,7 +195,7 @@ internal static (VersionMarkConfig? Config, IReadOnlyList Issues) Loa } catch (YamlException ex) { - issues.Add(new LintIssue(filePath, ex.Start.Line + 1, ex.Start.Column + 1, LintSeverity.Error, $"Failed to parse YAML file '{filePath}': {ex.Message}")); + issues.Add(new LintIssue(filePath, ex.Start.Line + 1, ex.Start.Column + 1, LintSeverity.Error, $"Failed to parse YAML file: {ex.Message}")); return (null, issues); } diff --git a/src/DemaConsulting.VersionMark/Program.cs b/src/DemaConsulting.VersionMark/Program.cs index 6e57f92..523d046 100644 --- a/src/DemaConsulting.VersionMark/Program.cs +++ b/src/DemaConsulting.VersionMark/Program.cs @@ -249,7 +249,7 @@ private static void RunCapture(Context context) { if (issue.Severity == LintSeverity.Error) { - context.WriteError($"Error: {issue}"); + context.WriteError(issue.ToString()); } else { diff --git a/test/DemaConsulting.VersionMark.Tests/IntegrationTests.cs b/test/DemaConsulting.VersionMark.Tests/IntegrationTests.cs index 787ee42..537358e 100644 --- a/test/DemaConsulting.VersionMark.Tests/IntegrationTests.cs +++ b/test/DemaConsulting.VersionMark.Tests/IntegrationTests.cs @@ -439,7 +439,7 @@ public void IntegrationTest_CaptureCommandWithMissingConfig_ReturnsError() // Assert - Verify the command fails with error message about missing config Assert.AreNotEqual(0, exitCode); - Assert.Contains("Error:", output); + Assert.Contains("error:", output); } /// diff --git a/test/DemaConsulting.VersionMark.Tests/ProgramTests.cs b/test/DemaConsulting.VersionMark.Tests/ProgramTests.cs index 10cda3b..4d99d25 100644 --- a/test/DemaConsulting.VersionMark.Tests/ProgramTests.cs +++ b/test/DemaConsulting.VersionMark.Tests/ProgramTests.cs @@ -359,7 +359,7 @@ public void Program_Run_WithCaptureCommandWithMissingConfig_ReturnsError() // Assert - Verify error is reported on stderr and exit code indicates failure var errorOutput = errWriter.ToString(); - Assert.Contains("Error:", errorOutput); + Assert.Contains("error:", errorOutput); Assert.AreEqual(1, context.ExitCode); } finally From 5df947a84e050a39aef86731952503e2fd003937 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 14:57:07 +0000 Subject: [PATCH 5/6] Refactor: Introduce VersionMarkLoadResult record with ReportIssues helper to eliminate duplicate issue-reporting code Agent-Logs-Url: https://github.com/demaconsulting/VersionMark/sessions/fd67ed8e-5bbe-45e8-af79-f174afb5f2e5 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- .../Configuration/LintIssue.cs | 41 +++++++++++++++++++ .../Configuration/VersionMarkConfig.cs | 32 +++++++-------- src/DemaConsulting.VersionMark/Program.cs | 38 ++++------------- 3 files changed, 66 insertions(+), 45 deletions(-) diff --git a/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs b/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs index e1bc16b..62169e7 100644 --- a/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs +++ b/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs @@ -18,6 +18,8 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. +using DemaConsulting.VersionMark.Cli; + namespace DemaConsulting.VersionMark.Configuration; /// @@ -60,3 +62,42 @@ internal sealed record LintIssue( public override string ToString() => $"{FilePath}({Line},{Column}): {Severity.ToString().ToLowerInvariant()}: {Description}"; } + +/// +/// Result returned by , containing both the loaded +/// configuration (when successful) and the full list of validation issues found during loading. +/// +/// +/// The loaded , or when one or more +/// -severity issues prevented the configuration from being built. +/// +/// +/// All validation issues found during loading. Always populated; may contain warnings even +/// when is non-. +/// +internal sealed record VersionMarkLoadResult( + VersionMarkConfig? Config, + IReadOnlyList Issues) +{ + /// + /// Writes all validation issues to the specified context, routing errors to the error + /// stream and warnings to the standard output stream. + /// + /// The context used to write output. + public void ReportIssues(Context context) + { + ArgumentNullException.ThrowIfNull(context); + + foreach (var issue in Issues) + { + if (issue.Severity == LintSeverity.Error) + { + context.WriteError($"Error: {issue}"); + } + else + { + context.WriteLine(issue.ToString()); + } + } + } +} diff --git a/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs b/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs index 980606e..8f8eade 100644 --- a/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs +++ b/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs @@ -170,11 +170,11 @@ internal VersionMarkConfig(Dictionary tools) /// /// Path to the YAML configuration file. /// - /// A tuple containing the loaded (or - /// when fatal errors prevent loading) and a read-only list of objects - /// describing all warnings and errors encountered. + /// A containing the loaded + /// (or when fatal errors prevent loading) and a read-only list of + /// objects describing all warnings and errors encountered. /// - internal static (VersionMarkConfig? Config, IReadOnlyList Issues) Load(string filePath) + internal static VersionMarkLoadResult Load(string filePath) { var issues = new List(); @@ -182,7 +182,7 @@ internal static (VersionMarkConfig? Config, IReadOnlyList Issues) Loa if (!File.Exists(filePath)) { issues.Add(new LintIssue(filePath, 1, 1, LintSeverity.Error, "Configuration file not found")); - return (null, issues); + return new VersionMarkLoadResult(null, issues); } // Parse YAML, reporting any syntax errors with their source location @@ -196,14 +196,14 @@ internal static (VersionMarkConfig? Config, IReadOnlyList Issues) Loa catch (YamlException ex) { issues.Add(new LintIssue(filePath, ex.Start.Line + 1, ex.Start.Column + 1, LintSeverity.Error, $"Failed to parse YAML file: {ex.Message}")); - return (null, issues); + return new VersionMarkLoadResult(null, issues); } // Validate that the file contains at least one YAML document if (yaml.Documents.Count == 0) { issues.Add(new LintIssue(filePath, 1, 1, LintSeverity.Error, "YAML file contains no documents")); - return (null, issues); + return new VersionMarkLoadResult(null, issues); } // Validate that the root node is a YAML mapping @@ -211,7 +211,7 @@ internal static (VersionMarkConfig? Config, IReadOnlyList Issues) Loa { var node = yaml.Documents[0].RootNode; issues.Add(CreateIssue(filePath, node, LintSeverity.Error, "Root node must be a YAML mapping")); - return (null, issues); + return new VersionMarkLoadResult(null, issues); } // Warn about unknown top-level keys; they are non-fatal @@ -224,21 +224,21 @@ internal static (VersionMarkConfig? Config, IReadOnlyList Issues) Loa if (!rootNode.Children.TryGetValue(new YamlScalarNode("tools"), out var toolsNode)) { issues.Add(CreateIssue(filePath, rootNode, LintSeverity.Error, "Configuration must contain a 'tools' section")); - return (null, issues); + return new VersionMarkLoadResult(null, issues); } // Validate that 'tools' is a YAML mapping if (toolsNode is not YamlMappingNode toolsMapping) { issues.Add(CreateIssue(filePath, toolsNode, LintSeverity.Error, "The 'tools' section must be a mapping")); - return (null, issues); + return new VersionMarkLoadResult(null, issues); } // Require at least one tool to be defined if (toolsMapping.Children.Count == 0) { issues.Add(CreateIssue(filePath, toolsMapping, LintSeverity.Error, "Configuration must contain at least one tool")); - return (null, issues); + return new VersionMarkLoadResult(null, issues); } // Validate each tool entry and collect all issues before deciding whether to build the config @@ -272,11 +272,11 @@ internal static (VersionMarkConfig? Config, IReadOnlyList Issues) Loa // Return null config if any errors were found, so callers can distinguish warnings-only from failures if (issues.Any(i => i.Severity == LintSeverity.Error)) { - return (null, issues); + return new VersionMarkLoadResult(null, issues); } // Build and return the successfully validated configuration - return (new VersionMarkConfig(tools), issues); + return new VersionMarkLoadResult(new VersionMarkConfig(tools), issues); } /// @@ -288,14 +288,14 @@ internal static (VersionMarkConfig? Config, IReadOnlyList Issues) Loa public static VersionMarkConfig ReadFromFile(string filePath) { // Delegate to Load and convert the first error to an ArgumentException for backward compatibility - var (config, issues) = Load(filePath); - var firstError = issues.FirstOrDefault(i => i.Severity == LintSeverity.Error); + var result = Load(filePath); + var firstError = result.Issues.FirstOrDefault(i => i.Severity == LintSeverity.Error); if (firstError != null) { throw new ArgumentException(firstError.Description); } - return config!; + return result.Config!; } /// diff --git a/src/DemaConsulting.VersionMark/Program.cs b/src/DemaConsulting.VersionMark/Program.cs index 523d046..0b6a5ae 100644 --- a/src/DemaConsulting.VersionMark/Program.cs +++ b/src/DemaConsulting.VersionMark/Program.cs @@ -202,23 +202,13 @@ private static void RunLint(Context context) context.WriteLine($"Linting '{configFile}'..."); // Load the configuration, which performs all validation in a single pass - var (config, issues) = VersionMarkConfig.Load(configFile); + var result = VersionMarkConfig.Load(configFile); - // Report all discovered issues, routing errors to the error stream - foreach (var issue in issues) - { - if (issue.Severity == LintSeverity.Error) - { - context.WriteError(issue.ToString()); - } - else - { - context.WriteLine(issue.ToString()); - } - } + // Report all discovered issues to the context + result.ReportIssues(context); // Confirm success when no errors were found - if (config != null) + if (result.Config != null) { context.WriteLine($"'{configFile}': No issues found"); } @@ -244,21 +234,11 @@ private static void RunCapture(Context context) context.WriteLine($"Output file: {outputFile}"); // Load and validate configuration, reporting all issues before proceeding - var (config, issues) = VersionMarkConfig.Load(".versionmark.yaml"); - foreach (var issue in issues) - { - if (issue.Severity == LintSeverity.Error) - { - context.WriteError(issue.ToString()); - } - else - { - context.WriteLine(issue.ToString()); - } - } + var loadResult = VersionMarkConfig.Load(".versionmark.yaml"); + loadResult.ReportIssues(context); // Abort capture if the configuration could not be loaded - if (config == null) + if (loadResult.Config == null) { return; } @@ -268,12 +248,12 @@ private static void RunCapture(Context context) // Determine which tools to capture var toolNames = context.ToolNames.Length > 0 ? context.ToolNames - : config.Tools.Keys.ToArray(); + : loadResult.Config.Tools.Keys.ToArray(); context.WriteLine($"Capturing {toolNames.Length} tool(s)..."); // Capture versions - var versionInfo = config.FindVersions(toolNames, context.JobId); + var versionInfo = loadResult.Config.FindVersions(toolNames, context.JobId); // Save to file versionInfo.SaveToFile(outputFile); From 7582e5abe9f565853f65daf1d90a07d73f0b1363 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 15:25:42 +0000 Subject: [PATCH 6/6] Fix: Apply PR review feedback on Load/LintIssue/ReportIssues Agent-Logs-Url: https://github.com/demaconsulting/VersionMark/sessions/6d2cef84-e903-4de0-95c7-745bcb4a9338 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- .../Configuration/LintIssue.cs | 10 +++++----- .../Configuration/VersionMarkConfig.cs | 11 +++++++++-- src/DemaConsulting.VersionMark/Program.cs | 4 ++-- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs b/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs index 62169e7..6a2269a 100644 --- a/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs +++ b/src/DemaConsulting.VersionMark/Configuration/LintIssue.cs @@ -25,7 +25,7 @@ namespace DemaConsulting.VersionMark.Configuration; /// /// Severity level for a lint issue. /// -internal enum LintSeverity +public enum LintSeverity { /// /// Non-fatal advisory message. @@ -46,7 +46,7 @@ internal enum LintSeverity /// One-based column number of the issue. /// Severity of the issue. /// Human-readable description of the issue. -internal sealed record LintIssue( +public sealed record LintIssue( string FilePath, long Line, long Column, @@ -75,7 +75,7 @@ public override string ToString() => /// All validation issues found during loading. Always populated; may contain warnings even /// when is non-. /// -internal sealed record VersionMarkLoadResult( +public sealed record VersionMarkLoadResult( VersionMarkConfig? Config, IReadOnlyList Issues) { @@ -84,7 +84,7 @@ internal sealed record VersionMarkLoadResult( /// stream and warnings to the standard output stream. /// /// The context used to write output. - public void ReportIssues(Context context) + internal void ReportIssues(Context context) { ArgumentNullException.ThrowIfNull(context); @@ -92,7 +92,7 @@ public void ReportIssues(Context context) { if (issue.Severity == LintSeverity.Error) { - context.WriteError($"Error: {issue}"); + context.WriteError(issue.ToString()); } else { diff --git a/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs b/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs index 8f8eade..9f23bf0 100644 --- a/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs +++ b/src/DemaConsulting.VersionMark/Configuration/VersionMarkConfig.cs @@ -174,7 +174,7 @@ internal VersionMarkConfig(Dictionary tools) /// (or when fatal errors prevent loading) and a read-only list of /// objects describing all warnings and errors encountered. /// - internal static VersionMarkLoadResult Load(string filePath) + public static VersionMarkLoadResult Load(string filePath) { var issues = new List(); @@ -254,6 +254,13 @@ internal static VersionMarkLoadResult Load(string filePath) var toolName = toolKeyNode.Value ?? string.Empty; + // Tool names must be non-empty + if (string.IsNullOrWhiteSpace(toolName)) + { + issues.Add(CreateIssue(filePath, toolKeyNode, LintSeverity.Error, "Tool names must not be empty or whitespace")); + continue; + } + // Tool configuration must be a mapping if (toolEntry.Value is not YamlMappingNode toolNode) { @@ -292,7 +299,7 @@ public static VersionMarkConfig ReadFromFile(string filePath) var firstError = result.Issues.FirstOrDefault(i => i.Severity == LintSeverity.Error); if (firstError != null) { - throw new ArgumentException(firstError.Description); + throw new ArgumentException(firstError.ToString()); } return result.Config!; diff --git a/src/DemaConsulting.VersionMark/Program.cs b/src/DemaConsulting.VersionMark/Program.cs index 0b6a5ae..d2a2c91 100644 --- a/src/DemaConsulting.VersionMark/Program.cs +++ b/src/DemaConsulting.VersionMark/Program.cs @@ -207,8 +207,8 @@ private static void RunLint(Context context) // Report all discovered issues to the context result.ReportIssues(context); - // Confirm success when no errors were found - if (result.Config != null) + // Confirm success when no issues were found + if (result.Config != null && result.Issues.Count == 0) { context.WriteLine($"'{configFile}': No issues found"); }