feat: Add Problem Matcher for dotnet-format#583
feat: Add Problem Matcher for dotnet-format#583nogic1008 wants to merge 7 commits intoactions:mainfrom
dotnet-format#583Conversation
|
I overlooked unit tests for |
90a52e1 to
e609693
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds a problem matcher for the dotnet format command to enable automatic annotation of formatting issues in GitHub Actions workflows.
Key changes:
- Refactored problem matcher registration to support multiple matchers via an array
- Added
dotnet-format.jsonproblem matcher configuration file - Consolidated test files for problem matchers into a unified test suite
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/setup-dotnet.ts |
Refactored to register multiple problem matchers from an array instead of hardcoding a single matcher path |
dist/setup/index.js |
Compiled/bundled version of the source changes |
.github/dotnet-format.json |
New problem matcher configuration for dotnet format output parsing |
__tests__/setup-dotnet.test.ts |
Added test assertions to verify both problem matchers are registered |
__tests__/problem-matchers.json.test.ts |
New unified test file for validating both csc and dotnet-format problem matcher patterns |
__tests__/csc.test.ts |
Removed old standalone test file (tests moved to new consolidated file) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "owner": "dotnet-format", | ||
| "pattern": [ | ||
| { | ||
| "regexp": "^\\s*(.*)\\((\\d+),(\\d+)\\):\\s+(error|warning)\\s+(.+):\\s+(.*)\\s+\\[(.+)\\]$", |
There was a problem hiding this comment.
The regex pattern for capturing the 'code' field (capture group 5) uses .+ which is too greedy. Based on the test case, the code should match WHITESPACE but the pattern will incorrectly capture everything up to the last colon before the message. Consider using ([A-Z_]+) or ([^:]+) to match the diagnostic code more precisely.
| "regexp": "^\\s*(.*)\\((\\d+),(\\d+)\\):\\s+(error|warning)\\s+(.+):\\s+(.*)\\s+\\[(.+)\\]$", | |
| "regexp": "^\\s*(.*)\\((\\d+),(\\d+)\\):\\s+(error|warning)\\s+([^:]+):\\s+(.*)\\s+\\[(.+)\\]$", |
| for (const key in expected) { | ||
| expect(res?.[problemMatcher[key]]).toBe(expected[key]); | ||
| } |
There was a problem hiding this comment.
The test uses for...in loop without checking hasOwnProperty, which could iterate over inherited properties. Consider using Object.keys(expected).forEach(key => ...) or add a hasOwnProperty check for safer iteration.
| for (const key in expected) { | ||
| expect(res?.[problemMatcher[key]]).toBe(expected[key]); | ||
| } |
There was a problem hiding this comment.
The test uses for...in loop without checking hasOwnProperty, which could iterate over inherited properties. Consider using Object.keys(expected).forEach(key => ...) or add a hasOwnProperty check for safer iteration.
Description:
Add Problem Matcher for
dotnet formatcommand.Related issue:
No
Check list: