Invalid suggestion created

[vadi2]

Check out https://github.com/Mudlet/Mudlet/pull/5981#discussion_r814601177 - the action generated a syntactically invalid C++ suggestion by the looks of it?

[ZedThree]

That's a very odd suggestion! My suspicion is that it's due to that header file requiring Qt's moc. I'm not sure how you should best handle that -- maybe either run moc over the headers first? Or just exclude them from clang-tidy-review?

[vadi2]

moc runs over pretty much the entire codebase as far as I know, I don't think there is much I can do about it.

[ZedThree]

I thought it only applied to the headers? At any rate, I'm afraid I think this is an issue between clang-tidy and Qt -- you would have the same issue running clang-tidy locally.

I think you'll need to either generate the "real" C++ headers first, or tell clang-tidy to ignore all the header files.

[vadi2]

clang-tidy does not generate invalid C++ like the action does -

vadi@penguin:~/Programs/Mudlet-1/build$ clang-tidy --checks=readability-redundant-access-specifiers -p compile_commands.json ../src/dlgMapper.h 
67354 warnings generated.
/home/vadi/Programs/Mudlet-1/build/../src/dlgMapper.h:41:7: warning: class 'dlgMapper' defines a copy constructor and a copy assignment operator but does not define a destructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class dlgMapper : public QWidget, public Ui::mapper
      ^
/home/vadi/Programs/Mudlet-1/build/../src/dlgMapper.h:53:10: warning: method 'getDefaultAreaShown' can be made const [readability-make-member-function-const]
    bool getDefaultAreaShown() { return mShowDefaultArea; }
         ^
                               const
/home/vadi/Programs/Mudlet-1/build/../src/dlgMapper.h:57:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
public slots:
^~~~~~~~~~~~~
/home/vadi/Programs/Mudlet-1/build/../src/dlgMapper.h:45:1: note: previously declared here
public:
^
Suppressed 67351 warnings (67351 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

vadi@penguin:~/Programs/Mudlet-1/build$ clang-tidy --checks=readability-redundant-access-specifiers -p compile_commands.json ../src/dlgMapper.h  --fix
67354 warnings generated.
/home/vadi/Programs/Mudlet-1/build/../src/dlgMapper.h:41:7: warning: class 'dlgMapper' defines a copy constructor and a copy assignment operator but does not define a destructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class dlgMapper : public QWidget, public Ui::mapper
      ^
/home/vadi/Programs/Mudlet-1/build/../src/dlgMapper.h:53:10: warning: method 'getDefaultAreaShown' can be made const [readability-make-member-function-const]
    bool getDefaultAreaShown() { return mShowDefaultArea; }
         ^
                               const
/home/vadi/Programs/Mudlet-1/build/../src/dlgMapper.h:53:31: note: FIX-IT applied suggested code changes
    bool getDefaultAreaShown() { return mShowDefaultArea; }
                              ^
/home/vadi/Programs/Mudlet-1/build/../src/dlgMapper.h:57:1: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers]
public slots:
^~~~~~~~~~~~~
/home/vadi/Programs/Mudlet-1/build/../src/dlgMapper.h:57:1: note: FIX-IT applied suggested code changes
/home/vadi/Programs/Mudlet-1/build/../src/dlgMapper.h:45:1: note: previously declared here
public:
^
clang-tidy applied 2 of 2 suggested fixes.
Suppressed 67351 warnings (67351 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

vadi@penguin:~/Programs/Mudlet-1/build$ git diff
diff --git a/3rdparty/edbee-lib b/3rdparty/edbee-lib
index 64e32081..8fbcdf95 160000
--- a/3rdparty/edbee-lib
+++ b/3rdparty/edbee-lib
@@ -1 +1 @@
-Subproject commit 64e32081588797c88eb614d7f22673df34a675fd
+Subproject commit 8fbcdf9527322c00dc8e8e42ad4655742e6b6668
diff --git a/3rdparty/qtkeychain b/3rdparty/qtkeychain
--- a/3rdparty/qtkeychain
+++ b/3rdparty/qtkeychain
@@ -1 +1 @@
-Subproject commit 5b62b11442d927d385aa4f26b593de5f8742f3ac
+Subproject commit 5b62b11442d927d385aa4f26b593de5f8742f3ac-dirty
diff --git a/3rdparty/vcpkg b/3rdparty/vcpkg
index 5cf60186..772fe6cb 160000
--- a/3rdparty/vcpkg
+++ b/3rdparty/vcpkg
@@ -1 +1 @@
-Subproject commit 5cf60186a241e84e8232641ee973395d4fde90e1
+Subproject commit 772fe6cbce530cb3a5f0fee67b57e9861676e5d0
diff --git a/src/dlgMapper.h b/src/dlgMapper.h
index 7e069fdb..ebb2e27a 100644
--- a/src/dlgMapper.h
+++ b/src/dlgMapper.h
@@ -50,11 +50,11 @@ public:
 #endif
     void updateAreaComboBox();
     void setDefaultAreaShown(bool);
-    bool getDefaultAreaShown() { return mShowDefaultArea; }
+    bool getDefaultAreaShown() const { return mShowDefaultArea; }
     void resetAreaComboBoxToPlayerRoomArea();
     bool isFloatAndDockable() const;

-public slots:
+
     void slot_toggleRoundRooms(const bool);
     void slot_toggleShowRoomIDs(int s);
     void slot_toggleShowRoomNames(int s);
vadi@penguin:~/Programs/Mudlet-1/build$ git log --oneline
00bba69d (HEAD -> label-create-improve, Delwing/label-create-improve) Merge remote-tracking branch 'upstream/development' into label-create-improve

[ZedThree]

Ah, ok! Interesting that those are also different warnings from clang-tidy-review

Could you re-run it with --export-fixes=clang-tidy-fixes.yaml and post the resulting yaml file?

[vadi2]

I think the warnings were different because I ran just one check only - re-running it again with export-fixes and the same checks string, here is the resulting file.

clang-tidy-fixes.txt

[ZedThree]

It looks like maybe clang-tidy-review is reading the fixes file wrong, maybe an off-by-one error somehow. I need to find some time to reproduce your setup and get the actual fixes file locally so I can see what's going on.

I think it might be a good idea for clang-tidy-review to automatically upload the fixes file as a build artefact, that might make debugging a bit easier

[vadi2]

That would be a good debugging opt-in feature!

[oleg-derevenetz]

Hi @ZedThree most likely the reason for such an invalid suggestions is this:

https://github.com/ihhub/fheroes2/pull/7991

[ZedThree]

Ah, amazing, thanks @oleg-derevenetz! It looks like it needs a combo of checking out a different ref and then also getting the PR base as well. Hopefully we could at least automate that second part