#14912 Reject enum names that conflict with module symbols

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<RimExtrudedCurveIntersection::CrossSectionDirEnum>::setUp()
 
 } // namespace caf
 
+namespace
+{
+struct RegisterScriptEnumNames
+{
+    RegisterScriptEnumNames()
+    {
+        caf::PdmScriptEnumNameRegistry::registerName<RimExtrudedCurveIntersection::CrossSectionEnum>( "CrossSectionType" );
+    }
+};
+const RegisterScriptEnumNames s_registerScriptEnumNames;
+} // namespace
+
 CAF_PDM_SOURCE_INIT( RimExtrudedCurveIntersection, "CurveIntersection", "CrossSection" );
 
 //--------------------------------------------------------------------------------------------------

Fwk/AppFwk/cafPdmScripting/cafPdmPythonGenerator.cpp

@@ -165,7 +165,7 @@ QString caf::PdmPythonGenerator::generate( PdmObjectFactory* factory, std::vecto
     std::map<QString, std::pair<QString, QString>> enumTypeAndFieldByBaseName;
     std::set<QString>                              ambiguousEnumDataTypesReported;
     std::set<QString>                              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<QString> 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<QString> 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;
 }
 

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" );
 
 //--------------------------------------------------------------------------------------------------

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<caf::AppEnum<CollisionTestEnum>> m_enumField;
 
 private:
     static bool s_useCollisionEnumField;
+    static bool s_useReservedNameEnumField;
 };
 
 class MyPdmDocument : public caf::PdmDocument

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<QString>    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" ) );
+}