This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch 
bugfix/distinguish-literal-from-reference-values-in-properties
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git

commit 1315675d1ef82b4c34b83758375af82d5195e406
Author: Konrad Windszus <[email protected]>
AuthorDate: Sat May 9 11:51:05 2026 +0200

    Distinguish between literal and reference property values
---
 .../scripts/migrate_annotations.py                 | 62 ++++++++++++++++++----
 .../scripts/tests/test_migrate_annotations.py      | 25 +++++++--
 2 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/skills/osgi-scr-migrator/scripts/migrate_annotations.py 
b/skills/osgi-scr-migrator/scripts/migrate_annotations.py
index 70385722..17f9014c 100755
--- a/skills/osgi-scr-migrator/scripts/migrate_annotations.py
+++ b/skills/osgi-scr-migrator/scripts/migrate_annotations.py
@@ -41,6 +41,12 @@ class Property:
     description: Optional[str] = None
     is_configurable: bool = False
 
+    def _strip_quotes(self, val: str) -> str:
+        """Strip surrounding quotes from a value if present."""
+        if val.startswith('"') and val.endswith('"'):
+            return val[1:-1]
+        return val
+
     def is_property_type_annotation(self) -> bool:
         """Check if this property should use a Component Property Type 
annotation.
 
@@ -77,27 +83,64 @@ class Property:
 
         Precondition: is_property_type_annotation() returned True.
         This method should always succeed if the precondition is met.
+        Unquotes literal values before using them in annotations.
         """
+        val = self._strip_quotes(self.value)
+        
         if self.name == 'service.ranking':
-            return f'@ServiceRanking({self.value})'
+            return f'@ServiceRanking({val})'
         elif self.name == 'service.description':
-            return f'@ServiceDescription("{self.value}")'
+            return f'@ServiceDescription("{val}")'
         elif self.name == 'service.vendor':
-            return f'@ServiceVendor("{self.value}")'
+            return f'@ServiceVendor("{val}")'
 
         # Should never reach here if precondition is met
         raise ValueError(f"Unknown property type annotation: {self.name}")
 
     def to_component_property(self) -> List[str]:
-        """Convert to @Component property format."""
+        """Convert to @Component property format.
+        
+        Constant references are formatted as string concatenation: 
"name="+CONSTANT
+        Literal values (with quotes stripped) are kept unquoted within the 
property string: "name=value"
+        """
         properties = []
         type_suffix = f":{self.type.value}" if self.type != 
PropertyType.STRING else ""
 
+        def is_reference(val: str) -> bool:
+            """Return True when the value is not a valid literal for this 
property type."""
+            val = val.strip()
+
+            if self.type == PropertyType.STRING:
+                return not val.startswith('"')
+
+            if self.type == PropertyType.BOOLEAN:
+                return not bool(re.fullmatch(r'true|false', val))
+
+            if self.type in (PropertyType.INTEGER, PropertyType.LONG):
+                return not bool(re.fullmatch(r'-?\d+[lL]?', val))
+
+            if self.type in (PropertyType.FLOAT, PropertyType.DOUBLE):
+                return not 
bool(re.fullmatch(r'-?(?:\d+\.\d*|\d*\.\d+|\d+)(?:[fFdD])?', val))
+
+            return True
+
         if self.values:
             for val in self.values:
-                properties.append(f'"{self.name}{type_suffix}={val}"')
+                if is_reference(val):
+                    # Reference - use string concatenation: "name="+CONSTANT
+                    properties.append(f'"{self.name}{type_suffix}="+{val}')
+                else:
+                    # Literal value - strip quotes and keep unquoted within 
property string
+                    stripped_val = self._strip_quotes(val)
+                    
properties.append(f'"{self.name}{type_suffix}={stripped_val}"')
         elif self.value is not None:
-            properties.append(f'"{self.name}{type_suffix}={self.value}"')
+            if is_reference(self.value):
+                # Reference - use string concatenation: "name="+CONSTANT
+                properties.append(f'"{self.name}{type_suffix}="+{self.value}')
+            else:
+                # Literal value - strip quotes and keep unquoted within 
property string
+                stripped_val = self._strip_quotes(self.value)
+                properties.append(f'"{self.name}{type_suffix}={stripped_val}"')
 
         return properties
 
@@ -385,11 +428,12 @@ class AnnotationMigrator:
             prop.type = PropertyType.DOUBLE
             prop.value = double_value_match.group(1)
         elif values_match:
-            # Array of values
+            # Array of values - preserve quotes on string literals
             values_str = values_match.group(1)
-            prop.values = [v.strip().strip('"') for v in values_str.split(',')]
+            prop.values = [v.strip() for v in values_str.split(',')]
         elif value_match:
-            prop.value = value_match.group(1)
+            # String value from regex - add quotes back to preserve as literal
+            prop.value = f'"{value_match.group(1)}"'
 
         if label_match:
             prop.label = label_match.group(1)
diff --git a/skills/osgi-scr-migrator/scripts/tests/test_migrate_annotations.py 
b/skills/osgi-scr-migrator/scripts/tests/test_migrate_annotations.py
index f15eb00d..7f7f69d5 100644
--- a/skills/osgi-scr-migrator/scripts/tests/test_migrate_annotations.py
+++ b/skills/osgi-scr-migrator/scripts/tests/test_migrate_annotations.py
@@ -23,7 +23,7 @@ from migrate_annotations import (
 class TestPropertyParsing(unittest.TestCase):
     """Test @Property annotation parsing."""
 
-    def test_simple_string_property(self):
+    def test_simple_string_literal_property(self):
         """Test parsing simple string property."""
         content = '''
 @Property(name = "service.vendor", value = "Apache Software Foundation")
@@ -35,7 +35,22 @@ class TestPropertyParsing(unittest.TestCase):
         self.assertEqual(len(migrator.component_properties), 1)
         prop = migrator.component_properties[0]
         self.assertEqual(prop.name, "service.vendor")
-        self.assertEqual(prop.value, "Apache Software Foundation")
+        self.assertEqual(prop.value, "\"Apache Software Foundation\"")
+        self.assertEqual(prop.type, PropertyType.STRING)
+
+    def test_simple_string_reference_property(self):
+        """Test parsing simple string property."""
+        content = '''
+@Property(name = "service.vendor", value = Constants.SERVICE_VENDOR)
+'''
+        stats = MigrationStats()
+        migrator = AnnotationMigrator(content, Path("test.java"), stats)
+        migrator._collect_properties()
+
+        self.assertEqual(len(migrator.component_properties), 1)
+        prop = migrator.component_properties[0]
+        self.assertEqual(prop.name, "service.vendor")
+        self.assertEqual(prop.value, "Constants.SERVICE_VENDOR")
         self.assertEqual(prop.type, PropertyType.STRING)
 
     def test_boolean_property(self):
@@ -81,7 +96,7 @@ class TestPropertyParsing(unittest.TestCase):
         prop = migrator.component_properties[0]
         self.assertEqual(prop.name, "paths")
         self.assertEqual(len(prop.values), 3)
-        self.assertEqual(prop.values, ["/path1", "/path2", "/path3"])
+        self.assertEqual(prop.values, ["\"/path1\"", "\"/path2\"", 
"\"/path3\""])
 
     def test_property_with_label(self):
         """Test parsing property with label and description."""
@@ -105,7 +120,7 @@ class TestPropertyConversion(unittest.TestCase):
 
     def test_simple_property_conversion(self):
         """Test converting simple property to component format."""
-        prop = Property(name="service.vendor", value="Apache Software 
Foundation")
+        prop = Property(name="service.vendor", value="\"Apache Software 
Foundation\"")
         result = prop.to_component_property()
 
         self.assertEqual(len(result), 1)
@@ -129,7 +144,7 @@ class TestPropertyConversion(unittest.TestCase):
 
     def test_array_property_conversion(self):
         """Test converting array property."""
-        prop = Property(name="paths", values=["/path1", "/path2"])
+        prop = Property(name="paths", values=["\"/path1\"", "\"/path2\""])
         result = prop.to_component_property()
 
         self.assertEqual(len(result), 2)

Reply via email to