-
Notifications
You must be signed in to change notification settings - Fork 543
feat: Add Problem Matcher for dotnet-format
#583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: Add Problem Matcher for dotnet-format
#583
Conversation
|
I overlooked unit tests for |
90a52e1 to
e609693
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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+\\[(.+)\\]$", |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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]); | ||
| } |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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]); | ||
| } |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: