fix: improve cxx detector scoping#415
Conversation
peteromallet
left a comment
There was a problem hiding this comment.
Review
Three focused improvements to the C++ plugin — all addressing real problems. The implementation is sound.
Looks good
- Security scoping: filtering findings to the scoped first-party file set is the right approach. Empty
entry["file"]resolves to CWD which safely gets filtered out. - CMake coverage mapping: the
_CMAKE_SOURCE_SPEC_REregex handles common patterns well, and the test-tree ancestor walk is correctly scoped. - Disabling unused-imports for C++ — correct call,
#includesemantics don't map to the generic tree-sitter unused-import phase. _extract_import_namerefactor: the new ordering (strip path separators → strip extensions → split namespace separators) correctly handles cases likevendor/json.hpp.
Issues to fix
-
Duplicate test —
test_detect_cxx_security_ignores_findings_outside_scanned_files(~line 93 of test_security.py) andtest_detect_cxx_security_ignores_findings_outside_scoped_files(~line 613) are nearly identical. Remove one. -
Missing newline at EOF in
test_init.pyandtest_security.py— will likely fail linting.
Minor suggestions
-
Extra blank lines added between functions in
test_coverage.pyandmapping_imports.pycreate noise — the codebase typically uses standard PEP 8 spacing. -
_discover_additional_test_mapping_filesis exported inmapping_imports.__all__with a leading underscore — slightly unusual for a public export. Since it's called cross-module, consider dropping the underscore. -
No test for
_parse_cmake_source_specswithtarget_sources()or CMake comments — would be good to add.
Solid PR overall. Fix the duplicate test and EOF issues and it's good to merge.
|
Merged onto the 0.9.9 branch (7fe01b42) with minor cleanup — removed a duplicate test and added missing EOF newlines. Thanks @Dragoy! |
- Filter C/C++ security findings to scoped first-party file set - CMake-based test coverage mapping via add_executable/add_library/target_sources - Disable unsound generic unused-import phase for C++ - Fix _extract_import_name for C++ header extensions (.h, .hh, .hpp) - Remove duplicate test, add missing EOF newlines Co-Authored-By: Dragoy <Dragoy@users.noreply.github.com>
Summary
Test Plan