Add analyzer diagnostics for With() method (MTLOG009-MTLOG013) #55
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Implement MTLOG009-013 diagnostics for With() method validation - Add cross-call duplicate detection (MTLOG011) with data flow analysis - Support odd arguments, non-string keys, empty keys, and reserved properties - Include suggested fixes for all With() diagnostics - Update GoLand plugin to handle both camelCase and snake_case JSON fields - Add test coverage for all With() validation scenarios
- Add 5 new diagnostic codes for With() method validation: - MTLOG009: odd argument count detection - MTLOG010: non-string key validation - MTLOG011: cross-call duplicate property detection - MTLOG012: reserved property name warnings - MTLOG013: empty key string detection - Enhance quick fix parser to support pos/end string format - Add comprehensive test coverage for all new diagnostics - Update help system with explanations for new codes - Improve error handling with validation and user feedback - Cache namespace lookups for better performance
…OG013) - Add diagnostic descriptions for new With() method checks - Update suppression enum to include MTLOG009-MTLOG013 - Document new diagnostics in CHANGELOG.md
- Add cross-call duplicate example to analyzer docs - Improve toSnakeCase() documentation with detailed transition cases - Add tests and benchmarks for With() with up to 200 arguments - Verify optimal performance for ≤64 fields, graceful map fallback for larger sets
- Fix circular dependency between mtlog and analyzer modules by moving reanalyze_all() to analyzer - Fix incorrect test expectation in diagnostics_spec (should expect false when fix fails) - Fix async callback scope issue in analyzer_spec by capturing done callback - Update path resolution in minimal_init.lua to use absolute paths - Make apply_suggested_fix() properly return false when edits fail All tests now pass successfully.
…orld testing - Remove concurrent check guards in analyzer for simpler synchronous operation - Add high-resolution timing using vim.loop.now() for sub-second cache accuracy - Fix cache invalidation to properly update LRU order and stats - Enhance diagnostic edit range validation with proper bounds checking - Replace test mocks with real mtlog-analyzer integration and file operations - Add comprehensive test environment setup and verification scripts - Improve cache TTL handling and cleanup with configurable intervals - Add With() method diagnostics testing for MTLOG009-MTLOG013 support
Use vim.fn.resolve() for path comparison to handle /var -> /private/var symlink resolution on macOS in workspace_spec.lua test.
Add fix.message to format_item fallback chain to display actual analyzer fix descriptions instead of generic "Fix" labels when multiple fixes available.
willibrandon
added
enhancement
Redirect stderr and check for E121 errors in health check validation to prevent "Undefined variable: s:output" when running checkhealth.
Remove fallback to legacy require('health') which causes E121: Undefined variable: s:output in CI environments.
Remove v0.8.0 and v0.9.0 from CI matrix as vim.health API is only fully functional in v0.10.0+.
- Fix health check validation on macOS where checkhealth outputs to buffer instead of stdout - Write buffer content to file to capture health check results on all platforms - Initialize plugin in minimal_init.lua to eliminate health check warning - Update CI to only test Neovim v0.10.0+ where vim.health API is available
…egration test - Remove buffer write attempt that failed due to scratch buffer restrictions - Use Lua IO to create real file on disk for mtlog-analyzer to analyze - Ensures test works in both local and CI environments
- Use test_helpers.create_test_go_file() to place file in Go module directory - Fixes Ubuntu CI failure where analyzer couldn't resolve mtlog import without go.mod - Aligns with all other passing tests that use the same approach
- Add suppression checks for MTLOG009-013 in with_checks.go - Implement isDiagnosticSuppressed helper method - Add tests for flag and env var suppression - Update README with suppression documentation - Fix GoLand plugin: timeouts, JSON parsing, path validation - Optimize test infrastructure setup
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive static analysis diagnostics for the With() method in mtlog, adding five new diagnostic codes (MTLOG009-MTLOG013) to help developers catch common mistakes when using structured field logging. The implementation includes analyzer integration, quick fixes, and comprehensive editor support.
- Adds five new diagnostic codes for comprehensive With() method validation
- Implements cross-call analysis to detect duplicate keys across method chains
- Provides automatic quick fixes for all detected issues including odd argument counts and property name corrections
Reviewed Changes
Copilot reviewed 72 out of 72 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| with_longargs_test.go | Performance tests for With() method handling large argument lists and edge cases |
| vscode-extension/mtlog-analyzer/src/types.ts | Updates TypeScript definitions with new diagnostic codes MTLOG009-MTLOG013 |
| vscode-extension/mtlog-analyzer/package.json | Extends VS Code configuration to support new diagnostic suppression options |
| vscode-extension/mtlog-analyzer/CHANGELOG.md | Documents new With() diagnostic features for VS Code extension |
| neovim-plugin/tests/test_helpers.lua | Comprehensive test infrastructure for real file operations and analyzer integration |
| neovim-plugin/tests/spec/*.lua (multiple files) | Extensive test coverage for all new diagnostic features, workspace management, and plugin integrations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
- Add clarifying comment for single event emission validation in with_longargs_test.go - Make mtlog module path configurable via MTLOG_MODULE_PATH env var in test_helpers.lua - Replace brittle string assertions with flexible pattern matching in with_diagnostics_spec.lua - Add get_all_diagnostic_codes() function to dynamically generate diagnostic code list
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR implements comprehensive static analysis diagnostics for the
With()method introduced in PR #53. It adds five new diagnostic codes (MTLOG009-MTLOG013) to help developers catch common mistakes when using variadic key-value pairs for structured field logging.The implementation includes:
Each diagnostic includes appropriate quick fixes to resolve the issues automatically.
Type of change
Checklist
go test ./...)golangci-lint run)Additional notes
Key implementation details:
checkWithArgumentsfunction to analyze With() method callsCrossCallAnalyzerfor tracking logger variable assignments and property accumulationBreaking changes:
None. All new diagnostics are additive and respect existing configuration.
Fixes #54