search

ZedThree / clang-tidy-review

Create a pull request review based on clang-tidy warnings
MIT License
88 stars 44 forks source link

Convert all replacement FilePaths to absolute paths in `collate_replacement_sets()` #66

Closed zhongfu closed 1 year ago

zhongfu commented 1 year ago

The clang-tidy output can sometimes output sorta-relative replacement FilePaths (with .. and . as path components), which leads to a KeyError:

Traceback (most recent call last):
  File "/action/review.py", line 205, in <module>
    main(
  File "/action/review.py", line 46, in main
    review = create_review(
  File "/action/post/clang_tidy_review/__init__.py", line 673, in create_review
    review = create_review_file(
  File "/action/post/clang_tidy_review/__init__.py", line 568, in create_review_file
    comment_body, end_line = make_comment_from_diagnostic(
  File "/action/post/clang_tidy_review/__init__.py", line 532, in make_comment_from_diagnostic
    code_blocks, end_line = format_diff_line(
  File "/action/post/clang_tidy_review/__init__.py", line 428, in format_diff_line
    old_line, new_line = replace_one_line(
  File "/action/post/clang_tidy_review/__init__.py", line 367, in replace_one_line
    line_offset = offset_lookup[filename][line_num]
KeyError: '/github/workspace/foo/bar/../baz.h'

This happens because the path gets turned into an absolute path with os.path.abspath in make_file_offset_lookup() before being used as a key, but in replace_one_line, the file path is used verbatim as a key.

To fix this, we convert all replacement FilePaths to absolute paths in collate_replacement_sets() before doing anything else with them.