Add problem matchers for Prettier --check output #3866
Conversation
Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com>
|
@copilot we need tests for this |
Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com>
Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com>
Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com>
Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds problem matcher support for Prettier's --check output, enabling VS Code tasks to display formatting issues in the Problems panel. The implementation is based on Rayat Rahman's MIT-licensed work, with improvements to regex patterns for better specificity.
Key changes:
- Added 4 problem matchers to handle error and warning output in both regular and watch modes
- Created comprehensive test coverage for pattern matching behavior
- Updated documentation with usage examples for tasks and watch mode
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Defines 4 problem matchers ($prettier, $prettier-watch, $prettier-warn, $prettier-warn-watch) with regex patterns to parse Prettier CLI output |
| src/test/suite/problemMatcher.test.ts | Adds unit tests verifying problem matcher registration and regex pattern matching |
| README.md | Documents the problem matchers with usage examples for regular and watch mode tasks |
| THIRDPARTY.md | Adds MIT license attribution for Rayat Rahman's original problem matcher code |
Comments suppressed due to low confidence (1)
src/test/suite/problemMatcher.test.ts:216
- The test is attempting to access
testCase.inputon line 215, butshouldNotMatchis an array of strings (line 208-212), not objects with aninputproperty. This will result inmatchbeingundefined.match(pattern), which will throw a TypeError at runtime.
Fix:
shouldNotMatch.forEach((testCase) => {
const match = testCase.match(pattern); // testCase is already a string
assert.ok(!match, `Should NOT match: ${testCase}`);
});|
|
||
| /** | ||
| * Helper to get problem matchers from extension | ||
| */ | ||
| function getProblemMatchers() { | ||
| const extension = vscode.extensions.getExtension(EXTENSION_ID); | ||
| assert.ok(extension, "Extension should be available"); | ||
| return extension.packageJSON.contributes.problemMatchers; | ||
| } | ||
|
|
There was a problem hiding this comment.
The test file should follow the project convention of using ensureExtensionActivated() from testUtils.js in a before() hook at the top level, rather than manually checking and activating the extension in each test. This is the pattern used consistently across all other test files in the suite (e.g., format.test.ts, config.test.ts, module.test.ts, etc.).
Recommended refactoring:
import { ensureExtensionActivated } from "./testUtils.js";
describe("Problem Matchers", () => {
before(async () => {
await ensureExtensionActivated();
});
it("should have prettier problem matcher registered", () => {
const extension = vscode.extensions.getExtension("prettier.prettier-vscode");
assert.ok(extension, "Extension should be available");
// ... rest of test
});This ensures consistent test setup and avoids redundant activation logic in every test.
| ); | ||
| assert.ok( | ||
| endsPattern.test("All matched files use Prettier code style!"), | ||
| "Should match end pattern for success", | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The test cases for file path matching should include edge cases to ensure the regex pattern handles various file path formats correctly:
- File paths with directories containing dots (e.g.,
[warn] src/v2.0/file.js) - File names with multiple dots (e.g.,
[warn] component.test.d.ts) - File paths with underscores or hyphens (e.g.,
[warn] my-component_v2.js)
These edge cases would help validate that the regex pattern in package.json (line 507) correctly matches all valid file paths while properly excluding summary messages.
Added comprehensive tests in commit 4bc8ecd. The test suite includes:
All tests use helper functions to reduce duplication and improve maintainability. |
|
@copilot rwview the test failures and keep working |
…date CHANGELOG Co-authored-by: ntotten <282782+ntotten@users.noreply.github.com>
Fixed the test issues in commit 5a7eaf6:
All tests now follow project conventions and include comprehensive edge case coverage. |
Description
This PR adds problem matcher definitions to the Prettier VS Code extension, enabling VS Code tasks running
prettier --checkto display formatting issues directly in the Problems panel. The implementation is based on Rayat Rahman's (@riotrah) MIT-licensed work, with improvements to regex patterns for better specificity and robustness.Problem Matchers Added:
$prettier- Matches Prettier syntax errors with line and column information$prettier-watch- Same as$prettierbut for watch mode tasks (e.g., withonchange)$prettier-warn- Matches file-level warnings for unformatted files$prettier-warn-watch- Same as$prettier-warnbut for watch mode tasksPattern Improvements:
[^:]+to match filenames up to the first colon[^\s]+\.[a-zA-Z0-9.-]+to match files with complex extensions (.test.js,.d.ts,.spec.tsx)Related Issue
Fixes #2554
Type of Change
Checklist
npm run lintpasses)npm run prettierto format my codenpm test)Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.