Skip to content

Conversation

@nogic1008
Copy link
Contributor

Description:
Add Problem Matcher for dotnet format command.

Related issue:
No

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@nogic1008 nogic1008 requested a review from a team as a code owner January 13, 2025 14:41
@nogic1008
Copy link
Contributor Author

I overlooked unit tests for csc.json. I'll rewrite it.

Copilot AI review requested due to automatic review settings November 13, 2025 23:00
@nogic1008 nogic1008 force-pushed the feat/problem-matcher-dotnet-format branch from 90a52e1 to e609693 Compare November 13, 2025 23:00
Copy link

Copilot AI left a 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.json problem 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+\\[(.+)\\]$",
Copy link

Copilot AI Nov 13, 2025

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.

Suggested change
"regexp": "^\\s*(.*)\\((\\d+),(\\d+)\\):\\s+(error|warning)\\s+(.+):\\s+(.*)\\s+\\[(.+)\\]$",
"regexp": "^\\s*(.*)\\((\\d+),(\\d+)\\):\\s+(error|warning)\\s+([^:]+):\\s+(.*)\\s+\\[(.+)\\]$",

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +41
for (const key in expected) {
expect(res?.[problemMatcher[key]]).toBe(expected[key]);
}
Copy link

Copilot AI Nov 13, 2025

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.

Copilot uses AI. Check for mistakes.
Comment on lines +66 to +68
for (const key in expected) {
expect(res?.[problemMatcher[key]]).toBe(expected[key]);
}
Copy link

Copilot AI Nov 13, 2025

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant