Skip to content
Permalink
Browse files

Remove QgsGeometry bool operator

This is too dangerous -- it gets silently casted to numeric values
instead of throwing compilation errors
  • Loading branch information
nyalldawson committed Nov 4, 2018
1 parent 137cbd7 commit 7be2925649af880a4d4b18eee6ca23431dec591b
Showing with 66 additions and 93 deletions.
  1. +0 −2 python/core/auto_generated/geometry/qgsgeometry.sip.in
  2. +1 −1 src/analysis/interpolation/qgsinterpolator.cpp
  3. +1 −1 src/analysis/processing/qgsalgorithmboundary.cpp
  4. +1 −1 src/analysis/processing/qgsalgorithmbuffer.cpp
  5. +2 −2 src/analysis/processing/qgsalgorithmcentroid.cpp
  6. +2 −2 src/analysis/processing/qgsalgorithmconvexhull.cpp
  7. +2 −2 src/analysis/processing/qgsalgorithmdissolve.cpp
  8. +1 −1 src/analysis/processing/qgsalgorithmextendlines.cpp
  9. +1 −1 src/analysis/processing/qgsalgorithmfixgeometries.cpp
  10. +1 −1 src/analysis/processing/qgsalgorithmkmeansclustering.cpp
  11. +1 −1 src/analysis/processing/qgsalgorithmmergelines.cpp
  12. +1 −1 src/analysis/processing/qgsalgorithmmultiringconstantbuffer.cpp
  13. +2 −2 src/analysis/processing/qgsalgorithmpointonsurface.cpp
  14. +1 −1 src/analysis/processing/qgsalgorithmrotate.cpp
  15. +1 −1 src/analysis/processing/qgsalgorithmsmooth.cpp
  16. +1 −1 src/analysis/processing/qgsalgorithmsnaptogrid.cpp
  17. +1 −1 src/analysis/processing/qgsalgorithmsplitwithlines.cpp
  18. +1 −1 src/analysis/processing/qgsalgorithmsubdivide.cpp
  19. +2 −2 src/analysis/processing/qgsalgorithmtaperedbuffer.cpp
  20. +2 −2 src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.cpp
  21. +2 −2 src/analysis/vector/geometry_checker/qgssinglegeometrycheck.cpp
  22. +1 −1 src/analysis/vector/geometry_checker/qgsvectordataproviderfeaturepool.cpp
  23. +1 −1 src/app/qgsidentifyresultsdialog.cpp
  24. +1 −1 src/app/qgsmaptooloffsetcurve.cpp
  25. +1 −1 src/app/qgsmaptoolreverseline.cpp
  26. +5 −10 src/core/geometry/qgsgeometry.cpp
  27. +0 −7 src/core/geometry/qgsgeometry.h
  28. +1 −1 src/core/geometry/qgsgeos.cpp
  29. +1 −1 src/core/qgsmaphittest.cpp
  30. +1 −1 src/core/qgsogcutils.cpp
  31. +2 −2 src/core/qgspallabeling.cpp
  32. +1 −1 src/core/qgsvectorlayerdiagramprovider.cpp
  33. +1 −1 src/gui/qgsmaptoolcapture.cpp
  34. +1 −1 src/plugins/topology/topolTest.cpp
  35. +1 −1 src/providers/postgres/qgspostgresprovider.cpp
  36. +2 −2 src/server/services/wfs/qgswfsgetfeature.cpp
  37. +4 −4 tests/src/analysis/testqgsprocessing.cpp
  38. +8 −21 tests/src/core/testqgsgeometry.cpp
  39. +4 −4 tests/src/core/testqgsogcutils.cpp
  40. +3 −3 tests/src/core/testqgsvectorlayer.cpp
@@ -1824,8 +1824,6 @@ Downgrades a point list from QgsPoint to :py:class:`QgsPointXY`

operator QVariant() const;

operator bool() const;

}; // class QgsGeometry


@@ -114,7 +114,7 @@ QgsInterpolator::Result QgsInterpolator::cacheBaseData( QgsFeedback *feedback )

bool QgsInterpolator::addVerticesToCache( const QgsGeometry &geom, ValueSource source, double attributeValue )
{
if ( !geom || geom.isEmpty() )
if ( geom.isNull() || geom.isEmpty() )
return true; // nothing to do

//validate source
@@ -109,7 +109,7 @@ QgsFeatureList QgsBoundaryAlgorithm::processFeature( const QgsFeature &feature,
{
QgsGeometry inputGeometry = feature.geometry();
QgsGeometry outputGeometry = QgsGeometry( inputGeometry.constGet()->boundary() );
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->reportError( QObject::tr( "No boundary for feature %1 (possibly a closed linestring?)'" ).arg( feature.id() ) );
outFeature.clearGeometry();
@@ -136,7 +136,7 @@ QVariantMap QgsBufferAlgorithm::processAlgorithm( const QVariantMap &parameters,
}

QgsGeometry outputGeometry = f.geometry().buffer( distance, segments, endCapStyle, joinStyle, miterLimit );
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
QgsMessageLog::logMessage( QObject::tr( "Error calculating buffer for feature %1" ).arg( f.id() ), QObject::tr( "Processing" ), Qgis::Warning );
}
@@ -104,7 +104,7 @@ QgsFeatureList QgsCentroidAlgorithm::processFeature( const QgsFeature &f, QgsPro
{
QgsGeometry partGeometry( geomCollection->geometryN( i )->clone() );
QgsGeometry outputGeometry = partGeometry.centroid();
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->pushInfo( QObject::tr( "Error calculating centroid for feature %1 part %2: %3" ).arg( feature.id() ).arg( i ).arg( outputGeometry.lastError() ) );
}
@@ -115,7 +115,7 @@ QgsFeatureList QgsCentroidAlgorithm::processFeature( const QgsFeature &f, QgsPro
else
{
QgsGeometry outputGeometry = feature.geometry().centroid();
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->pushInfo( QObject::tr( "Error calculating centroid for feature %1: %2" ).arg( feature.id() ).arg( outputGeometry.lastError() ) );
}
@@ -83,11 +83,11 @@ QgsFeatureList QgsConvexHullAlgorithm::processFeature( const QgsFeature &feature
else
{
outputGeometry = f.geometry().convexHull();
if ( !outputGeometry )
if ( outputGeometry.isNull() )
feedback->reportError( outputGeometry.lastError() );
f.setGeometry( outputGeometry );
}
if ( outputGeometry )
if ( !outputGeometry.isNull() )
{
QgsAttributes attrs = f.attributes();
attrs << outputGeometry.constGet()->area()
@@ -67,7 +67,7 @@ QVariantMap QgsCollectorAlgorithm::processCollection( const QVariantMap &paramet
firstFeature = false;
}

if ( f.hasGeometry() && f.geometry() )
if ( f.hasGeometry() && !f.geometry().isNull() )
{
geomQueue.append( f.geometry() );
if ( maxQueueLength > 0 && geomQueue.length() > maxQueueLength )
@@ -118,7 +118,7 @@ QVariantMap QgsCollectorAlgorithm::processCollection( const QVariantMap &paramet
attributeHash.insert( indexAttributes, f.attributes() );
}

if ( f.hasGeometry() && f.geometry() )
if ( f.hasGeometry() && !f.geometry().isNull() )
{
geometryHash[ indexAttributes ].append( f.geometry() );
}
@@ -129,7 +129,7 @@ QgsFeatureList QgsExtendLinesAlgorithm::processFeature( const QgsFeature &featur
endDistance = mEndDistanceProperty.valueAsDouble( context.expressionContext(), endDistance );

const QgsGeometry outGeometry = geometry.extendLine( startDistance, endDistance );
if ( !outGeometry )
if ( outGeometry.isNull() )
throw QgsProcessingException( QObject::tr( "Error calculating extended line" ) ); // don't think this can actually happen!

f.setGeometry( outGeometry );
@@ -93,7 +93,7 @@ QgsFeatureList QgsFixGeometriesAlgorithm::processFeature( const QgsFeature &feat
QgsFeature outputFeature = feature;

QgsGeometry outputGeometry = outputFeature.geometry().makeValid();
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->pushInfo( QObject::tr( "makeValid failed for feature %1 " ).arg( feature.id() ) );
outputFeature.clearGeometry();
@@ -115,7 +115,7 @@ QVariantMap QgsKMeansClusteringAlgorithm::processAlgorithm( const QVariantMap &p
else
{
QgsGeometry centroid = feat.geometry().centroid();
if ( !centroid )
if ( centroid.isNull() )
continue; // centroid failed, e.g. empty linestring

point = QgsPointXY( *qgsgeometry_cast< const QgsPoint * >( centroid.constGet() ) );
@@ -83,7 +83,7 @@ QgsFeatureList QgsMergeLinesAlgorithm::processFeature( const QgsFeature &feature

QgsFeature out = feature;
QgsGeometry outputGeometry = feature.geometry().mergeLines();
if ( !outputGeometry )
if ( outputGeometry.isNull() )
feedback->reportError( QObject::tr( "Error merging lines for feature %1" ).arg( feature.id() ) );

out.setGeometry( outputGeometry );
@@ -133,7 +133,7 @@ QgsFeatureList QgsMultiRingConstantBufferAlgorithm::processFeature( const QgsFea
QgsFeature out;
currentDistance = i * distance;
outputGeometry = feature.geometry().buffer( currentDistance, 40 );
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->reportError( QObject::tr( "Error calculating buffer for feature %1" ).arg( feature.id() ) );
continue;
@@ -102,7 +102,7 @@ QgsFeatureList QgsPointOnSurfaceAlgorithm::processFeature( const QgsFeature &f,
{
QgsGeometry partGeometry( geomCollection->geometryN( i )->clone() );
QgsGeometry outputGeometry = partGeometry.pointOnSurface();
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->pushInfo( QObject::tr( "Error calculating point on surface for feature %1 part %2: %3" ).arg( feature.id() ).arg( i ).arg( outputGeometry.lastError() ) );
}
@@ -113,7 +113,7 @@ QgsFeatureList QgsPointOnSurfaceAlgorithm::processFeature( const QgsFeature &f,
else
{
QgsGeometry outputGeometry = feature.geometry().pointOnSurface();
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->pushInfo( QObject::tr( "Error calculating point on surface for feature %1: %2" ).arg( feature.id() ).arg( outputGeometry.lastError() ) );
}
@@ -129,7 +129,7 @@ QgsFeatureList QgsRotateFeaturesAlgorithm::processFeature( const QgsFeature &fea
else
{
QgsGeometry centroid = geometry.centroid();
if ( centroid )
if ( !centroid.isNull() )
{
geometry.rotate( angle, centroid.asPoint() );
f.setGeometry( geometry );
@@ -147,7 +147,7 @@ QgsFeatureList QgsSmoothAlgorithm::processFeature( const QgsFeature &feature, Qg
maxAngle = mMaxAngleProperty.valueAsDouble( context.expressionContext(), maxAngle );

QgsGeometry outputGeometry = f.geometry().smooth( iterations, offset, -1, maxAngle );
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->reportError( QObject::tr( "Error smoothing geometry %1" ).arg( feature.id() ) );
}
@@ -145,7 +145,7 @@ QgsFeatureList QgsSnapToGridAlgorithm::processFeature( const QgsFeature &feature
intervalM = mIntervalMProperty.valueAsDouble( context.expressionContext(), intervalM );

QgsGeometry outputGeometry = f.geometry().snappedToGrid( intervalX, intervalY, intervalZ, intervalM );
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->reportError( QObject::tr( "Error snapping geometry %1" ).arg( feature.id() ) );
}
@@ -192,7 +192,7 @@ QVariantMap QgsSplitWithLinesAlgorithm::processAlgorithm( const QVariantMap &par
}

QgsGeometry inGeom = inGeoms.takeFirst();
if ( !inGeom )
if ( inGeom.isNull() )
continue;

if ( splitGeomEngine->intersects( inGeom.constGet() ) )
@@ -90,7 +90,7 @@ QgsFeatureList QgsSubdivideAlgorithm::processFeature( const QgsFeature &f, QgsPr
maxNodes = mMaxNodesProperty.valueAsDouble( context.expressionContext(), maxNodes );

feature.setGeometry( feature.geometry().subdivide( maxNodes ) );
if ( !feature.geometry() )
if ( !feature.hasGeometry() )
{
feedback->reportError( QObject::tr( "Error calculating subdivision for feature %1" ).arg( feature.id() ) );
}
@@ -141,7 +141,7 @@ QgsFeatureList QgsTaperedBufferAlgorithm::processFeature( const QgsFeature &feat
endWidth = mEndWidthProperty.valueAsDouble( context.expressionContext(), endWidth );

QgsGeometry outputGeometry = feature.geometry().taperedBuffer( startWidth, endWidth, segments );
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->reportError( QObject::tr( "Error buffering geometry %1: %2" ).arg( feature.id() ).arg( outputGeometry.lastError() ) );
}
@@ -241,7 +241,7 @@ QgsFeatureList QgsVariableWidthBufferByMAlgorithm::processFeature( const QgsFeat
segments = mSegmentsProperty.valueAsInt( context.expressionContext(), segments );

QgsGeometry outputGeometry = feature.geometry().variableWidthBufferByM( segments );
if ( !outputGeometry )
if ( outputGeometry.isNull() )
{
feedback->reportError( QObject::tr( "Error buffering geometry %1: %2" ).arg( feature.id() ).arg( outputGeometry.lastError() ) );
}
@@ -45,7 +45,7 @@ QgsGeometryCheckerUtils::LayerFeature::LayerFeature( const QgsFeaturePool *pool,
{
mGeometry.transform( transform );
}
catch ( const QgsCsException &e )
catch ( const QgsCsException & )
{
QgsDebugMsg( QStringLiteral( "Shrug. What shall we do with a geometry that cannot be converted?" ) );
}
@@ -195,7 +195,7 @@ bool QgsGeometryCheckerUtils::LayerFeatures::iterator::nextFeature( bool begin )
if ( mParent->mFeedback )
mParent->mFeedback->setProgress( mParent->mFeedback->progress() + 1.0 );
QgsFeature feature;
if ( featurePool->getFeature( *mFeatureIt, feature ) && feature.geometry() && feature.geometry().constGet() )
if ( featurePool->getFeature( *mFeatureIt, feature ) && !feature.geometry().isNull() )
{
mCurrentFeature.reset( new LayerFeature( mParent->mFeaturePools[*mLayerIt], feature, mParent->mContext, mParent->mUseMapCrs ) );
return true;
@@ -49,9 +49,9 @@ void QgsSingleGeometryCheckError::update( const QgsSingleGeometryCheckError *oth

bool QgsSingleGeometryCheckError::isEqual( const QgsSingleGeometryCheckError *other ) const
{
return mGeometry == other->mGeometry
return mGeometry.equals( other->mGeometry )
&& mCheck == other->mCheck
&& mErrorLocation == other->mErrorLocation
&& mErrorLocation.equals( other->mErrorLocation )
&& mVertexId == other->mVertexId;
}

@@ -35,7 +35,7 @@ QgsVectorDataProviderFeaturePool::QgsVectorDataProviderFeaturePool( QgsVectorLay
QgsFeatureIterator it = layer->getFeatures( req );
while ( it.nextFeature( feature ) )
{
if ( feature.geometry() )
if ( feature.hasGeometry() )
{
insertFeature( feature );
featureIds.insert( feature.id() );
@@ -1631,7 +1631,7 @@ void QgsIdentifyResultsDialog::highlightFeature( QTreeWidgetItem *item )
if ( mHighlights.contains( featItem ) )
return;

if ( !featItem->feature().geometry() || featItem->feature().geometry().wkbType() == QgsWkbTypes::Unknown )
if ( !featItem->feature().hasGeometry() || featItem->feature().geometry().wkbType() == QgsWkbTypes::Unknown )
return;

QgsHighlight *highlight = nullptr;
@@ -594,7 +594,7 @@ void QgsMapToolOffsetCurve::updateGeometryAndRubberBand( double offset )
offsetGeom = mManipulatedGeometry.buffer( offset, quadSegments, capStyle, joinStyle, miterLimit );
}

if ( !offsetGeom )
if ( offsetGeom.isNull() )
{
deleteRubberBandAndGeometry();
deleteUserInputWidget();
@@ -115,7 +115,7 @@ void QgsMapToolReverseLine::canvasReleaseEvent( QgsMapMouseEvent *e )

}

if ( geom )
if ( !geom.isNull() )
{
vlayer->beginEditCommand( tr( "Reverse line" ) );
vlayer->changeGeometry( f.id(), geom );
@@ -729,7 +729,7 @@ QgsGeometry::OperationResult QgsGeometry::addPart( const QgsGeometry &newPart )
{
return QgsGeometry::InvalidBaseGeometry;
}
if ( !newPart || !newPart.d->geometry )
if ( newPart.isNull() || !newPart.d->geometry )
{
return QgsGeometry::AddPartNotMultiGeometry;
}
@@ -752,7 +752,7 @@ QgsGeometry QgsGeometry::removeInteriorRings( double minimumRingArea ) const
for ( const QgsGeometry &part : parts )
{
QgsGeometry result = part.removeInteriorRings( minimumRingArea );
if ( result )
if ( !result.isNull() )
results << result;
}
if ( results.isEmpty() )
@@ -1729,7 +1729,7 @@ QgsGeometry QgsGeometry::offsetCurve( double distance, int segments, JoinStyle j
for ( const QgsGeometry &part : parts )
{
QgsGeometry result = part.offsetCurve( distance, segments, joinStyle, miterLimit );
if ( result )
if ( !result.isNull() )
results << result;
}
if ( results.isEmpty() )
@@ -1772,7 +1772,7 @@ QgsGeometry QgsGeometry::singleSidedBuffer( double distance, int segments, Buffe
for ( const QgsGeometry &part : parts )
{
QgsGeometry result = part.singleSidedBuffer( distance, segments, side, joinStyle, miterLimit );
if ( result )
if ( !result.isNull() )
results << result;
}
if ( results.isEmpty() )
@@ -1830,7 +1830,7 @@ QgsGeometry QgsGeometry::extendLine( double startDistance, double endDistance )
for ( const QgsGeometry &part : parts )
{
QgsGeometry result = part.extendLine( startDistance, endDistance );
if ( result )
if ( !result.isNull() )
results << result;
}
if ( results.isEmpty() )
@@ -2675,11 +2675,6 @@ void QgsGeometry::convertPointList( const QgsPointSequence &input, QVector<QgsPo
}
}

QgsGeometry::operator bool() const
{
return d->geometry.get();
}

void QgsGeometry::convertToPolyline( const QgsPointSequence &input, QgsPolylineXY &output )
{
output.clear();
@@ -1823,13 +1823,6 @@ class CORE_EXPORT QgsGeometry
return QVariant::fromValue( *this );
}

/**
* Returns true if the geometry is non empty (ie, isNull() returns false),
* or false if it is an empty, uninitialized geometry (ie, isNull() returns true).
* \since QGIS 3.0
*/
operator bool() const;

private:

QgsGeometryPrivate *d; //implicitly shared data pointer
@@ -393,7 +393,7 @@ QgsAbstractGeometry *QgsGeos::combine( const QVector<QgsGeometry> &geomList, QSt
geosGeometries.reserve( geomList.size() );
for ( const QgsGeometry &g : geomList )
{
if ( !g )
if ( g.isNull() )
continue;

geosGeometries << asGeos( g.constGet(), mPrecision ).release();
@@ -152,7 +152,7 @@ void QgsMapHitTest::runHitTestLayer( QgsVectorLayer *vl, SymbolSet &usedSymbols,
{
context.expressionContext().setFeature( f );
// filter out elements outside of the polygon
if ( f.geometry() && polygonEngine )
if ( f.hasGeometry() && polygonEngine )
{
if ( !polygonEngine->intersects( f.geometry().constGet() ) )
{
@@ -1135,7 +1135,7 @@ QDomElement QgsOgcUtils::geometryToGML( const QgsGeometry &geometry, QDomDocumen
const QString &gmlIdBase,
int precision )
{
if ( !geometry )
if ( geometry.isNull() )
return QDomElement();

// coordinate separator
@@ -1567,7 +1567,7 @@ void QgsPalLayerSettings::registerFeature( QgsFeature &f, QgsRenderContext &cont
}

geos::unique_ptr geosObstacleGeomClone;
if ( obstacleGeometry )
if ( !obstacleGeometry.isNull() )
{
geosObstacleGeomClone = QgsGeos::asGeos( obstacleGeometry );
}
@@ -2000,7 +2000,7 @@ void QgsPalLayerSettings::registerObstacleFeature( QgsFeature &f, QgsRenderConte
mCurFeat = &f;

QgsGeometry geom;
if ( obstacleGeometry )
if ( !obstacleGeometry.isNull() )
{
geom = obstacleGeometry;
}

0 comments on commit 7be2925

Please sign in to comment.
You can’t perform that action at this time.