DSL Items Parser: Fix incorrect parsing of keywords encountered in the wrong context

bundles/org.openhab.core.model.core/src/main/java/org/openhab/core/model/core/internal/ModelRepositoryImpl.java

@@ -330,7 +330,14 @@ private boolean validateModel(String name, InputStream inputStream, List<String>
                     final org.eclipse.emf.common.util.Diagnostic diagnostic = safeEmf
                             .call(() -> Diagnostician.INSTANCE.validate(resource.getContents().getFirst()));
                     for (org.eclipse.emf.common.util.Diagnostic d : diagnostic.getChildren()) {
-                        warnings.add(d.getMessage());
+                        if (d.getSeverity() == org.eclipse.emf.common.util.Diagnostic.ERROR) {
+                            errors.add(d.getMessage());
+                        } else {
+                            warnings.add(d.getMessage());
+                        }
+                    }
+                    if (!errors.isEmpty()) {
+                        return false;
                     }
                 } catch (NullPointerException e) {
                     // see https://github.com/eclipse/smarthome/issues/3335

bundles/org.openhab.core.model.item/bnd.bnd

@@ -22,6 +22,7 @@ Import-Package: javax.measure,\
  org.openhab.core.items,\
  org.openhab.core.items.dto,\
  org.openhab.core.items.fileconverter,\
+ org.openhab.core.library,\
  org.openhab.core.library.items,\
  org.openhab.core.library.types,\
  org.openhab.core.thing.util,\

bundles/org.openhab.core.model.item/src/org/openhab/core/model/Items.xtext

@@ -14,31 +14,18 @@ ItemModel:
 ;
 
 ModelItem:
-	(ModelNormalItem | ModelGroupItem) name=ID
+	type=ModelItemType ('(' args+=(ID|STRING) (',' args+=(ID|STRING))* ')')?
+	name=ID
 	(label=STRING)?
 	('<' icon=Icon '>')?
-	('(' groups+=ID (',' groups+=ID)* ')')? 
+	('(' groups+=ID (',' groups+=ID)* ')')?
 	('[' tags+=(ID|STRING) (',' tags+=(ID|STRING))* ']')?
-	('{' bindings+=ModelBinding (',' bindings+=ModelBinding)* '}')? 
-;
-
-ModelGroupItem:
-	{ModelGroupItem} 'Group' (':' type=ModelItemType ( ':' function=ModelGroupFunction ('(' args+=(ID|STRING) (',' args+=(ID|STRING))* ')')?)?)?
-;
-
-enum ModelGroupFunction:
-	EQUALITY='EQUALITY' | AND='AND' | OR='OR' | NAND='NAND' | NOR='NOR' | XOR='XOR' | AVG='AVG' | MEDIAN='MEDIAN' | SUM='SUM' | MAX='MAX' | MIN='MIN' | COUNT='COUNT' | LATEST='LATEST' | EARLIEST='EARLIEST'
-;
-
-ModelNormalItem:
-	type=ModelItemType
+	('{' bindings+=ModelBinding (',' bindings+=ModelBinding)* '}')?
 ;
 
+// Supports item types with up to 4 colon-separated segments, e.g., Group:Number:Dimension:MAX
 ModelItemType:
-	BaseModelItemType | ('Number' (':' ID)?)
-;
-BaseModelItemType:
-	'Switch' | 'Rollershutter' | 'String' | 'Dimmer' | 'Contact' | 'DateTime' | 'Color' | 'Player' | 'Location' | 'Call' | 'Image'
+   ID (':' ID (':' ID (':' ID )?)?)?
 ;
 
 ModelBinding:
@@ -56,7 +43,7 @@ ValueType returns ecore::EJavaObject:
     STRING | NUMBER | BOOLEAN
 ;
 
-BOOLEAN returns ecore::EBoolean: 
+BOOLEAN returns ecore::EBoolean:
     'true' | 'false'
 ;
 

bundles/org.openhab.core.model.item/src/org/openhab/core/model/formatting/ItemsFormatter.xtend

@@ -25,8 +25,10 @@ class ItemsFormatter extends AbstractDeclarativeFormatter {
 	@Inject extension ItemsGrammarAccess
 
 	override protected void configureFormatting(FormattingConfig c) {
-		c.setLinewrap(1, 1, 2).before(modelGroupItemRule)
-		c.setLinewrap(1, 1, 2).before(modelNormalItemRule)
+		c.setLinewrap(1, 1, 2).before(modelItemRule)
+
+		// No space between the item type and the opening parenthesis for the group function arguments
+		c.setNoSpace().between(modelItemTypeRule, modelItemAccess.leftParenthesisKeyword_1_0)
 
 		c.setNoSpace().withinKeywordPairs("<", ">")
 		c.setNoSpace().withinKeywordPairs("(", ")")

bundles/org.openhab.core.model.item/src/org/openhab/core/model/item/internal/GenericItemProvider.java

@@ -47,10 +47,7 @@
 import org.openhab.core.model.item.BindingConfigReader;
 import org.openhab.core.model.items.ItemModel;
 import org.openhab.core.model.items.ModelBinding;
-import org.openhab.core.model.items.ModelGroupFunction;
-import org.openhab.core.model.items.ModelGroupItem;
 import org.openhab.core.model.items.ModelItem;
-import org.openhab.core.model.items.ModelNormalItem;
 import org.openhab.core.types.StateDescriptionFragment;
 import org.openhab.core.types.StateDescriptionFragmentBuilder;
 import org.openhab.core.types.StateDescriptionFragmentProvider;
@@ -234,62 +231,54 @@ private void processBindingConfigsFromModel(String modelName, EventType type) {
     }
 
     private @Nullable Item createItemFromModelItem(ModelItem modelItem, String modelName) {
-        Item item;
-        if (modelItem instanceof ModelGroupItem modelGroupItem) {
-            Item baseItem;
-            try {
-                baseItem = createItemOfType(modelGroupItem.getType(), modelGroupItem.getName());
-            } catch (IllegalArgumentException e) {
-                logger.debug("Error creating base item for group item '{}', item will be ignored: {}",
-                        modelGroupItem.getName(), e.getMessage());
-                return null;
-            }
-            if (baseItem != null) {
-                // if the user did not specify a function the first value of the enum in xtext (EQUAL) will be used
-                ModelGroupFunction function = modelGroupItem.getFunction();
-                item = applyGroupFunction(baseItem, modelGroupItem, function);
-            } else {
-                item = new GroupItem(modelGroupItem.getName());
-            }
-        } else {
-            ModelNormalItem normalItem = (ModelNormalItem) modelItem;
-            try {
-                item = createItemOfType(normalItem.getType(), normalItem.getName());
-            } catch (IllegalArgumentException e) {
-                logger.debug("Error creating item '{}', item will be ignored: {}", normalItem.getName(),
-                        e.getMessage());
-                return null;
-            }
+        String itemType = modelItem.getType();
+        if (itemType == null || itemType.isBlank()) {
+            logger.warn("Item '{}' has no type defined, ignoring it.", modelItem.getName());
+            return null;
         }
-        if (item instanceof ActiveItem activeItem) {
-            String label = modelItem.getLabel();
-            String format = extractFormat(label);
-            if (format != null) {
-                label = label.substring(0, label.indexOf("[")).trim();
-                Map<String, String> formatters = Objects
-                        .requireNonNull(stateFormattersMap.computeIfAbsent(modelName, k -> new HashMap<>()));
-                formatters.put(modelItem.getName(), format);
-                if (!isIsolatedModel(modelName)) {
-                    stateDescriptionFragments.put(modelItem.getName(),
-                            StateDescriptionFragmentBuilder.create().withPattern(format).build());
-                }
-            } else {
-                Map<String, String> formatters = stateFormattersMap.get(modelName);
-                if (formatters != null) {
-                    formatters.remove(modelItem.getName());
-                    if (formatters.isEmpty()) {
-                        stateFormattersMap.remove(modelName);
+
+        try {
+            String[] itemTypeSegments = itemType.split(ItemUtil.EXTENSION_SEPARATOR);
+            String mainItemType = itemTypeSegments[0];
+
+            Item item = switch (mainItemType) {
+                case "Group" -> createGroupItem(modelItem, itemTypeSegments);
+                default -> createItemOfType(itemType, modelItem.getName());
+            };
+
+            if (item instanceof ActiveItem activeItem) {
+                String label = modelItem.getLabel();
+                String format = extractFormat(label);
+                if (format != null) {
+                    label = label.substring(0, label.indexOf("[")).trim();
+                    Map<String, String> formatters = Objects
+                            .requireNonNull(stateFormattersMap.computeIfAbsent(modelName, k -> new HashMap<>()));
+                    formatters.put(modelItem.getName(), format);
+                    if (!isIsolatedModel(modelName)) {
+                        stateDescriptionFragments.put(modelItem.getName(),
+                                StateDescriptionFragmentBuilder.create().withPattern(format).build());
+                    }
+                } else {
+                    Map<String, String> formatters = stateFormattersMap.get(modelName);
+                    if (formatters != null) {
+                        formatters.remove(modelItem.getName());
+                        if (formatters.isEmpty()) {
+                            stateFormattersMap.remove(modelName);
+                        }
+                    }
+                    if (!isIsolatedModel(modelName)) {
+                        stateDescriptionFragments.remove(modelItem.getName());
                     }
                 }
-                if (!isIsolatedModel(modelName)) {
-                    stateDescriptionFragments.remove(modelItem.getName());
-                }
+                activeItem.setLabel(label);
+                activeItem.setCategory(modelItem.getIcon());
+                assignTags(modelItem, activeItem);
+                return item;
+            } else {
+                return null;
             }
-            activeItem.setLabel(label);
-            activeItem.setCategory(modelItem.getIcon());
-            assignTags(modelItem, activeItem);
-            return item;
-        } else {
+        } catch (IllegalArgumentException e) {
+            logger.debug("Error creating item '{}', item will be ignored: {}", modelItem.getName(), e.getMessage());
             return null;
         }
     }
@@ -312,14 +301,14 @@ private void assignTags(ModelItem modelItem, ActiveItem item) {
         }
     }
 
-    private GroupItem applyGroupFunction(Item baseItem, ModelGroupItem modelGroupItem, ModelGroupFunction function) {
+    private GroupItem applyGroupFunction(Item baseItem, ModelItem modelItem, String function) {
         GroupFunctionDTO dto = new GroupFunctionDTO();
-        dto.name = function.getName();
-        dto.params = modelGroupItem.getArgs().toArray(new String[0]);
+        dto.name = function;
+        dto.params = modelItem.getArgs().toArray(new String[0]);
 
         GroupFunction groupFunction = ItemDTOMapper.mapFunction(baseItem, dto);
 
-        return new GroupItem(modelGroupItem.getName(), baseItem, groupFunction);
+        return new GroupItem(modelItem.getName(), baseItem, groupFunction);
     }
 
     private void dispatchBindingsPerItemType(String[] itemTypes) {
@@ -529,6 +518,47 @@ private Map<String, Item> toItemMap(@Nullable Collection<Item> items) {
         return ret;
     }
 
+    /**
+     * Creates a new GroupItem based on the given ModelItem and item type segments.
+     *
+     * @param modelItem The ModelItem to create the GroupItem from.
+     * @param itemTypeSegments The segments of the item type.
+     * @return A new GroupItem or null if the item type is invalid.
+     */
+    private @Nullable GroupItem createGroupItem(ModelItem modelItem, String[] itemTypeSegments) {
+        if (itemTypeSegments.length == 1) {
+            // Just plain "Group" with no base type
+            return new GroupItem(modelItem.getName());
+        }
+
+        String function = GroupFunction.DEFAULT;
+
+        String baseItemType = switch (itemTypeSegments.length) {
+            case 2 -> itemTypeSegments[1];
+            case 3 -> {
+                // 3 segments could either be Group:Type:Function, or Group:Number:Dimension -> Find out which one it is
+                if (!modelItem.getArgs().isEmpty() || GroupFunction.VALID_FUNCTIONS.contains(itemTypeSegments[2])) {
+                    // It's Group:Type:Function because there are arguments or the third segment is a valid function
+                    function = itemTypeSegments[2];
+                    yield itemTypeSegments[1];
+                } else {
+                    // Otherwise, it must be Group:Number:Dimension
+                    yield itemTypeSegments[1] + ItemUtil.EXTENSION_SEPARATOR + itemTypeSegments[2];
+                }
+            }
+            case 4 -> {
+                // 4 segments: "Group:Number:Dimension:Function"
+                function = itemTypeSegments[3];
+                yield itemTypeSegments[1] + ItemUtil.EXTENSION_SEPARATOR + itemTypeSegments[2];
+            }
+            default -> throw new IllegalArgumentException("Invalid group item type: " + modelItem.getType()
+                    + ". Expected formats are 'Group', 'Group:Type', 'Group:Type:Function', or 'Group:Number:Dimension:Function' with a maximum of 4 segments.");
+        };
+
+        Item baseItem = createItemOfType(baseItemType, modelItem.getName());
+        return applyGroupFunction(baseItem, modelItem, function);
+    }
+
     /**
      * Creates a new item of type {@code itemType} by utilizing an appropriate {@link ItemFactory}.
      *

bundles/org.openhab.core.model.item/src/org/openhab/core/model/item/internal/fileconverter/DslItemFileConverter.java

@@ -35,6 +35,7 @@
 import org.openhab.core.items.GroupFunction;
 import org.openhab.core.items.GroupItem;
 import org.openhab.core.items.Item;
+import org.openhab.core.items.ItemUtil;
 import org.openhab.core.items.Metadata;
 import org.openhab.core.items.fileconverter.AbstractItemFileGenerator;
 import org.openhab.core.items.fileconverter.ItemFileGenerator;
@@ -45,8 +46,6 @@
 import org.openhab.core.model.items.ItemModel;
 import org.openhab.core.model.items.ItemsFactory;
 import org.openhab.core.model.items.ModelBinding;
-import org.openhab.core.model.items.ModelGroupFunction;
-import org.openhab.core.model.items.ModelGroupItem;
 import org.openhab.core.model.items.ModelItem;
 import org.openhab.core.model.items.ModelProperty;
 import org.openhab.core.types.State;
@@ -115,26 +114,24 @@ public void generateFileFormat(String id, OutputStream out) {
 
     private ModelItem buildModelItem(Item item, List<Metadata> channelLinks, List<Metadata> metadata,
             @Nullable String stateFormatter, boolean hideDefaultParameters) {
-        ModelItem model;
+        ModelItem model = ItemsFactory.eINSTANCE.createModelItem();
         if (item instanceof GroupItem groupItem) {
-            ModelGroupItem modelGroup = ItemsFactory.eINSTANCE.createModelGroupItem();
-            model = modelGroup;
             Item baseItem = groupItem.getBaseItem();
+            List<String> groupType = new ArrayList<>();
+            groupType.add(groupItem.getType());
             if (baseItem != null) {
-                modelGroup.setType(baseItem.getType());
+                groupType.add(baseItem.getType());
                 GroupFunction function = groupItem.getFunction();
                 if (function != null) {
-                    ModelGroupFunction modelFunction = ModelGroupFunction
-                            .getByName(function.getClass().getSimpleName().toUpperCase());
-                    modelGroup.setFunction(modelFunction);
+                    groupType.add(function.getClass().getSimpleName().toUpperCase());
                     State[] parameters = function.getParameters();
                     for (int i = 0; i < parameters.length; i++) {
-                        modelGroup.getArgs().add(parameters[i].toString());
+                        model.getArgs().add(parameters[i].toString());
                     }
                 }
             }
+            model.setType(groupType.stream().collect(Collectors.joining(ItemUtil.EXTENSION_SEPARATOR)));
         } else {
-            model = ItemsFactory.eINSTANCE.createModelNormalItem();
             model.setType(item.getType());
         }