Skip to content

Add line number to validation errors/warnings in DSL thing provider #5433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lolodomo wants to merge 1 commit into
base: main
Choose a base branch
from
+25 −8
Open

Add line number to validation errors/warnings in DSL thing provider #5433

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions ...openhab.core.model.thing/src/org/openhab/core/model/thing/validation/ThingValidator.xtend
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@
*/
package org.openhab.core.model.thing.validation

import org.openhab.core.model.thing.thing.ModelThing
import org.eclipse.emf.ecore.EObject
import org.eclipse.xtext.nodemodel.util.NodeModelUtils
import org.eclipse.xtext.validation.Check
import org.openhab.core.model.thing.thing.ModelThing
import org.openhab.core.model.thing.thing.ThingPackage
import org.eclipse.xtext.nodemodel.util.NodeModelUtils
import org.openhab.core.thing.ThingUID

/**
Expand All @@ -33,18 +34,19 @@ class ThingValidator extends AbstractThingValidator {
@Check
def check_thing_has_valid_id(ModelThing thing) {
if (thing.nested) {
val warnMsg = buildMsgWithLineNb(thing, "Provide a thing type ID and a thing ID in this format:\n <thingTypeId> <thingId>")
// We have to provide thingTypeId and a thingId
if (!thing.eIsSet(ThingPackage.Literals.MODEL_THING__THING_TYPE_ID)) {
if (thing.eIsSet(ThingPackage.Literals.MODEL_PROPERTY_CONTAINER__ID)) {
warning("Provide a thing type ID and a thing ID in this format:\n <thingTypeId> <thingId>", ThingPackage.Literals.MODEL_PROPERTY_CONTAINER__ID)
warning(warnMsg, thing, ThingPackage.Literals.MODEL_PROPERTY_CONTAINER__ID)
} else {
if (thing.eIsSet(ThingPackage.Literals.MODEL_BRIDGE__BRIDGE)) {
warning("Provide a thing type ID and a thing ID in this format:\n <thingTypeId> <thingId>", ThingPackage.Literals.MODEL_BRIDGE__BRIDGE)
warning(warnMsg, thing, ThingPackage.Literals.MODEL_BRIDGE__BRIDGE)
}
Comment on lines +37 to 45
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For these nested-thing warnings you attach the marker to a specific feature (MODEL_PROPERTY_CONTAINER__ID, MODEL_BRIDGE__BRIDGE, etc.), but the "Line …" prefix is computed from NodeModelUtils.getNode(thing) (the entire thing). If the relevant feature is on a different line than the thing declaration, the reported line number can be misleading. Consider computing the line from the node(s) for the same feature you pass to warning(...) (or from getMessageAcceptor() offset/length) so the prefix matches the actual marker location.

Copilot uses AI. Check for mistakes.
}
} else {
if (!thing.eIsSet(ThingPackage.Literals.MODEL_THING__THING_ID)) {
warning("Provide a thing type ID and a thing ID in this format:\n <thingTypeId> <thingId>", ThingPackage.Literals.MODEL_THING__THING_TYPE_ID)
warning(warnMsg, thing, ThingPackage.Literals.MODEL_THING__THING_TYPE_ID)
}
}
} else { // thing in container
Expand All @@ -53,21 +55,36 @@ class ThingValidator extends AbstractThingValidator {
val thingIdFeature = NodeModelUtils.findNodesForFeature(thing, ThingPackage.Literals.MODEL_THING__THING_ID).head
val startOffset = thingTypeIdFeature.offset
val endOffset = thingIdFeature.endOffset
getMessageAcceptor().acceptWarning("Provide a thing UID in this format:\n <bindingId>:<thingTypeId>:<thingId>", thing, startOffset, endOffset - startOffset, null, null)
getMessageAcceptor().acceptWarning(
buildMsgWithLineNb(thing, "Provide a thing UID in this format:\n <bindingId>:<thingTypeId>:<thingId>"),
Comment on lines +58 to +59
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line numbers added via buildMsgWithLineNb(thing, …) are derived from the whole ModelThing node, but the warning here highlights only the thingTypeId..thingId range (startOffset/endOffset from the feature nodes). If the thing spans multiple lines, the message’s "Line …" prefix can disagree with the actual marked location. Consider deriving the line(s) from the same node(s)/offset range you pass to acceptWarning (e.g., using the feature nodes’ start/end line), or passing the relevant INode into the helper so the prefix matches the highlighted region.

Suggested change
getMessageAcceptor().acceptWarning(
buildMsgWithLineNb(thing, "Provide a thing UID in this format:\n <bindingId>:<thingTypeId>:<thingId>"),
val baseMsg = "Provide a thing UID in this format:\n <bindingId>:<thingTypeId>:<thingId>"
val startLine = thingTypeIdFeature.startLine
val endLine = thingIdFeature.endLine
val warnMsg = if (startLine == endLine) {
"Line " + startLine + ": " + baseMsg
} else {
"Line " + startLine + "-" + endLine + ": " + baseMsg
}
getMessageAcceptor().acceptWarning(
warnMsg,

Copilot uses AI. Check for mistakes.
thing, startOffset, endOffset - startOffset, null, null)
} else {
if (thing.id !== null) {
try {
new ThingUID(thing.id)
} catch (IllegalArgumentException e) {
error(e.message, ThingPackage.Literals.MODEL_PROPERTY_CONTAINER__ID)
error(buildMsgWithLineNb(thing, e.message), thing, ThingPackage.Literals.MODEL_PROPERTY_CONTAINER__ID)
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error marker is attached to MODEL_PROPERTY_CONTAINER__ID, but the line number prefix is computed from the whole thing node. If the id appears on a different line than the start of the thing declaration, the reported line can be inaccurate. Consider computing the line number from the node for the ID feature (or the acceptor offset) instead of from NodeModelUtils.getNode(thing).

Copilot uses AI. Check for mistakes.
}
}
}
}

}

def private isNested(ModelThing thing) {
thing.eContainingFeature == ThingPackage.Literals.MODEL_BRIDGE__THINGS
}

def private buildMsgWithLineNb(EObject object, String msg) {
val node = NodeModelUtils.getNode(object)
if (node === null) {
return msg
}
val startLine = node.startLine
val endLine = node.endLine
if (startLine == endLine) {
return "Line " + startLine + ": " + msg
} else {
return "Line " + startLine + "-" + endLine + ": " + msg
}
}
}
Loading