shiboken6: Fix field modifications

The logic was only partially present, neither removal
of getter/setter nor renaming had any effect.

Rewrite the code to resemble that of AbstractMetaFunction
(adding function applyFieldModifications())
Move some check functions from the generators to
AbstractMetaField/Type.

Add tests in libsample.

Change-Id: Ib29d4e37db51f122b46702cb5d96b13da6d0f224
Reviewed-by: Christian Tismer <tismer@stackless.com>

11 files changed, 124 insertions, 44 deletions

diff --git a/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp
index 0d86e028f..ac410e920 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp
@@ -1175,15 +1175,35 @@ std::optional<AbstractMetaField>
     return metaField;
 }
 
+static bool applyFieldModifications(AbstractMetaField *f)
+{
+    const auto &modifications = f->modifications();
+    for (const auto &mod : modifications) {
+        if (mod.isRemoveModifier() && mod.removal() == TypeSystem::All)
+            return false;
+        if (mod.isRenameModifier()) {
+            f->setOriginalName(f->name());
+            f->setName(mod.renamedToName());
+        } else if (!mod.isReadable()) {
+            f->setGetterEnabled(false);
+        } else if (!mod.isWritable()) {
+            f->setSetterEnabled(false);
+        }
+    }
+    f->setOriginalAttributes(f->attributes());
+    return true;
+}
+
 void AbstractMetaBuilderPrivate::traverseFields(const ScopeModelItem &scope_item,
                                                 AbstractMetaClass *metaClass)
 {
     const VariableList &variables = scope_item->variables();
     for (const VariableModelItem &field : variables) {
-        auto metaField = traverseField(field, metaClass);
-        if (metaField.has_value() && !metaField->isModifiedRemoved()) {
-            metaField->setOriginalAttributes(metaField->attributes());
-            metaClass->addField(*metaField);
+        auto metaFieldO = traverseField(field, metaClass);
+        if (metaFieldO.has_value()) {
+            AbstractMetaField metaField = metaFieldO.value();
+            if (applyFieldModifications(&metaField))
+                metaClass->addField(metaField);
         }
     }
 }
diff --git a/sources/shiboken6/ApiExtractor/abstractmetafield.cpp b/sources/shiboken6/ApiExtractor/abstractmetafield.cpp
index 841266a37..fa7ab64d9 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetafield.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetafield.cpp
@@ -41,8 +41,9 @@ public:
     QString m_originalName;
     QString m_name;
     AbstractMetaType m_type;
-    bool m_hasName = false;
     Documentation m_doc;
+    bool m_setterEnabled = true; // Modifications
+    bool m_getterEnabled = true; // Modifications
 };
 
 AbstractMetaField::AbstractMetaField() : d(new AbstractMetaFieldData)
@@ -101,27 +102,21 @@ QString AbstractMetaField::name() const
     return d->m_name;
 }
 
-void AbstractMetaField::setName(const QString &name, bool realName)
+void AbstractMetaField::setName(const QString &name)
 {
-    if (d->m_name != name || d->m_hasName != realName) {
+    if (d->m_name != name)
         d->m_name = name;
-        d->m_hasName = realName;
-    }
-}
-
-bool AbstractMetaField::hasName() const
-{
-    return d->m_hasName;
 }
 
 QString AbstractMetaField::qualifiedCppName() const
 {
-    return enclosingClass()->qualifiedCppName() + QLatin1String("::") + d->m_name;
+    return enclosingClass()->qualifiedCppName() + QLatin1String("::")
+           + originalName();
 }
 
 QString AbstractMetaField::originalName() const
 {
-    return d->m_originalName;
+    return d->m_originalName.isEmpty() ? d->m_name : d->m_originalName;
 }
 
 void AbstractMetaField::setOriginalName(const QString &name)
@@ -141,6 +136,39 @@ void AbstractMetaField::setDocumentation(const Documentation &doc)
         d->m_doc = doc;
 }
 
+bool AbstractMetaField::isGetterEnabled() const
+{
+    return d->m_getterEnabled;
+}
+
+void AbstractMetaField::setGetterEnabled(bool e)
+{
+    if (d->m_getterEnabled != e)
+        d->m_getterEnabled = e;
+}
+
+bool AbstractMetaField::isSetterEnabled() const
+{
+    return d->m_setterEnabled;
+}
+
+void AbstractMetaField::setSetterEnabled(bool e)
+{
+    if (e != d->m_setterEnabled)
+        d->m_setterEnabled = e;
+}
+
+bool AbstractMetaField::canGenerateGetter() const
+{
+    return d->m_getterEnabled && !isStatic();
+}
+
+bool AbstractMetaField::canGenerateSetter() const
+{
+    return d->m_setterEnabled && !isStatic()
+        && (!d->m_type.isConstant() || d->m_type.isPointerToConst());
+}
+
 FieldModificationList AbstractMetaField::modifications() const
 {
     const FieldModificationList &mods = enclosingClass()->typeEntry()->fieldModifications();
diff --git a/sources/shiboken6/ApiExtractor/abstractmetafield.h b/sources/shiboken6/ApiExtractor/abstractmetafield.h
index ccd597471..7964d5b57 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetafield.h
+++ b/sources/shiboken6/ApiExtractor/abstractmetafield.h
@@ -62,8 +62,8 @@ public:
     void setType(const AbstractMetaType &type);
 
     QString name() const;
-    void setName(const QString &name, bool realName = true);
-    bool hasName() const;
+    void setName(const QString &name);
+
     QString qualifiedCppName() const;
 
     QString originalName() const;
@@ -72,6 +72,14 @@ public:
     const Documentation &documentation() const;
     void setDocumentation(const Documentation& doc);
 
+    bool isGetterEnabled() const; // Modifications
+    void setGetterEnabled(bool e);
+    bool isSetterEnabled() const; // Modifications
+    void setSetterEnabled(bool e);
+
+    bool canGenerateGetter() const;
+    bool canGenerateSetter() const;
+
     static std::optional<AbstractMetaField>
         find(const AbstractMetaFieldList &haystack, const QString &needle);
 
diff --git a/sources/shiboken6/ApiExtractor/abstractmetatype.cpp b/sources/shiboken6/ApiExtractor/abstractmetatype.cpp
index fa28238f2..990d1ec07 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetatype.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetatype.cpp
@@ -689,6 +689,12 @@ bool AbstractMetaType::isPointer() const
         || isNativePointer() || isValuePointer();
 }
 
+bool AbstractMetaType::isPointerToConst() const
+{
+    return d->m_constant && !d->m_indirections.isEmpty()
+        && d->m_indirections.constLast() != Indirection::ConstPointer;
+}
+
 bool AbstractMetaType::isCString() const
 {
     return isNativePointer()
diff --git a/sources/shiboken6/ApiExtractor/abstractmetatype.h b/sources/shiboken6/ApiExtractor/abstractmetatype.h
index a65858364..599321262 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetatype.h
+++ b/sources/shiboken6/ApiExtractor/abstractmetatype.h
@@ -197,6 +197,9 @@ public:
     // Query functions for generators
     /// Check if type is a pointer.
     bool isPointer() const;
+    /// Helper for field setters: Check for "const QWidget *" (settable field),
+    /// but not "int *const" (read-only field).
+    bool isPointerToConst() const;
     /// Returns true if the type is a C string (const char *).
     bool isCString() const;
     /// Returns true if the type is a void pointer.
diff --git a/sources/shiboken6/ApiExtractor/typesystemparser.cpp b/sources/shiboken6/ApiExtractor/typesystemparser.cpp
index 209d88290..b56fef22a 100644
--- a/sources/shiboken6/ApiExtractor/typesystemparser.cpp
+++ b/sources/shiboken6/ApiExtractor/typesystemparser.cpp
@@ -2165,6 +2165,9 @@ bool TypeSystemParser::parseModifyField(const QXmlStreamReader &reader,
                       qPrintable(msgUnimplementedAttributeWarning(reader, name)));
             if (!convertBoolean(attributes->takeAt(i).value(), writeAttribute(), true))
                 fm.clearModifierFlag(FieldModification::Writable);
+        } else if (name == renameAttribute()) {
+            fm.setRenamedToName(attributes->takeAt(i).value().toString());
+            fm.setModifierFlag(Modification::Rename);
         }
     }
     if (fm.name.isEmpty()) {
diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.cpp b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
index 7fdea833f..48f7915be 100644
--- a/sources/shiboken6/generator/shiboken/cppgenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
@@ -291,21 +291,6 @@ static QString chopType(QString s)
     return s;
 }
 
-// Helper for field setters: Check for "const QWidget *" (settable field),
-// but not "int *const" (read-only field).
-static bool isPointerToConst(const AbstractMetaType &t)
-{
-    const AbstractMetaType::Indirections &indirections = t.indirectionsV();
-    return t.isConstant() && !indirections.isEmpty()
-        && indirections.constLast() != Indirection::ConstPointer;
-}
-
-static inline bool canGenerateFieldSetter(const AbstractMetaField &field)
-{
-    const AbstractMetaType &type = field.type();
-    return !type.isConstant() || isPointerToConst(type);
-}
-
 static bool isStdSetterName(QString setterName, QString propertyName)
 {
    return setterName.size() == propertyName.size() + 3
@@ -712,10 +697,9 @@ void CppGenerator::generateClass(TextStream &s, const GeneratorContext &classCon
     if (shouldGenerateGetSetList(metaClass) && !classContext.forSmartPointer()) {
         const AbstractMetaFieldList &fields = metaClass->fields();
         for (const AbstractMetaField &metaField : fields) {
-            if (metaField.isStatic())
-                continue;
-            writeGetterFunction(s, metaField, classContext);
-            if (canGenerateFieldSetter(metaField))
+            if (metaField.canGenerateGetter())
+                writeGetterFunction(s, metaField, classContext);
+            if (metaField.canGenerateSetter())
                 writeSetterFunction(s, metaField, classContext);
             s << '\n';
         }
@@ -733,10 +717,11 @@ void CppGenerator::generateClass(TextStream &s, const GeneratorContext &classCon
             << "[] = {\n" << indent;
         for (const AbstractMetaField &metaField : fields) {
             if (!metaField.isStatic()) {
-                const QString setter = canGenerateFieldSetter(metaField)
+                const QString getter = metaField.canGenerateGetter()
+                    ? cpythonGetterFunctionName(metaField) : QString();
+                const QString setter = metaField.canGenerateSetter()
                     ? cpythonSetterFunctionName(metaField) : QString();
-                writePyGetSetDefEntry(s, metaField.name(),
-                                      cpythonGetterFunctionName(metaField), setter);
+                writePyGetSetDefEntry(s, metaField.name(), getter, setter);
             }
         }
 
@@ -4583,7 +4568,8 @@ void CppGenerator::writeGetterFunction(TextStream &s,
             << context.wrapperName() << " *>("
             << CPP_SELF_VAR << ")->" << protectedFieldGetterName(metaField) << "()";
     } else {
-        cppField = QLatin1String(CPP_SELF_VAR) + QLatin1String("->") + metaField.name();
+        cppField = QLatin1String(CPP_SELF_VAR) + QLatin1String("->")
+                   + metaField.originalName();
         if (newWrapperSameObject) {
             cppField.prepend(QLatin1String("&("));
             cppField.append(QLatin1Char(')'));
@@ -4704,7 +4690,8 @@ void CppGenerator::writeSetterFunction(TextStream &s,
                                 fieldType, context);
 
 
-    QString cppField = QLatin1String(CPP_SELF_VAR) + QLatin1String("->") + metaField.name();
+    const QString cppField = QLatin1String(CPP_SELF_VAR) + QLatin1String("->")
+                             + metaField.originalName();
     if (avoidProtectedHack() && metaField.isProtected()) {
         s << getFullTypeNameWithoutModifiers(fieldType);
         if (fieldType.indirections() == 1)
@@ -4721,7 +4708,7 @@ void CppGenerator::writeSetterFunction(TextStream &s,
             << PYTHON_TO_CPP_VAR << "(pyIn, &cppOut_local);\n"
             << cppField << " = cppOut_local";
     } else {
-        if (isPointerToConst(fieldType))
+        if (fieldType.isPointerToConst())
             s << "const ";
         s << getFullTypeNameWithoutModifiers(fieldType)
             << QString::fromLatin1(" *").repeated(fieldType.indirections()) << "& cppOut_ptr = "
diff --git a/sources/shiboken6/tests/libsample/abstract.cpp b/sources/shiboken6/tests/libsample/abstract.cpp
index e60c792c4..3b2b1ef0a 100644
--- a/sources/shiboken6/tests/libsample/abstract.cpp
+++ b/sources/shiboken6/tests/libsample/abstract.cpp
@@ -36,7 +36,7 @@ const int Abstract::staticPrimitiveField = 0;
 
 Abstract::Abstract(int id) : m_id(id)
 {
-    primitiveField = 123;
+    toBeRenamedField = readOnlyField = primitiveField = 123;
     valueTypeField = Point(12, 34);
     objectTypeField = nullptr;
     bitField = 0;
diff --git a/sources/shiboken6/tests/libsample/abstract.h b/sources/shiboken6/tests/libsample/abstract.h
index 09906f1ee..746107c71 100644
--- a/sources/shiboken6/tests/libsample/abstract.h
+++ b/sources/shiboken6/tests/libsample/abstract.h
@@ -67,6 +67,8 @@ public:
     Complex userPrimitiveField;
     Point valueTypeField;
     ObjectType* objectTypeField;
+    int toBeRenamedField;
+    int readOnlyField;
 
     Abstract(int id = -1);
     virtual ~Abstract();
diff --git a/sources/shiboken6/tests/samplebinding/class_fields_test.py b/sources/shiboken6/tests/samplebinding/class_fields_test.py
index 202efcafb..9cc59762e 100644
--- a/sources/shiboken6/tests/samplebinding/class_fields_test.py
+++ b/sources/shiboken6/tests/samplebinding/class_fields_test.py
@@ -65,6 +65,27 @@ class TestAccessingCppFields(unittest.TestCase):
         # attribution with invalid type
         self.assertRaises(TypeError, lambda : setattr(d, 'primitiveField', None))
 
+    def testAccessingRenamedFields(self):
+        '''Reads and writes a renamed field.'''
+        d = Derived()
+        self.assertEqual(type(d.renamedField), int)
+        old_value = d.renamedField
+        new_value = 2255
+        d.renamedField = new_value
+        self.assertEqual(d.renamedField, new_value)
+        self.assertNotEqual(d.renamedField, old_value)
+
+    def testAccessingReadOnlyFields(self):
+        '''Tests a read-only field.'''
+        d = Derived()
+        self.assertEqual(type(d.readOnlyField), int)
+        old_value = d.readOnlyField
+        try:
+            d.readOnlyField = 25555
+        except AttributeError:
+            pass
+        self.assertEqual(d.readOnlyField, old_value)
+
     def testAccessingUsersPrimitiveTypeField(self):
         '''Reads and writes an user's primitive type (in this case an 'Complex') field.'''
         d = Derived()
diff --git a/sources/shiboken6/tests/samplebinding/typesystem_sample.xml b/sources/shiboken6/tests/samplebinding/typesystem_sample.xml
index e1c5ab74a..6d9c842ac 100644
--- a/sources/shiboken6/tests/samplebinding/typesystem_sample.xml
+++ b/sources/shiboken6/tests/samplebinding/typesystem_sample.xml
@@ -640,6 +640,8 @@
         <enum-type name="PrintFormat"/>
         <modify-function signature="id()" rename="id_"/>
         <modify-function signature="hideFunction(HideType*)" remove="all"/>
+        <modify-field name="toBeRenamedField" rename="renamedField"/>
+        <modify-field name="readOnlyField" write="false"/>
     </object-type>
 
     <object-type name="Derived" polymorphic-id-expression="%1->type() == Derived::TpDerived">