ZedThree / clang-tidy-review

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

review.py crash on invalid Qt path #33

Closed vadi2 closed 1 year ago

vadi2 commented 2 years ago

My workflow installs Qt to Qt5_DIR (standard variable) which is /home/runner/work/Mudlet/Mudlet/Qt/5.14.1/gcc_64. That works fine for compiling purposes, but the clang-tidy-review action then tries to access an invalid path:

  File "/review.py", line 811, in <module>
    main(
  File "/review.py", line 649, in main
    review = make_review(
  File "/review.py", line 446, in make_review
    comment_body, end_line = make_comment_from_diagnostic(
  File "/review.py", line 422, in make_comment_from_diagnostic
    code_blocks += format_notes(notes, offset_lookup)
  File "/review.py", line 374, in format_notes
    line_num = find_line_number_from_offset(
  File "/review.py", line 180, in find_line_number_from_offset
    offset_lookup.update(make_file_offset_lookup([name]))
  File "/review.py", line 157, in make_file_offset_lookup
    with open(filename, "r") as file:
FileNotFoundError: [Errno 2] No such file or directory: '/Qt/5.14.1/gcc_64/include/QtCore/qglobal.h'

https://github.com/Mudlet/Mudlet/runs/5349151395?check_suite_focus=true

ZedThree commented 2 years ago

Looks like the emitted diagnostic is:

{
'DiagnosticName': 'readability-function-cognitive-complexity', 
'DiagnosticMessage': {
  'Message': "function 'textAroundOffset' has cognitive complexity of 31 (threshold 25)", 
  'FilePath': '/github/workspace/src/TAccessibleTextEdit.cpp', 
  'FileOffset': 13337, 
  'Replacements': []}, 
  'Notes': [
    {'Message': '+1', 
     'FilePath': '/github/workspace/src/TAccessibleTextEdit.cpp', 
     'FileOffset': 13713, 'Replacements': []}, 
    {'Message': "expanded from macro 'Q_ASSERT_X'", 
     'FilePath': '../../Qt/5.14.1/gcc_64/include/QtCore/qglobal.h', 
     'FileOffset': 30235, 'Replacements': []}, ...

You can see the second message has a relative path, which clang-tidy-review is interpreting as being relative to where it's being run, but it should actually be relative to the top-level FilePath for this diagnostic.

That shouldn't be too difficult to fix, I just need to find time to do so!

vadi2 commented 2 years ago

This is still an issue with 0.9.0 unfortunately, if anyone could take this on =)

ZedThree commented 1 year ago

Do you know if this is still a problem with the latest version?

vadi2 commented 1 year ago

I'm guessing it has to do with the fact that machine folders aren't accessible to the Docker image. Can that limitation be worked around, ie, can the machine folders be exposed to Docker?

ZedThree commented 1 year ago

Ah, yes, of course, that will be it. Unfortunately, it's a hard limitation of Docker that only subdirectories of the Dockerfile are able to be imported or copied into the image. You can map directories into a container when you run it, but unless things have changed recently, there's no way to supply those arguments.

So two possible workarounds: