diff --git a/src/App/PropertyExpressionEngine.cpp b/src/App/PropertyExpressionEngine.cpp index 02f21654ec..63fd2c40fb 100644 --- a/src/App/PropertyExpressionEngine.cpp +++ b/src/App/PropertyExpressionEngine.cpp @@ -1105,22 +1105,18 @@ void PropertyExpressionEngine::getLinksTo(std::vector& id identifiers.push_back(expressionId); break; } - bool found = false; - for (const auto& path : paths) { - if (path.getSubObjectName() == subname) { - identifiers.push_back(expressionId); - found = true; - break; - } - App::SubObjectT sobjT(obj, path.getSubObjectName().c_str()); - if (sobjT.getSubObject() == sobj && sobjT.getOldElementName() == subElement) { - identifiers.push_back(expressionId); - found = true; - break; - } - } - if (found) { - break; + if (std::any_of(paths.begin(), + paths.end(), + [subname, obj, sobj, &subElement](const auto& path) { + if (path.getSubObjectName() == subname) { + return true; + } + + App::SubObjectT sobjT(obj, path.getSubObjectName().c_str()); + return (sobjT.getSubObject() == sobj + && sobjT.getOldElementName() == subElement); + })) { + identifiers.push_back(expressionId); } } } diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index b7f81199cc..848704b9ed 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -73,7 +73,7 @@ void PropertyLinkBase::setAllowExternal(bool allow) { } void PropertyLinkBase::setSilentRestore(bool allow) { - setFlag(LinkSilentRestore,allow); + setFlag(LinkSilentRestore, allow); } void PropertyLinkBase::setReturnNewElement(bool enable) { @@ -800,13 +800,15 @@ void PropertyLink::getLinks(std::vector &objs, objs.push_back(_pcLink); } -void PropertyLink::getLinksTo(std::vector& identifiers, - App::DocumentObject* obj, - const char* subname, - bool all) const -{ - (void)subname; - if ((all || _pcScope != LinkScope::Hidden) && obj && _pcLink == obj) { +void PropertyLink::getLinksTo(std::vector &identifiers, + App::DocumentObject *obj, + const char *subname, + bool all) const { + (void) subname; + if (!all && _pcScope == LinkScope::Hidden) { + return; // Don't get hidden links unless all is specified. + } + if (obj && _pcLink == obj) { identifiers.emplace_back(*this); } } @@ -1165,9 +1167,9 @@ void PropertyLinkList::getLinksTo(std::vector& identifier return; } int i = -1; - for (auto o : _lValueList) { + for (auto docObj : _lValueList) { ++i; - if (o == obj) { + if (docObj == obj) { identifiers.emplace_back(*this, i); break; } @@ -1658,13 +1660,13 @@ void PropertyLinkBase::_getLinksTo(std::vector& identifie if (!subObject) { continue; } - // There is a subobject and the subname doesn't match our current entry + // After above, there is a subobject and the subname doesn't match our current entry App::SubObjectT sobjT(obj, sub.c_str()); if (sobjT.getSubObject() == subObject && sobjT.getOldElementName() == subElement) { identifiers.emplace_back(*this); return; } - // The oldElementName ( short, I.E. "Edge5" ) doesn't match. + // And the oldElementName ( short, I.E. "Edge5" ) doesn't match. if (i < (int)shadows.size()) { const auto& [shadowNewName, shadowOldName] = shadows[i]; if (shadowNewName == subname || shadowOldName == subname) { @@ -2909,15 +2911,18 @@ void PropertyLinkSubList::getLinksTo(std::vector& identif auto subElement = objT.getOldElementName(); int i = -1; - for (const auto& o : _lValueList) { + for (const auto& docObj : _lValueList) { ++i; - if (o != obj) { + if (docObj != obj) { continue; } + // If we don't specify a subname we looking for all; or if the subname is in our + // property, add this entry to our result if (!subname || (i < (int)_lSubList.size() && subname == _lSubList[i])) { identifiers.emplace_back(*this, i); continue; } + // If we couldn't find any subobjects or this object's index is in our list, ignore it if (!subObject || i < (int)_lSubList.size()) { continue; } @@ -4917,12 +4922,12 @@ void PropertyXLinkSubList::getLinksTo(std::vector& identi const char* subname, bool all) const { - if (all || _pcScope != LinkScope::Hidden) { - for (auto& l : _Links) { - // This is the same algorithm as _getLinksTo, but returns lists, not single entries - if (obj && obj == l._pcLink) { - _getLinksToList(identifiers, obj, subname, l._SubList, l._ShadowSubList); - } + if ( ! all && _pcScope != LinkScope::Hidden) { + return; + } + for (auto& l : _Links) { + if (obj && obj == l._pcLink) { + _getLinksToList(identifiers, obj, subname, l._SubList, l._ShadowSubList); } } } diff --git a/src/App/PropertyLinks.h b/src/App/PropertyLinks.h index dd17cc3448..63f8a71dc8 100644 --- a/src/App/PropertyLinks.h +++ b/src/App/PropertyLinks.h @@ -183,7 +183,7 @@ public: virtual void getLinks(std::vector &objs, bool all=false, std::vector *subs=nullptr, bool newStyle=true) const = 0; - /** Obtain identifiers from this link property that link to a give object + /** Obtain identifiers from this link property that link to a given object * @param identifiers: holds the returned identifier to reference the given object * @param obj: the referenced object * @param subname: optional subname reference diff --git a/src/Base/Bitmask.h b/src/Base/Bitmask.h index 133c470e2c..a232df6820 100644 --- a/src/Base/Bitmask.h +++ b/src/Base/Bitmask.h @@ -113,7 +113,8 @@ class Flags { Enum i; public: - constexpr inline Flags(Enum f = Enum()) : i(f) {} + // Linter seems wrong on next line, don't want explicit here forcing downstream changes + constexpr inline Flags(Enum f = Enum()) : i(f) {} // NOLINT (runtime/explicit) constexpr bool testFlag(Enum f) const { using u = typename std::underlying_type::type; return (i & f) == f && (static_cast(f) != 0 || i == f); diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index 07ac4d7dd9..54e0143eaf 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -41,7 +41,6 @@ #include #include #include -#include #include #include #include @@ -55,9 +54,6 @@ #include #include #include -#include -#include -#include #include #include #include @@ -81,7 +77,6 @@ #include #include #include -#include #include #include @@ -106,7 +101,6 @@ #include #include #include -#include #include FC_LOG_LEVEL_INIT("TopoShape", true, true) // NOLINT @@ -319,12 +313,12 @@ TopoDS_Shape TopoShape::findShape(TopAbs_ShapeEnum type, int idx) const return _cache->findShape(_Shape, type, idx); } -std::vector -TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(misc-no-recursion) - std::vector *names, - Data::SearchOptions options, - double tol, - double atol) const { +std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& subshape, + std::vector* names, + Data::SearchOptions options, + double tol, + double atol) const +{ bool checkGeometry = options.testFlag(Data::SearchOption::CheckGeometry); bool singleSearch = options.testFlag(Data::SearchOption::SingleResult); std::vector res; @@ -335,31 +329,42 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m int index = 0; TopAbs_ShapeEnum shapeType = subshape.shapeType(); - // This is an intentionally recursive method, which will exit after looking through all ancestors. - auto searchCompositeShape = [&](TopAbs_ShapeEnum childType) { // NOLINT(misc-no-recursion) + // This is an intentionally recursive method, which will exit after looking through all + // ancestors. + auto searchCompositeShape = [&](TopAbs_ShapeEnum childType) { // NOLINT (misc-no-recu2rsive) unsigned long count = subshape.countSubShapes(childType); - if (!count) + if (!count) { return; + } auto first = subshape.getSubTopoShape(childType, 1); - for (const auto &child: findSubShapesWithSharedVertex(first, nullptr, options, tol, atol)) { - for (int idx: findAncestors(child.getShape(), shapeType)) { + for (const auto& child : + findSubShapesWithSharedVertex(first, nullptr, options, tol, atol)) { + for (int idx : findAncestors(child.getShape(), shapeType)) { auto shape = getSubTopoShape(shapeType, idx); - if (shape.countSubShapes(childType) != count) + if (shape.countSubShapes(childType) != count) { continue; + } bool found = true; for (unsigned long i = 2; i < count; ++i) { - if (shape.findSubShapesWithSharedVertex(subshape.getSubTopoShape(childType, i), nullptr, - options, tol, atol).empty()) { + if (shape + .findSubShapesWithSharedVertex(subshape.getSubTopoShape(childType, i), + nullptr, + options, + tol, + atol) + .empty()) { found = false; break; } } if (found) { res.push_back(shape); - if (names) + if (names) { names->push_back(shapeName(shapeType) + std::to_string(idx)); - if (singleSearch) + } + if (singleSearch) { return; + } } } } @@ -382,38 +387,46 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m // special treatment of single sub-shape compound, that is, search // its extracting the compound if (countSubShapes(TopAbs_SHAPE) == 1) { - return findSubShapesWithSharedVertex(subshape.getSubTopoShape(TopAbs_SHAPE, 1), names, options, tol, + return findSubShapesWithSharedVertex(subshape.getSubTopoShape(TopAbs_SHAPE, 1), + names, + options, + tol, atol); - } else if (unsigned long count = countSubShapes(TopAbs_SHAPE)) { + } + else if (unsigned long count = countSubShapes(TopAbs_SHAPE)) { // For multi-sub-shape compound, only search for compound with the same // structure int idx = 0; - for (const auto &compound: getSubTopoShapes(shapeType)) { + for (const auto& compound : getSubTopoShapes(shapeType)) { ++idx; - if (compound.countSubShapes(TopAbs_SHAPE) != count) + if (compound.countSubShapes(TopAbs_SHAPE) != count) { continue; + } int i = 0; bool found = true; - for (const auto &s: compound.getSubTopoShapes(TopAbs_SHAPE)) { + for (const auto& s : compound.getSubTopoShapes(TopAbs_SHAPE)) { ++i; auto ss = subshape.getSubTopoShape(TopAbs_SHAPE, i); - if (ss.isNull() && s.isNull()) + if (ss.isNull() && s.isNull()) { continue; + } auto options2 = options; options2.setFlag(Data::SearchOption::SingleResult); - if (ss.isNull() || s.isNull() - || ss.shapeType() != s.shapeType() - || ss.findSubShapesWithSharedVertex(s, nullptr, options2, tol, atol).empty()) { + if (ss.isNull() || s.isNull() || ss.shapeType() != s.shapeType() + || ss.findSubShapesWithSharedVertex(s, nullptr, options2, tol, atol) + .empty()) { found = false; break; } } if (found) { - if (names) + if (names) { names->push_back(shapeName(shapeType) + std::to_string(idx)); + } res.push_back(compound); - if (singleSearch) + if (singleSearch) { return res; + } } } } @@ -421,17 +434,18 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m case TopAbs_VERTEX: // Vertex search will do comparison with tolerance to account for // rounding error inccured through transformation. - for (auto &shape: getSubTopoShapes(TopAbs_VERTEX)) { + for (auto& shape : getSubTopoShapes(TopAbs_VERTEX)) { ++index; if (BRep_Tool::Pnt(TopoDS::Vertex(shape.getShape())) - .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(subshape.getShape()))) + .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(subshape.getShape()))) <= tol2) { if (names) { names->push_back(std::string("Vertex") + std::to_string(index)); } res.push_back(shape); - if (singleSearch) + if (singleSearch) { return res; + } } } break; @@ -446,7 +460,8 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m if (shapeType == TopAbs_FACE) { wire = subshape.splitWires(); vertices = wire.getSubShapes(TopAbs_VERTEX); - } else { + } + else { vertices = subshape.getSubShapes(TopAbs_VERTEX); } @@ -458,12 +473,13 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m if (shapeType == TopAbs_EDGE) { isLine = (geom->isDerivedFrom(GeomLine::getClassTypeId()) || geom->isDerivedFrom(GeomLineSegment::getClassTypeId())); - } else { + } + else { isPlane = geom->isDerivedFrom(GeomPlane::getClassTypeId()); } } - auto compareGeometry = [&](const TopoShape &s, bool strict) { + auto compareGeometry = [&](const TopoShape& s, bool strict) { std::unique_ptr g2(Geometry::fromShape(s.getShape())); if (!g2) { return false; @@ -476,14 +492,16 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m && !g2->isDerivedFrom(GeomLineSegment::getClassTypeId())) { return false; } - } else if (isPlane && !strict) { + } + else if (isPlane && !strict) { // For planes, don't compare geometry either, so that // we don't need to worry about orientation and so on. // Just check the edges. if (!g2->isDerivedFrom(GeomPlane::getClassTypeId())) { return false; } - } else if (!g2 || !g2->isSame(*geom, tol, atol)) { + } + else if (!g2 || !g2->isSame(*geom, tol, atol)) { return false; } return true; @@ -492,15 +510,16 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m if (vertices.empty()) { // Probably an infinite shape, so we have to search by geometry int idx = 0; - for (auto &shape: getSubTopoShapes(shapeType)) { + for (auto& shape : getSubTopoShapes(shapeType)) { ++idx; if (!shape.countSubShapes(TopAbs_VERTEX) && compareGeometry(shape, true)) { if (names) { names->push_back(shapeName(shapeType) + std::to_string(idx)); } res.push_back(shape); - if (singleSearch) + if (singleSearch) { return res; + } } } break; @@ -513,9 +532,9 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m // * Perform geometry comparison of the ancestor and input shape. // * For face, perform addition geometry comparison of each edge. std::unordered_set shapeSet; - for (auto &vert: - findSubShapesWithSharedVertex(vertices[0], nullptr, options, tol, atol)) { - for (auto idx: findAncestors(vert.getShape(), shapeType)) { + for (auto& vert : + findSubShapesWithSharedVertex(vertices[0], nullptr, options, tol, atol)) { + for (auto idx : findAncestors(vert.getShape(), shapeType)) { auto shape = getSubTopoShape(shapeType, idx); if (!shapeSet.insert(shape).second) { continue; @@ -529,27 +548,27 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m continue; } otherVertices = otherWire.getSubShapes(TopAbs_VERTEX); - } else { + } + else { otherVertices = shape.getSubShapes(TopAbs_VERTEX); } if (otherVertices.size() != vertices.size()) { continue; } - if (checkGeometry - && !compareGeometry(shape, false)) { + if (checkGeometry && !compareGeometry(shape, false)) { continue; } unsigned ind = 0; bool matched = true; - for (auto &vertex: vertices) { + for (auto& vertex : vertices) { bool found = false; for (unsigned inner = 0; inner < otherVertices.size(); ++inner) { - auto &vertex1 = otherVertices[ind]; + auto& vertex1 = otherVertices[ind]; if (++ind == otherVertices.size()) { ind = 0; } if (BRep_Tool::Pnt(TopoDS::Vertex(vertex)) - .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(vertex1))) + .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(vertex1))) <= tol2) { found = true; break; @@ -573,7 +592,7 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m bool matched2 = true; unsigned i = 0; auto edges = wire.getSubShapes(TopAbs_EDGE); - for (auto &edge: edges) { + for (auto& edge : edges) { std::unique_ptr geom2(Geometry::fromShape(edge, true)); if (!geom2) { matched2 = false; @@ -590,8 +609,8 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m // We will tolerate on edge reordering bool found = false; for (unsigned j = 0; j < otherEdges.size(); j++) { - auto &e1 = otherEdges[i]; - auto &g1 = geos[i]; + auto& e1 = otherEdges[i]; + auto& g1 = geos[i]; if (++i >= otherEdges.size()) { i = 0; } @@ -605,9 +624,9 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m if (g1->isDerivedFrom(GeomLine::getClassTypeId()) || g1->isDerivedFrom(GeomLineSegment::getClassTypeId())) { auto p1 = - BRep_Tool::Pnt(TopExp::FirstVertex(TopoDS::Edge(e1))); + BRep_Tool::Pnt(TopExp::FirstVertex(TopoDS::Edge(e1))); auto p2 = - BRep_Tool::Pnt(TopExp::LastVertex(TopoDS::Edge(e1))); + BRep_Tool::Pnt(TopExp::LastVertex(TopoDS::Edge(e1))); if ((p1.SquareDistance(pt1) <= tol2 && p2.SquareDistance(pt2) <= tol2) || (p1.SquareDistance(pt2) <= tol2 diff --git a/src/Mod/Spreadsheet/App/PropertySheet.cpp b/src/Mod/Spreadsheet/App/PropertySheet.cpp index 4473de6e00..9c6bc8b045 100644 --- a/src/Mod/Spreadsheet/App/PropertySheet.cpp +++ b/src/Mod/Spreadsheet/App/PropertySheet.cpp @@ -2295,23 +2295,18 @@ void PropertySheet::getLinksTo(std::vector& identifiers, identifiers.emplace_back(owner, cellName.toString().c_str()); break; } - bool found = false; - for (const auto& path : paths) { - if (path.getSubObjectName() == subname) { - identifiers.emplace_back(owner, cellName.toString().c_str()); - found = true; - break; - } - App::SubObjectT sobjT(obj, path.getSubObjectName().c_str()); - if (sobjT.getSubObject() == subObject - && sobjT.getOldElementName() == subElement) { - identifiers.emplace_back(owner, cellName.toString().c_str()); - found = true; - break; - } - } - if (found) { - break; + if (std::any_of(paths.begin(), + paths.end(), + [subname, obj, subObject, &subElement](const auto& path) { + if (path.getSubObjectName() == subname) { + return true; + } + + App::SubObjectT sobjT(obj, path.getSubObjectName().c_str()); + return (sobjT.getSubObject() == subObject + && sobjT.getOldElementName() == subElement); + })) { + identifiers.emplace_back(owner, cellName.toString().c_str()); } } } diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index 85855e0f22..55f0ac492c 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -1140,16 +1140,19 @@ TEST_F(TopoShapeExpansionTest, findSubShapesWithSharedVertexEverything) exp.Init(box1, TopAbs_VERTEX); auto vertex = exp.Current(); // Act - auto shapes = - box1TS.findSubShapesWithSharedVertex(face, &names, CheckGeometry::checkGeometry, tol, atol); + auto shapes = box1TS.findSubShapesWithSharedVertex(face, + &names, + Data::SearchOption::CheckGeometry, + tol, + atol); auto shapes1 = box1TS.findSubShapesWithSharedVertex(edge, &names1, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); auto shapes2 = box1TS.findSubShapesWithSharedVertex(vertex, &names2, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); // Assert @@ -1185,16 +1188,19 @@ TEST_F(TopoShapeExpansionTest, findSubShapesWithSharedVertexMid) exp.Init(box1, TopAbs_VERTEX); auto vertex = exp.Current(); // Act - auto shapes = - box1TS.findSubShapesWithSharedVertex(face, &names, CheckGeometry::checkGeometry, tol, atol); + auto shapes = box1TS.findSubShapesWithSharedVertex(face, + &names, + Data::SearchOption::CheckGeometry, + tol, + atol); auto shapes1 = box1TS.findSubShapesWithSharedVertex(edge, &names1, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); auto shapes2 = box1TS.findSubShapesWithSharedVertex(vertex, &names2, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); // Assert @@ -1224,16 +1230,19 @@ TEST_F(TopoShapeExpansionTest, findSubShapesWithSharedVertexClose) exp.Init(box1, TopAbs_VERTEX); auto vertex = exp.Current(); // Act - auto shapes = - box1TS.findSubShapesWithSharedVertex(face, &names, CheckGeometry::checkGeometry, tol, atol); + auto shapes = box1TS.findSubShapesWithSharedVertex(face, + &names, + Data::SearchOption::CheckGeometry, + tol, + atol); auto shapes1 = box1TS.findSubShapesWithSharedVertex(edge, &names1, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); auto shapes2 = box1TS.findSubShapesWithSharedVertex(vertex, &names2, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); // Assert