Python: Reject ambiguous generated enum names

ApplicationLibCode/ProjectDataModel/Appearance/RimFontSizeField.cpp

@@ -21,8 +21,18 @@
 #include "RiaPreferences.h"
 
 #include "cafAppEnum.h"
+#include "cafPdmScriptEnumNameRegistry.h"
 #include "cafPdmUiFieldHandle.h"
 
+namespace
+{
+struct RegisterScriptEnumNames
+{
+    RegisterScriptEnumNames() { caf::PdmScriptEnumNameRegistry::registerName<caf::FontTools::RelativeSize>( "SubTitleFontSize" ); }
+};
+const RegisterScriptEnumNames s_registerScriptEnumNames;
+} // namespace
+
 //--------------------------------------------------------------------------------------------------
 ///
 //--------------------------------------------------------------------------------------------------

ApplicationLibCode/ProjectDataModel/RimWbsParameters.cpp

@@ -28,6 +28,16 @@
 #include "RimWellPath.h"
 
 #include "cafPdmFieldScriptingCapability.h"
+#include "cafPdmScriptEnumNameRegistry.h"
+
+namespace
+{
+struct RegisterScriptEnumNames
+{
+    RegisterScriptEnumNames() { caf::PdmScriptEnumNameRegistry::registerName<RigWbsParameter::Source>( "PorePressureReservoirSource" ); }
+};
+const RegisterScriptEnumNames s_registerScriptEnumNames;
+} // namespace
 
 CAF_PDM_SOURCE_INIT( RimWbsParameters, "WbsParameters" );
 

Fwk/AppFwk/cafPdmScripting/cafPdmPythonGenerator.cpp

@@ -159,8 +159,10 @@ QString caf::PdmPythonGenerator::generate( PdmObjectFactory* factory, std::vecto
         std::vector<std::pair<QString, QString>> members; // (memberName, originalText), in declaration order
         std::map<QString, QString>               memberNameByText;
     };
-    std::map<QString, EnumClassDef> enumDefByDataTypeName;
-    std::map<QString, QString>      enumClassNameByDataTypeName;
+    std::map<QString, EnumClassDef>                enumDefByDataTypeName;
+    std::map<QString, QString>                     enumClassNameByDataTypeName;
+    std::map<QString, std::pair<QString, QString>> inferredEnumNameAndFieldByDataTypeName;
+    std::set<QString>                              ambiguousEnumDataTypesReported;
     // 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",
@@ -214,14 +216,34 @@ QString caf::PdmPythonGenerator::generate( PdmObjectFactory* factory, std::vecto
         if ( enumTexts.empty() ) return QString();
 
         QString dataTypeName = field->capability<PdmXmlFieldHandle>()->dataTypeName();
-        auto    cached       = enumClassNameByDataTypeName.find( dataTypeName );
-        if ( cached != enumClassNameByDataTypeName.end() ) return cached->second;
 
         // Prefer an explicit override registered by application code so the Python enum class name
         // is stable independent of which field the generator visits first.
-        QString baseName = PdmScriptEnumNameRegistry::lookup( dataTypeName );
-        if ( baseName.isEmpty() ) baseName = snakeToCamelCase( camelToSnakeCase( scriptability->scriptFieldName() ) );
-        if ( baseName.isEmpty() ) baseName = "Enum";
+        QString baseName     = PdmScriptEnumNameRegistry::lookup( dataTypeName );
+        QString inferredName = snakeToCamelCase( camelToSnakeCase( scriptability->scriptFieldName() ) );
+        if ( inferredName.isEmpty() ) inferredName = "Enum";
+
+        if ( baseName.isEmpty() )
+        {
+            QString fieldName = QString( "%1.%2" ).arg( field->ownerClass() ).arg( scriptability->scriptFieldName() );
+            auto [it, inserted] =
+                inferredEnumNameAndFieldByDataTypeName.emplace( dataTypeName, std::make_pair( inferredName, fieldName ) );
+            if ( !inserted && it->second.first != inferredName && !ambiguousEnumDataTypesReported.count( dataTypeName ) )
+            {
+                errorMessages.push_back(
+                    QString( "Ambiguous Python enum class names '%1' and '%2' inferred from fields '%3' and '%4'. "
+                             "Register a canonical name with caf::PdmScriptEnumNameRegistry." )
+                        .arg( it->second.first )
+                        .arg( inferredName )
+                        .arg( it->second.second )
+                        .arg( fieldName ) );
+                ambiguousEnumDataTypesReported.insert( dataTypeName );
+            }
+            baseName = it->second.first;
+        }
+
+        auto cached = enumClassNameByDataTypeName.find( dataTypeName );
+        if ( cached != enumClassNameByDataTypeName.end() ) return cached->second;
 
         QString candidate = baseName;
         int     suffix    = 1;

Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafMockObjects.cpp

@@ -130,6 +130,8 @@ DemoPdmObject::~DemoPdmObject()
 
 CAF_PDM_SOURCE_INIT( InheritedDemoObj, "InheritedDemoObj" );
 
+bool InheritedDemoObj::s_useAmbiguousEnumField = false;
+
 //--------------------------------------------------------------------------------------------------
 ///
 //--------------------------------------------------------------------------------------------------
@@ -149,6 +151,10 @@ InheritedDemoObj::InheritedDemoObj()
     CAF_PDM_InitScriptableField( &m_pairFloatField, "PairFloatField", std::make_pair( false, 0.0f ), "Pair Float Field" );
     CAF_PDM_InitFieldNoDefault( &m_testEnumField, "TestEnumValue", "An Enum" );
     CAF_PDM_InitScriptableFieldNoDefault( &m_myAppEnum, "MyAppEnumValue", "My App Enum" );
+    if ( s_useAmbiguousEnumField )
+    {
+        CAF_PDM_InitScriptableFieldNoDefault( &m_ambiguousAppEnum, "AlternativeEnumValue", "Alternative Enum" );
+    }
 
     CAF_PDM_InitFieldNoDefault( &m_simpleObjectsField,
                                 "SimpleObjects",
@@ -158,6 +164,14 @@ InheritedDemoObj::InheritedDemoObj()
                                 "Whatsthis SimpleObjectsField" );
 }
 
+//--------------------------------------------------------------------------------------------------
+///
+//--------------------------------------------------------------------------------------------------
+void InheritedDemoObj::setUseAmbiguousEnumField( bool useAmbiguousEnumField )
+{
+    s_useAmbiguousEnumField = useAmbiguousEnumField;
+}
+
 CAF_PDM_SOURCE_INIT( MyPdmDocument, "MyPdmDocument" );
 
 //--------------------------------------------------------------------------------------------------

Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafMockObjects.h

@@ -119,6 +119,8 @@ public:
 
     InheritedDemoObj();
 
+    static void setUseAmbiguousEnumField( bool useAmbiguousEnumField );
+
     caf::PdmField<std::vector<QString>>    m_texts;
     caf::PdmField<std::vector<double>>     m_numbers;
     caf::PdmField<std::optional<double>>   m_optionalNumber;
@@ -126,7 +128,11 @@ public:
     caf::PdmField<std::pair<bool, float>>  m_pairFloatField;
 
     caf::PdmField<caf::AppEnum<TestEnumType>> m_testEnumField;
+    MyAppEnumField                            m_ambiguousAppEnum;
     caf::PdmChildArrayField<SimpleObj*>       m_simpleObjectsField;
+
+private:
+    static bool s_useAmbiguousEnumField;
 };
 
 class MyPdmDocument : public caf::PdmDocument

Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests/cafPdmPythonGeneratorTest.cpp

@@ -37,6 +37,7 @@
 #include "gtest/gtest.h"
 
 #include "cafMockObjects.h"
+#include "cafPdmDefaultObjectFactory.h"
 #include "cafPdmPythonGenerator.h"
 
 //--------------------------------------------------------------------------------------------------
@@ -194,3 +195,21 @@ TEST( PdmPythonGenerator, DataTypeString_ChildField )
                   caf::PdmPythonGenerator::dataTypeString( &obj.m_simpleObjPtrField, false ).toStdString().c_str() );
     EXPECT_STREQ( "str", caf::PdmPythonGenerator::dataTypeString( &obj.m_simpleObjPtrField, true ).toStdString().c_str() );
 }
+
+//--------------------------------------------------------------------------------------------------
+//--------------------------------------------------------------------------------------------------
+TEST( PdmPythonGenerator, ReportsAmbiguousEnumClassNames )
+{
+    InheritedDemoObj::setUseAmbiguousEnumField( true );
+
+    caf::PdmPythonGenerator generator;
+    std::vector<QString>    errorMessages;
+    generator.generate( caf::PdmDefaultObjectFactory::instance(), errorMessages );
+
+    InheritedDemoObj::setUseAmbiguousEnumField( false );
+
+    ASSERT_EQ( 1u, errorMessages.size() );
+    EXPECT_TRUE( errorMessages.front().contains( "MyAppEnumValue" ) );
+    EXPECT_TRUE( errorMessages.front().contains( "AlternativeEnumValue" ) );
+    EXPECT_TRUE( errorMessages.front().contains( "PdmScriptEnumNameRegistry" ) );
+}