Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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 7be2925
Show file tree
Hide file tree
Showing 40 changed files with 66 additions and 93 deletions.
2 changes: 0 additions & 2 deletions python/core/auto_generated/geometry/qgsgeometry.sip.in
Expand Up @@ -1824,8 +1824,6 @@ Downgrades a point list from QgsPoint to :py:class:`QgsPointXY`

operator QVariant() const;

operator bool() const;

}; // class QgsGeometry


Expand Down
2 changes: 1 addition & 1 deletion src/analysis/interpolation/qgsinterpolator.cpp
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmboundary.cpp
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmbuffer.cpp
Expand Up @@ -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 );
}
Expand Down
4 changes: 2 additions & 2 deletions src/analysis/processing/qgsalgorithmcentroid.cpp
Expand Up @@ -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() ) );
}
Expand All @@ -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() ) );
}
Expand Down
4 changes: 2 additions & 2 deletions src/analysis/processing/qgsalgorithmconvexhull.cpp
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions src/analysis/processing/qgsalgorithmdissolve.cpp
Expand Up @@ -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 )
Expand Down Expand Up @@ -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() );
}
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmextendlines.cpp
Expand Up @@ -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 );
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmfixgeometries.cpp
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmkmeansclustering.cpp
Expand Up @@ -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() ) );
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmmergelines.cpp
Expand Up @@ -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 );
Expand Down
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/analysis/processing/qgsalgorithmpointonsurface.cpp
Expand Up @@ -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() ) );
}
Expand All @@ -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() ) );
}
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmrotate.cpp
Expand Up @@ -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 );
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmsmooth.cpp
Expand Up @@ -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() ) );
}
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmsnaptogrid.cpp
Expand Up @@ -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() ) );
}
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmsplitwithlines.cpp
Expand Up @@ -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() ) )
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmsubdivide.cpp
Expand Up @@ -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() ) );
}
Expand Down
4 changes: 2 additions & 2 deletions src/analysis/processing/qgsalgorithmtaperedbuffer.cpp
Expand Up @@ -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() ) );
}
Expand Down Expand Up @@ -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() ) );
}
Expand Down
Expand Up @@ -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?" ) );
}
Expand Down Expand Up @@ -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;
Expand Down
Expand Up @@ -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;
}

Expand Down
Expand Up @@ -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() );
Expand Down
2 changes: 1 addition & 1 deletion src/app/qgsidentifyresultsdialog.cpp
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/app/qgsmaptooloffsetcurve.cpp
Expand Up @@ -594,7 +594,7 @@ void QgsMapToolOffsetCurve::updateGeometryAndRubberBand( double offset )
offsetGeom = mManipulatedGeometry.buffer( offset, quadSegments, capStyle, joinStyle, miterLimit );
}

if ( !offsetGeom )
if ( offsetGeom.isNull() )
{
deleteRubberBandAndGeometry();
deleteUserInputWidget();
Expand Down
2 changes: 1 addition & 1 deletion src/app/qgsmaptoolreverseline.cpp
Expand Up @@ -115,7 +115,7 @@ void QgsMapToolReverseLine::canvasReleaseEvent( QgsMapMouseEvent *e )

}

if ( geom )
if ( !geom.isNull() )
{
vlayer->beginEditCommand( tr( "Reverse line" ) );
vlayer->changeGeometry( f.id(), geom );
Expand Down
15 changes: 5 additions & 10 deletions src/core/geometry/qgsgeometry.cpp
Expand Up @@ -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;
}
Expand All @@ -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() )
Expand Down Expand Up @@ -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() )
Expand Down Expand Up @@ -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() )
Expand Down Expand Up @@ -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() )
Expand Down Expand Up @@ -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();
Expand Down
7 changes: 0 additions & 7 deletions src/core/geometry/qgsgeometry.h
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/core/geometry/qgsgeos.cpp
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsmaphittest.cpp
Expand Up @@ -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() ) )
{
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsogcutils.cpp
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/core/qgspallabeling.cpp
Expand Up @@ -1567,7 +1567,7 @@ void QgsPalLayerSettings::registerFeature( QgsFeature &f, QgsRenderContext &cont
}

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

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

0 comments on commit 7be2925

Please sign in to comment.