Semi unrelated changes from and for PR #5350

 - mostly IWYU fixups
 - missing includes
 - required changes to compile with MSVC
 - Trailing return type declarations for neighbor functions
   - higher readability, consistency
 - PageViewFindObjectHelper.h was not a self containing header

.clang-tidy

@@ -3,44 +3,45 @@ modernize-*,\
 misc*,\
 performance*,\
 bug*,\
--cppcoreguidelines-pro-bounds-pointer-arithmetic,\
--cppcoreguidelines-pro-type-cstyle-cast,\
--readability-identifier-naming,\
--misc-unused-parameters,\
--cppcoreguidelines-pro-type-union-access,\
+-*-magic-numbers,\
+-*-non-private-member-variables-in-classes,\
+-cert-err58-cpp,\
+-cert-msc32-c,\
+-cert-msc51-cpp,\
 -clang-analyzer-alpha*,\
--google-build-using-namespace,\
 -clang-analyzer-optin.osx*,\
+-clang-analyzer-optin.cplusplus.VirtualCall,\
 -clang-analyzer-osx*,\
--llvm-include-order,\
--llvm-header-guard,\
--readability-named-parameter,\
--readability-implicit-bool-cast,\
--readability-implicit-bool-conversion,\
+-cppcoreguidelines-owning-memory,\
+-cppcoreguidelines-pro-bounds-pointer-arithmetic,\
+-cppcoreguidelines-pro-type-cstyle-cast,\
 -cppcoreguidelines-pro-type-reinterpret-cast,\
+-cppcoreguidelines-pro-type-union-access,\
 -cppcoreguidelines-pro-type-vararg,\
--misc-move-const-arg,\
--google-runtime-references,\
--cert-err58-cpp,\
 -fuchsia-overloaded-operator,\
 -fuchsia-default-arguments,\
--hicpp-vararg,\
--clang-analyzer-optin.cplusplus.VirtualCall,\
--cppcoreguidelines-owning-memory,\
--*-magic-numbers,\
--*-non-private-member-variables-in-classes,\
 -fuchsia-statically-constructed-objects,\
--readability-isolate-declaration,\
 -fuchsia-multiple-inheritance,\
 -fuchsia-default-arguments-calls,\
 -fuchsia-trailing-return,\
--portability-simd-intrinsics,\
--modernize-use-nodiscard,\
+-google-build-using-namespace,\
+-google-runtime-references,\
+-hicpp-vararg,\
 -hicpp-multiway-paths-covered,\
 -hicpp-deprecated-headers,\
 -hicpp-uppercase-literal-suffix,\
--cert-msc32-c,\
--cert-msc51-cpp"
+-llvm-include-order,\
+-llvm-header-guard,\
+-misc-unused-parameters,\
+-misc-use-anonymous-namespace,\
+-misc-move-const-arg,\
+-modernize-use-nodiscard,\
+-portability-simd-intrinsics,\
+-readability-identifier-naming,\
+-readability-named-parameter,\
+-readability-implicit-bool-cast,\
+-readability-implicit-bool-conversion,\
+-readability-isolate-declaration"
 #WarningsAsErrors: '*'
 HeaderFilterRegex: '*.(h|hpp|hxx)'
 AnalyzeTemporaryDtors: false

src/core/control/ToolBase.h

@@ -132,7 +132,6 @@ class ToolBase {
      * Alpha for fill
      */
     int fillAlpha = 128;
-    ;
 
     /**
      * Style of the line drawing

src/core/control/layer/LayerController.cpp

@@ -236,7 +236,7 @@ void LayerController::mergeCurrentLayerDown() {
     Layer* layerBelow = (*page->getLayers())[layerBelowIndex];
 
     UndoActionPtr undo_redo_action =
-            std::make_unique<MergeLayerDownUndoAction>(this, page, currentLayer, layerBelow, layerID - 1, pageID);
+            std::make_unique<MergeLayerDownUndoAction>(this, page, currentLayer, layerID - 1, layerBelow, pageID);
     undo_redo_action->redo(this->control);
 
     control->getUndoRedoHandler()->addUndoAction(std::move(undo_redo_action));

src/core/control/tools/EditSelection.cpp

@@ -203,13 +203,6 @@ std::unique_ptr<EditSelection> addElementsFromActiveLayer(Control* ctrl, EditSel
 EditSelection::EditSelection(Control* ctrl, InsertionOrder elts, const PageRef& page, Layer* layer, XojPageView* view,
                              const Range& bounds, const Range& snappingBounds):
         snappedBounds(snappingBounds),
-        mouseDownType(CURSOR_SELECTION_NONE),
-        relMousePosX(0),
-        relMousePosY(0),
-        relMousePosRotX(0),
-        relMousePosRotY(0),
-        preserveAspectRatio(false),
-        supportMirroring(true),
         btnWidth(std::max(10, ctrl->getSettings()->getDisplayDpi() / 8)),
         sourcePage(page),
         sourceLayer(layer),
@@ -230,9 +223,6 @@ EditSelection::EditSelection(Control* ctrl, InsertionOrder elts, const PageRef&
     this->view->getXournal()->getCursor()->setRotationAngle(0);
     this->view->getXournal()->getCursor()->setMirror(false);
 
-    this->preserveAspectRatio = false;
-    this->supportMirroring = true;
-    this->supportRotation = true;
     for (auto&& e: contents->getElements()) {
         this->preserveAspectRatio = this->preserveAspectRatio || e->rescaleOnlyAspectRatio();
         this->supportMirroring = this->supportMirroring && e->rescaleWithMirror();
@@ -242,19 +232,7 @@ EditSelection::EditSelection(Control* ctrl, InsertionOrder elts, const PageRef&
 
 
 EditSelection::EditSelection(Control* ctrl, const PageRef& page, Layer* layer, XojPageView* view):
-        x(0),
-        y(0),
-        rotation(0),
-        width(0),
-        height(0),
         snappedBounds(Rectangle<double>{}),
-        mouseDownType(CURSOR_SELECTION_NONE),
-        relMousePosX(0),
-        relMousePosY(0),
-        relMousePosRotX(0),
-        relMousePosRotY(0),
-        preserveAspectRatio(false),
-        supportMirroring(true),
 
         btnWidth(std::max(10, ctrl->getSettings()->getDisplayDpi() / 8)),
         sourcePage(page),
@@ -356,7 +334,9 @@ auto EditSelection::getSnappedBounds() const -> Rectangle<double> { return Recta
 /**
  * get the original bounding rectangle in document coordinates
  */
-auto EditSelection::getOriginalBounds() const -> Rectangle<double> { return Rectangle<double>{this->contents->getOriginalBounds()}; }
+auto EditSelection::getOriginalBounds() const -> Rectangle<double> {
+    return Rectangle<double>{this->contents->getOriginalBounds()};
+}
 
 /**
  * Get the rotation angle of the selection
@@ -1174,7 +1154,9 @@ void EditSelection::serialize(ObjectOutputStream& out) const {
     out.endObject();
 
     out.writeInt(static_cast<int>(this->getElements().size()));
-    for (Element* e: this->getElements()) { e->serialize(out); }
+    for (Element* e: this->getElements()) {
+        e->serialize(out);
+    }
 }
 
 void EditSelection::readSerialized(ObjectInputStream& in) {

src/core/control/tools/EditSelection.h

@@ -19,15 +19,15 @@
 #include <cairo.h>  // for cairo_t, cairo_matrix_t
 #include <glib.h>   // for GSource
 
-#include "control/ToolEnums.h"              // for ToolSize
-#include "model/Element.h"                  // for Element, Element::Index
-#include "model/ElementContainer.h"         // for ElementContainer
+#include "control/ToolEnums.h"               // for ToolSize
+#include "model/Element.h"                   // for Element, Element::Index
+#include "model/ElementContainer.h"          // for ElementContainer
 #include "model/ElementInsertionPosition.h"  // for InsertionOrder
-#include "model/PageRef.h"                  // for PageRef
-#include "undo/UndoAction.h"                // for UndoAction (ptr only)
-#include "util/Color.h"                     // for Color
-#include "util/Rectangle.h"                 // for Rectangle
-#include "util/serializing/Serializable.h"  // for Serializable
+#include "model/PageRef.h"                   // for PageRef
+#include "undo/UndoAction.h"                 // for UndoAction (ptr only)
+#include "util/Color.h"                      // for Color
+#include "util/Rectangle.h"                  // for Rectangle
+#include "util/serializing/Serializable.h"   // for Serializable
 
 #include "CursorSelectionType.h"     // for CursorSelectionType, CURS...
 #include "SnapToGridInputHandler.h"  // for SnapToGridInputHandler
@@ -46,27 +46,28 @@ class Document;
 class EditSelection;
 
 namespace SelectionFactory {
-std::unique_ptr<EditSelection> createFromFloatingElement(Control* ctrl, const PageRef& page, Layer* layer,
-                                                         XojPageView* view, Element* e);
-std::pair<std::unique_ptr<EditSelection>, Range> createFromFloatingElements(Control* ctrl, const PageRef& page,
-                                                                            Layer* layer, XojPageView* view,
-                                                                            InsertionOrder elts);
-std::unique_ptr<EditSelection> createFromElementOnActiveLayer(Control* ctrl, const PageRef& page, XojPageView* view,
-                                                              Element* e, Element::Index pos = Element::InvalidIndex);
-std::unique_ptr<EditSelection> createFromElementsOnActiveLayer(Control* ctrl, const PageRef& page, XojPageView* view,
-                                                               InsertionOrder elts);
+auto createFromFloatingElement(Control* ctrl, const PageRef& page, Layer* layer, XojPageView* view, Element* e)
+        -> std::unique_ptr<EditSelection>;
+auto createFromFloatingElements(Control* ctrl, const PageRef& page, Layer* layer, XojPageView* view,
+                                InsertionOrder elts)  //
+        -> std::pair<std::unique_ptr<EditSelection>, Range>;
+auto createFromElementOnActiveLayer(Control* ctrl, const PageRef& page, XojPageView* view, Element* e,
+                                    Element::Index pos = Element::InvalidIndex)  //
+        -> std::unique_ptr<EditSelection>;
+auto createFromElementsOnActiveLayer(Control* ctrl, const PageRef& page, XojPageView* view, InsertionOrder elts)
+        -> std::unique_ptr<EditSelection>;
 /**
  * @brief Creates a new instance containing base->getElements() and *e. The content of *base is cleared but *base is not
  * destroyed.
  */
-std::unique_ptr<EditSelection> addElementFromActiveLayer(Control* ctrl, EditSelection* base, Element* e,
-                                                         Element::Index pos);
+auto addElementFromActiveLayer(Control* ctrl, EditSelection* base, Element*, Element::Index pos)
+        -> std::unique_ptr<EditSelection>;
 /**
  * @brief Creates a new instance containing base->getElements() and the content of elts. The content of *base is cleared
  * but *base is not destroyed.
  */
-std::unique_ptr<EditSelection> addElementsFromActiveLayer(Control* ctrl, EditSelection* base,
-                                                          const InsertionOrder& elts);
+auto addElementsFromActiveLayer(Control* ctrl, EditSelection* base, const InsertionOrder& elts)
+        -> std::unique_ptr<EditSelection>;
 };  // namespace SelectionFactory
 
 class EditSelection: public ElementContainer, public Serializable {
@@ -210,12 +211,12 @@ class EditSelection: public ElementContainer, public Serializable {
     /**
      * Returns all containing elements of this selection
      */
-    const std::vector<Element*>& getElements() const override;
+    auto getElements() const -> std::vector<Element*> const& override;
 
     /**
      * Returns the insert order of this selection
      */
-    const InsertionOrder& getInsertOrder() const;
+    auto getInsertOrder() const -> InsertionOrder const&;
 
     enum class OrderChange {
         BringToFront,
@@ -227,7 +228,7 @@ class EditSelection: public ElementContainer, public Serializable {
     /**
      * Change the insert order of this selection.
      */
-    UndoActionPtr rearrangeInsertOrder(const OrderChange change);
+    auto rearrangeInsertOrder(const OrderChange change) -> UndoActionPtr;
 
     /**
      * Finish the current movement

src/core/control/tools/EditSelectionContents.h

@@ -11,6 +11,8 @@
 
 #pragma once
 
+#include <memory>   // for unique_ptr
+#include <utility>  // for pair
 #include <vector>   // for vector
 
 #include <cairo.h>  // for cairo_surface_t, cairo_t
@@ -91,12 +93,12 @@ class EditSelectionContents: public ElementContainer, public Serializable {
     /**
      * Returns all containing elements of this selection
      */
-    const std::vector<Element*>& getElements() const override;
+    auto getElements() const -> std::vector<Element*> const& override;
 
     /**
      * Returns the insert order of this selection
      */
-    const InsertionOrder& getInsertionOrder() const;
+    auto getInsertionOrder() const -> const InsertionOrder&;
 
     /** replaces all elements by a new vector of elements
      * @param newElements: the elements which should replace the old elements
@@ -161,7 +163,7 @@ class EditSelectionContents: public ElementContainer, public Serializable {
     /**
      * Gets the complete original bounding box as rectangle
      */
-    xoj::util::Rectangle<double> getOriginalBounds() const;
+    auto getOriginalBounds() const -> xoj::util::Rectangle<double>;
 
 public:
     // Serialize interface

src/core/control/tools/Selection.h

@@ -41,15 +41,15 @@ class Selection: public ShapeContainer, public OverlayBase {
     virtual bool userTapped(double zoom) const = 0;
     virtual const std::vector<BoundaryPoint>& getBoundary() const = 0;
 
-    inline const std::shared_ptr<xoj::util::DispatchPool<xoj::view::SelectionView>>& getViewPool() const {
+    inline auto getViewPool() const -> const std::shared_ptr<xoj::util::DispatchPool<xoj::view::SelectionView>>& {
         return viewPool;
     }
 
-    bool isMultiLayerSelection();
+    auto isMultiLayerSelection() -> bool;
     /**
      * Get the selected elements and clears them (std::move)
      */
-    InsertionOrder releaseElements();
+    auto releaseElements() -> InsertionOrder;
 
 private:
 protected:

src/core/control/tools/TextEditor.cpp

@@ -1108,22 +1108,8 @@ void TextEditor::initializeEditionAt(double x, double y) {
         this->control->setFontSelected(text->getFont());
         this->originalTextElement = text;
 
-        // The original code did not use clone here (but did the cloning by hand). Why?
         this->textElement.reset(text->clone());
 
-        /*
-         * Original code
-         *
-        this->textElement = std::make_unique<Text>();
-        this->textElement->setX(text->getX());
-        this->textElement->setY(text->getY());
-        this->textElement->setColor(text->getColor());
-        this->textElement->setFont(text->getFont());
-        this->textElement->setText(text->getText());
-        this->textElement->setTimestamp(text->getTimestamp());
-        this->textElement->setAudioFilename(text->getAudioFilename());
-        **/
-
         text->setInEditing(true);
         this->page->fireElementChanged(text);
     }

src/core/control/tools/VerticalToolHandler.cpp

@@ -1,7 +1,6 @@
 #include "VerticalToolHandler.h"
 
 #include <algorithm>  // for max, min, minmax
-#include <cmath>      // for abs
 #include <memory>     // for __shared_ptr_access
 
 #include <cairo.h>           // for cairo_fill, cairo_...

src/core/control/tools/VerticalToolHandler.h

@@ -12,7 +12,6 @@
 #pragma once
 
 #include <memory>    // for unique_ptr
-#include <optional>  // for optional
 #include <vector>    // for vector
 
 #include <cairo.h>    // for cairo_surface_t, cairo_t

src/core/gui/PageViewFindObjectHelper.h

@@ -14,15 +14,20 @@
 // No include needed, this is included after PageView.h
 
 #include <limits>
+#include <mutex>
 #include <optional>
 
 #include "control/AudioController.h"
+#include "control/Control.h"
+#include "control/layer/LayerController.h"
+#include "control/settings/Settings.h"
 #include "control/tools/EditSelection.h"
-#include "util/PathUtil.h"
-#include "util/safe_casts.h"  // for as_unsigned
+#include "gui/PageView.h"
+#include "model/Layer.h"
+#include "model/XojPage.h"
+#include "util/safe_casts.h"
 
 #include "XournalView.h"
-#include "filesystem.h"
 
 class BaseSelectObject {
 public:

src/core/model/Element.cpp

@@ -14,8 +14,6 @@ using xoj::util::Rectangle;
 
 Element::Element(ElementType type): type(type) {}
 
-Element::~Element() = default;
-
 auto Element::getType() const -> ElementType { return this->type; }
 
 void Element::setX(double x) {

src/core/model/Element.h

@@ -36,7 +36,7 @@ class Element: public Serializable {
     Element(ElementType type);
 
 public:
-    ~Element() override;
+    ~Element() override = default;
 
     using Index = std::ptrdiff_t;
     static constexpr auto InvalidIndex = static_cast<Index>(-1);

src/core/model/Layer.cpp

@@ -1,5 +1,10 @@
 #include "Layer.h"
 
+#include <cstddef>
+#include <memory>
+#include <utility>
+#include <vector>
+
 #include <glib.h>  // for g_warning
 
 #include "model/Element.h"    // for Element, Element::Index, Element::Inval...