Open
Conversation
chvp
approved these changes
Feb 16, 2026
tests/partial_output.json
Outdated
Comment on lines
+88
to
+95
| "rows": { | ||
| "type": "integer", | ||
| "minimum": 1 | ||
| }, | ||
| "columns": { | ||
| "type": "integer", | ||
| "minimum": 1 | ||
| } |
Member
There was a problem hiding this comment.
Is this something we should apply in the Dodona codebase as well?
Member
Author
There was a problem hiding this comment.
- For rows, not sure. For manual annotations this is enforced, but the client code does seem to handle it, so unsure if there are valid use-cases.
- For columns this is actually a good point, Dodona does support it, so I changed TESTed to emit those as well (instead of ignoring the column span information in that case), forgot we supported this.
- The use case is e.g. end-of-line annotations
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is a follow-up to #610, unifying the handling of line/column numbers across all linters, adding comprehensive tests, and fixing several uncovered bugs and off-by-one errors.
Changes:
- Introduced centralized
get_linter_positionandannotation_from_positionhelper functions for consistent position conversion across all linters - Added comprehensive unit tests for linter position conversion logic covering edge cases like zero-width annotations, exclusive vs inclusive ranges, and 0-based vs 1-based indexing
- Refactored all language-specific linters (Bash/ShellCheck, C/C++/cppcheck, Haskell/HLint, Java/Checkstyle, JavaScript/ESLint, Kotlin/ktlint, Python/Pylint, TypeScript/ESLint) to use the new helper functions
- Added detailed integration tests for each linter validating specific error messages, positions, and configurations
- Updated annotation schema to allow columns=0 for zero-width annotations
- Added TypeScript-specific ESLint rule URL handling
- Removed language parameter from ShellCheck (always uses bash)
- Added pytest warning suppression for Testcase class
Reviewed changes
Copilot reviewed 59 out of 59 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tested/judge/linter.py | Added core position conversion helpers: get_linter_position and annotation_from_position, plus centralized annotation sorting |
| tested/languages/bash/linter.py | Refactored to use new helpers, removed unused language parameter |
| tested/languages/c/linter.py | Refactored to use new helpers, improved location element handling |
| tested/languages/haskell/linter.py | Refactored to use new helpers for HLint position conversion |
| tested/languages/java/linter.py | Refactored to use new helpers for Checkstyle position conversion |
| tested/languages/javascript/linter.py | Refactored to use new helpers for ESLint position conversion |
| tested/languages/kotlin/linter.py | Refactored to use new helpers for ktlint position conversion |
| tested/languages/python/linter.py | Refactored to use new helpers for Pylint position conversion |
| tested/languages/typescript/linter.py | Refactored to use new helpers, added TypeScript-specific ESLint rule URL handling |
| tests/test_utils_linter.py | Comprehensive unit tests for get_linter_position covering all edge cases |
| tests/test_linters_*.py | Integration tests for each linter validating error detection, positioning, and configurations |
| tests/partial_output.json | Updated schema to allow columns=0 and rows >= 1 |
| tests/exercises/linter/solution/* | Test fixture files for each linter |
| tests/exercises/linter/evaluation/* | Configuration files for linter tests |
| tested/testsuite.py | Added pytest warning suppression for Testcase class |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Follow-up to #610: