From 43b63ac4bfabd20a6ac3ff9ffebf7f7ec35974ae Mon Sep 17 00:00:00 2001 From: Kristian Bendiksen Date: Fri, 23 Oct 2026 10:46:10 +0200 Subject: [PATCH] #14912 Reject enum names that conflict with module symbols --- .../RimExtrudedCurveIntersection.cpp | 13 ++++++++ .../cafPdmScripting/cafPdmPythonGenerator.cpp | 30 ++++++++++++------- .../cafMockObjects.cpp | 12 +++++++- .../cafMockObjects.h | 2 ++ .../cafPdmPythonGeneratorTest.cpp | 19 ++++++++++++ 5 files changed, 64 insertions(+), 12 deletions(-) diff --git a/ApplicationLibCode/ProjectDataModel/Intersections/RimExtrudedCurveIntersection.cpp b/ApplicationLibCode/ProjectDataModel/Intersections/RimExtrudedCurveIntersection.cpp index cec2449808..ebf0a3d920 100644 --- a/ApplicationLibCode/ProjectDataModel/Intersections/RimExtrudedCurveIntersection.cpp +++ b/ApplicationLibCode/ProjectDataModel/Intersections/RimExtrudedCurveIntersection.cpp @@ -51,6 +51,7 @@ #include "cafPdmFieldScriptingCapability.h" #include "cafPdmFieldScriptingCapabilityCvfVec3d.h" #include "cafPdmObjectScriptingCapability.h" +#include "cafPdmScriptEnumNameRegistry.h" #include "cafPdmUiCheckBoxEditor.h" #include "cafPdmUiDoubleSliderEditor.h" #include "cafPdmUiListEditor.h" @@ -83,6 +84,18 @@ void caf::AppEnum::setUp() } // namespace caf +namespace +{ +struct RegisterScriptEnumNames +{ + RegisterScriptEnumNames() + { + caf::PdmScriptEnumNameRegistry::registerName( "CrossSectionType" ); + } +}; +const RegisterScriptEnumNames s_registerScriptEnumNames; +} // namespace + CAF_PDM_SOURCE_INIT( RimExtrudedCurveIntersection, "CurveIntersection", "CrossSection" ); //-------------------------------------------------------------------------------------------------- diff --git a/Fwk/AppFwk/cafPdmScripting/cafPdmPythonGenerator.cpp b/Fwk/AppFwk/cafPdmScripting/cafPdmPythonGenerator.cpp index fd9ef92f75..773eb99dd4 100644 --- a/Fwk/AppFwk/cafPdmScripting/cafPdmPythonGenerator.cpp +++ b/Fwk/AppFwk/cafPdmScripting/cafPdmPythonGenerator.cpp @@ -165,7 +165,7 @@ QString caf::PdmPythonGenerator::generate( PdmObjectFactory* factory, std::vecto std::map> enumTypeAndFieldByBaseName; std::set ambiguousEnumDataTypesReported; std::set duplicateEnumClassNamesReported; - bool duplicateEnumClassNameDetected = false; + bool enumClassNameCollisionDetected = false; // Seed with names that are already bound at module scope in the generated file (imports // and module-level functions) so a generated enum cannot shadow them. std::set usedEnumClassNames = { "PdmObjectBase", @@ -250,7 +250,7 @@ QString caf::PdmPythonGenerator::generate( PdmObjectFactory* factory, std::vecto enumTypeAndFieldByBaseName.emplace( baseName, std::make_pair( dataTypeName, fieldName ) ); if ( !inserted && enumNameIt->second.first != dataTypeName ) { - duplicateEnumClassNameDetected = true; + enumClassNameCollisionDetected = true; if ( !duplicateEnumClassNamesReported.count( baseName ) ) { errorMessages.push_back( QString( "Duplicate Python enum class name '%1' is inferred for distinct enum " @@ -269,15 +269,23 @@ QString caf::PdmPythonGenerator::generate( PdmObjectFactory* factory, std::vecto auto cached = enumClassNameByDataTypeName.find( dataTypeName ); if ( cached != enumClassNameByDataTypeName.end() ) return cached->second; - QString candidate = baseName; - int suffix = 1; - while ( usedEnumClassNames.count( candidate ) ) + if ( usedEnumClassNames.count( baseName ) ) { - candidate = baseName + QString::number( ++suffix ); + enumClassNameCollisionDetected = true; + if ( !duplicateEnumClassNamesReported.count( baseName ) ) + { + errorMessages.push_back( + QString( "Python enum class name '%1' inferred for field '%2' conflicts with an existing module " + "name. Register a distinct name with caf::PdmScriptEnumNameRegistry." ) + .arg( baseName ) + .arg( fieldName ) ); + duplicateEnumClassNamesReported.insert( baseName ); + } + return QString(); } EnumClassDef def; - def.className = candidate; + def.className = baseName; std::set usedMemberNames; for ( const QString& text : enumTexts ) { @@ -293,10 +301,10 @@ QString caf::PdmPythonGenerator::generate( PdmObjectFactory* factory, std::vecto def.memberNameByText[text] = unique; } - enumClassNameByDataTypeName[dataTypeName] = candidate; + enumClassNameByDataTypeName[dataTypeName] = baseName; enumDefByDataTypeName[dataTypeName] = def; - usedEnumClassNames.insert( candidate ); - return candidate; + usedEnumClassNames.insert( baseName ); + return baseName; }; auto enumClassNameForMethodReturn = [&]( const QString& scriptName, const QStringList& enumTexts ) -> QString @@ -807,7 +815,7 @@ QString caf::PdmPythonGenerator::generate( PdmObjectFactory* factory, std::vecto } } - if ( duplicateEnumClassNameDetected ) return QString(); + if ( enumClassNameCollisionDetected ) return QString(); return generatedCode; } diff --git a/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafMockObjects.cpp b/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafMockObjects.cpp index 5dda3d6527..940d30e072 100644 --- a/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafMockObjects.cpp +++ b/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafMockObjects.cpp @@ -186,7 +186,8 @@ void InheritedDemoObj::setUseAmbiguousEnumField( bool useAmbiguousEnumField ) CAF_PDM_SOURCE_INIT( CollisionTestPdmObject, "CollisionTestPdmObject" ); -bool CollisionTestPdmObject::s_useCollisionEnumField = false; +bool CollisionTestPdmObject::s_useCollisionEnumField = false; +bool CollisionTestPdmObject::s_useReservedNameEnumField = false; CollisionTestPdmObject::CollisionTestPdmObject() { @@ -195,6 +196,10 @@ CollisionTestPdmObject::CollisionTestPdmObject() { CAF_PDM_InitScriptableFieldNoDefault( &m_enumField, "MyAppEnumValue", "A duplicate enum class name" ); } + else if ( s_useReservedNameEnumField ) + { + CAF_PDM_InitScriptableFieldNoDefault( &m_enumField, "Type", "An enum with a reserved class name" ); + } } void CollisionTestPdmObject::setUseCollisionEnumField( bool useCollisionEnumField ) @@ -202,6 +207,11 @@ void CollisionTestPdmObject::setUseCollisionEnumField( bool useCollisionEnumFiel s_useCollisionEnumField = useCollisionEnumField; } +void CollisionTestPdmObject::setUseReservedNameEnumField( bool useReservedNameEnumField ) +{ + s_useReservedNameEnumField = useReservedNameEnumField; +} + CAF_PDM_SOURCE_INIT( MyPdmDocument, "MyPdmDocument" ); //-------------------------------------------------------------------------------------------------- diff --git a/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafMockObjects.h b/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafMockObjects.h index 1fd6ab3a18..e4ef7db3af 100644 --- a/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafMockObjects.h +++ b/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafMockObjects.h @@ -152,11 +152,13 @@ class CollisionTestPdmObject : public caf::PdmObject public: CollisionTestPdmObject(); static void setUseCollisionEnumField( bool useCollisionEnumField ); + static void setUseReservedNameEnumField( bool useReservedNameEnumField ); caf::PdmField> m_enumField; private: static bool s_useCollisionEnumField; + static bool s_useReservedNameEnumField; }; class MyPdmDocument : public caf::PdmDocument diff --git a/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafPdmPythonGeneratorTest.cpp b/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafPdmPythonGeneratorTest.cpp index fa842fb582..c6d85de548 100644 --- a/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafPdmPythonGeneratorTest.cpp +++ b/Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafPdmPythonGeneratorTest.cpp @@ -233,3 +233,22 @@ TEST( PdmPythonGenerator, RejectsDuplicateEnumClassNames ) EXPECT_TRUE( errorMessages.front().contains( "CollisionTestPdmObject.MyAppEnumValue" ) ); EXPECT_TRUE( errorMessages.front().contains( "shared header" ) ); } + +//-------------------------------------------------------------------------------------------------- +//-------------------------------------------------------------------------------------------------- +TEST( PdmPythonGenerator, RejectsEnumClassNamesThatConflictWithModuleNames ) +{ + CollisionTestPdmObject::setUseReservedNameEnumField( true ); + + caf::PdmPythonGenerator generator; + std::vector errorMessages; + QString generatedCode = generator.generate( caf::PdmDefaultObjectFactory::instance(), errorMessages ); + + CollisionTestPdmObject::setUseReservedNameEnumField( false ); + + ASSERT_EQ( 1u, errorMessages.size() ); + EXPECT_TRUE( generatedCode.isEmpty() ); + EXPECT_TRUE( errorMessages.front().contains( "enum class name 'Type'" ) ); + EXPECT_TRUE( errorMessages.front().contains( "CollisionTestPdmObject.Type" ) ); + EXPECT_TRUE( errorMessages.front().contains( "PdmScriptEnumNameRegistry" ) ); +}