Conversation
ee28670 to
8cbdc7c
Compare
8cbdc7c to
2c5f9a6
Compare
|
In Java and Kotlin, we also do -1 so the same fix should be applied there. |
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the C linter (cppcheck) could return column 0 for certain syntax errors, which after subtracting 1 would result in a negative column value (-1). This violated Dodona's schema requirements causing internal errors. The fix sets the column to None when the computed value would be negative.
Changes:
- Added a safety check in the C linter to set column to None when column 0 is received from cppcheck
- Added a test case with intentionally malformed C code to verify the fix works correctly
- Created a test fixture file with a missing semicolon to reproduce the column 0 scenario
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tested/languages/c/linter.py | Added check to set column to None when value is negative after 1-based to 0-based conversion |
| tests/test_linters.py | Added new test function to verify linter handles bad column values correctly |
| tests/exercises/echo-function/solution/bad-column-cppcheck.c | Created test fixture with missing semicolon to trigger column 0 from cppcheck |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
niknetniko
left a comment
There was a problem hiding this comment.
Makes sense to me. In a follow-up I'll create some helpers to add some more tests for our linters, hopefully preventing similar issues in the future.
For some bad student code the linter was returning a column 0, which resulted in a negative column in the actual judge output, which is not allowed by Dodona. This created internal errors.
I applied a band-aid fix for this to just set to column to None in this case, since I'd guess that the
- 1actually means that the columns are 1-indexed and 0 means that there isn't really a column to latch on to, but I'm not a 100% sure of that.