From e1e01458f2bc63ec434c1c19b411f7292c01ac6a Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Fri, 10 Apr 2026 09:49:26 +0200 Subject: [PATCH 01/17] feat: add sap-description-column-label ESLint rule --- .changeset/rich-meals-shake.md | 5 + packages/eslint-plugin-fiori-tools/README.md | 3 +- .../rules/sap-description-column-label.md | 107 ++++++ .../src/constants.ts | 3 +- .../eslint-plugin-fiori-tools/src/index.ts | 1 + .../src/language/diagnostics.ts | 19 + .../src/rules/index.ts | 3 + .../src/rules/sap-description-column-label.ts | 330 ++++++++++++++++++ .../sap-description-column-label.test.ts | 138 ++++++++ 9 files changed, 607 insertions(+), 2 deletions(-) create mode 100644 .changeset/rich-meals-shake.md create mode 100644 packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md create mode 100644 packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts create mode 100644 packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts diff --git a/.changeset/rich-meals-shake.md b/.changeset/rich-meals-shake.md new file mode 100644 index 00000000000..67ecaa63f29 --- /dev/null +++ b/.changeset/rich-meals-shake.md @@ -0,0 +1,5 @@ +--- +'@sap-ux/eslint-plugin-fiori-tools': minor +--- + +[rule] Add rule to check that a Common.Text description property has a meaningful Common.Label diff --git a/packages/eslint-plugin-fiori-tools/README.md b/packages/eslint-plugin-fiori-tools/README.md index 81b5386f06c..b4d8f6a0aa0 100644 --- a/packages/eslint-plugin-fiori-tools/README.md +++ b/packages/eslint-plugin-fiori-tools/README.md @@ -120,7 +120,8 @@ npx --yes @sap-ux/create@latest convert eslint-config --help | Since | Rule | Description | Recommended | Recommended for S/4HANA | |:---------:|------|-------------|:-----------:|:-----------------------:| -| 9.11.0 | [sap-no-data-field-intent-based-navigation](docs/rules/sap-no-data-field-intent-based-navigation.md) | Ensures neither `DataFieldForIntentBasedNavigation` nor `DataFieldWithIntentBasedNavigation` are used in tables or form fields in SAP Fiori elements applications. | | ✅ | +| new | [sap-description-column-label](docs/rules/sap-description-column-label.md) | Ensures that the description (text) property referenced via `Common.Text` has a meaningful `Common.Label` — not a generic value such as `"Name"` or `"Description"`, and not the same label as the ID property. | | ✅ | +| 9.11.0 | [sap-no-data-field-intent-based-navigation](docs/rules/sap-no-data-field-intent-based-navigation.md) | Ensures neither `DataFieldForIntentBasedNavigation` nor `DataFieldWithIntentBasedNavigation` are used in tables or form fields in SAP Fiori elements applications. | | ✅ | | 9.10.0 | [sap-condensed-table-layout](docs/rules/sap-condensed-table-layout.md) | Requires `condensedTableLayout` to be enabled when using a grid table, analytical table, or tree table. | | ✅ | | 9.9.0 | [sap-strict-uom-filtering](docs/rules/sap-strict-uom-filtering.md) | Ensures that `disableStrictUomFiltering` is not set to `true` in `sap.fe.app` manifest configuration | | ✅ | | 9.8.0 | [sap-table-personalization](docs/rules/sap-table-personalization.md) | Ensures that all table `personalization` options are enabled in the OData V4 applications. | | ✅ | diff --git a/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md b/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md new file mode 100644 index 00000000000..3f26ebbdd2e --- /dev/null +++ b/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md @@ -0,0 +1,107 @@ +# Require Meaningful Labels for Description (Text) Properties (`sap-description-column-label`) + +Ensures that the property referenced as the description (text) value via `Common.Text` has a meaningful `Common.Label` — not a generic value such as `"Name"` or `"Description"`, and not the same label as the ID property it describes. + +## Rule Details + +When a field uses `Common.Text` to associate a human-readable description property with a technical key property, the description property should carry a label that clearly identifies its content. A generic label like `"Name"` or `"Description"` provides no useful context to the user in the UI (for example, as a column header in a table), and a label that is identical to the ID property's label creates ambiguity between the two columns. + +The rule checks every `Common.Text` annotation on a property that belongs to an entity type used in the application (i.e. linked to at least one page via the manifest). For each such annotation the rule: + +1. Resolves the path to the referenced text property (including navigation segments such as `category/name`). +2. Reads the `Common.Label` annotation on that property. +3. Flags the annotation if the label is `"Name"` or `"Description"` (case-insensitive) — **trivialLabel**. +4. Flags the annotation if the label is identical (case-insensitive) to the `Common.Label` of the ID property that carries `Common.Text` — **duplicateLabel**. + +If the text property has no `Common.Label`, the rule does not flag it. + +### Why Was This Rule Introduced? + +When a property is displayed using a description column (e.g. `TextArrangement/TextOnly`), the column header comes from the `Common.Label` of the text property. A label of `"Name"` or `"Description"` is semantically meaningless in that context — it does not tell the user _which_ name or description is shown. Similarly, if the description column carries the exact same label as the ID column, the user cannot distinguish the two columns. + +### Warning Messages + +**trivialLabel** +``` +The text property "{{textPropertyTarget}}" has a generic label "{{textPropertyLabel}}". Use a more descriptive label that distinguishes it from other properties. +``` + +**duplicateLabel** +``` +The text property "{{textPropertyTarget}}" has the same label "{{textPropertyLabel}}" as the ID property "{{idPropertyTarget}}". The description column label should be different from the ID label. +``` + +## Examples + +### Incorrect Annotations + +**`trivialLabel` — the text property label is too generic:** + +```xml + + + + + + + + + +``` + +**`duplicateLabel` — the text property shares its label with the ID property:** + +```xml + + + + + + + + + + +``` + +### Correct Annotations + +**Meaningful, distinct label for the text property:** + +```xml + + + + + + + + +``` + +**Different labels for ID and text properties:** + +```xml + + + + + + + + +``` + +## Bug Report + +If you encounter an issue with this rule, please open a [GitHub issue](https://github.com/SAP/open-ux-tools/issues). + +## When Not to Disable This Rule + +This rule should not be disabled unless you have a confirmed backend constraint that prevents renaming the label. In most cases the fix is straightforward: update the `Common.Label` on the text property to something meaningful and distinct from the ID property label. + +## Further Reading + +- [UI5 Further Features of the Field - OData V4](https://ui5.sap.com/#/topic/f49a0f7eaafe444daf4cd62d48120ad0) +- [UI5 Displaying Text and ID for Value Help Input Fields - OData V2](https://ui5.sap.com/#/topic/080886d8d4af4ac6a68a476beab17da3) +- [OData Common Vocabulary - Label](https://github.com/SAP/odata-vocabularies/blob/main/vocabularies/Common.md#Label) diff --git a/packages/eslint-plugin-fiori-tools/src/constants.ts b/packages/eslint-plugin-fiori-tools/src/constants.ts index 5813903b2f9..9c3d5b34a46 100644 --- a/packages/eslint-plugin-fiori-tools/src/constants.ts +++ b/packages/eslint-plugin-fiori-tools/src/constants.ts @@ -1,3 +1,4 @@ export const UI_LINE_ITEM = 'com.sap.vocabularies.UI.v1.LineItem'; - +export const COMMON_TEXT = 'com.sap.vocabularies.Common.v1.Text'; +export const COMMON_LABEL = 'com.sap.vocabularies.Common.v1.Label'; export const UI_FIELD_GROUP = 'com.sap.vocabularies.UI.v1.FieldGroup'; diff --git a/packages/eslint-plugin-fiori-tools/src/index.ts b/packages/eslint-plugin-fiori-tools/src/index.ts index 9feaeccd6d1..22cf9a2a5e4 100644 --- a/packages/eslint-plugin-fiori-tools/src/index.ts +++ b/packages/eslint-plugin-fiori-tools/src/index.ts @@ -472,6 +472,7 @@ const fioriLanguageConfig: Linter.Config[] = [ '@sap-ux/fiori-tools/sap-flex-enabled': 'warn', '@sap-ux/fiori-tools/sap-width-including-column-header': 'warn', '@sap-ux/fiori-tools/sap-copy-to-clipboard': 'warn', + '@sap-ux/fiori-tools/sap-description-column-label': 'warn', '@sap-ux/fiori-tools/sap-enable-export': 'warn', '@sap-ux/fiori-tools/sap-enable-paste': 'warn', '@sap-ux/fiori-tools/sap-creation-mode-for-table': 'warn', diff --git a/packages/eslint-plugin-fiori-tools/src/language/diagnostics.ts b/packages/eslint-plugin-fiori-tools/src/language/diagnostics.ts index db37069d9c6..a93cc855bd5 100644 --- a/packages/eslint-plugin-fiori-tools/src/language/diagnostics.ts +++ b/packages/eslint-plugin-fiori-tools/src/language/diagnostics.ts @@ -15,6 +15,7 @@ export const TABLE_COLUMN_VERTICAL_ALIGNMENT = 'sap-table-column-vertical-alignm export const NO_DATA_FIELD_INTENT_BASED_NAVIGATION = 'sap-no-data-field-intent-based-navigation'; export const CONDENSED_TABLE_LAYOUT = 'sap-condensed-table-layout'; export const STRICT_UOM_FILTERING = 'sap-strict-uom-filtering'; +export const DESCRIPTION_COLUMN_LABEL = 'sap-description-column-label'; export interface WidthIncludingColumnHeaderDiagnostic { type: typeof WIDTH_INCLUDING_COLUMN_HEADER_RULE_TYPE; @@ -137,12 +138,30 @@ export interface StrictUomFiltering { manifest: ManifestPropertyDiagnosticData; } +export type DescriptionColumnLabelMessageId = + | 'trivialLabel' // label is "Name" or "Description" + | 'duplicateLabel'; // label of text property matches label of ID property + +export interface DescriptionColumnLabel { + type: typeof DESCRIPTION_COLUMN_LABEL; + messageId: DescriptionColumnLabelMessageId; + pageNames: string[]; + annotation: { + reference: AnnotationReference; + idPropertyTarget: string; + textPropertyTarget: string; + textPropertyLabel: string; + idPropertyLabel?: string; + }; +} + export type Diagnostic = | WidthIncludingColumnHeaderDiagnostic | AnchorBarVisible | FlexEnabled | CopyToClipboard | CreationModeForTable + | DescriptionColumnLabel | EnableExport | EnablePaste | StatePreservationMode diff --git a/packages/eslint-plugin-fiori-tools/src/rules/index.ts b/packages/eslint-plugin-fiori-tools/src/rules/index.ts index 993ce4b7bba..e473533c4a1 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/index.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/index.ts @@ -6,6 +6,7 @@ import { WIDTH_INCLUDING_COLUMN_HEADER_RULE_TYPE, COPY_TO_CLIPBOARD, CREATION_MODE_FOR_TABLE, + DESCRIPTION_COLUMN_LABEL, ENABLE_EXPORT, ENABLE_PASTE, STATE_PRESERVATION_MODE, @@ -73,6 +74,7 @@ import creationModeForTable from './sap-creation-mode-for-table'; import statePreservationMode from './sap-state-preservation-mode'; import strictUomFilteringRule from './sap-strict-uom-filtering'; import copyToClipboard from './sap-copy-to-clipboard'; +import descriptionColumnLabel from './sap-description-column-label'; import enableExport from './sap-enable-export'; import enablePaste from './sap-enable-paste'; import tablePersonalization from './sap-table-personalization'; @@ -135,6 +137,7 @@ export const rules: Record child.name === segment); + if (!navProp?.structuredType) { + return undefined; + } + currentEntityTypeName = navProp.structuredType; + } + return { entityTypeName: currentEntityTypeName, propertyName }; +} + +/** + * Reads a Common.Label String value from the annotation index. + * + * @param propertyTarget - Fully-qualified target path (e.g. "Service.Entity/prop") + * @param service - The parsed OData service + * @returns The label string, or undefined if not found + */ +function getLabelForProperty(propertyTarget: string, service: ParsedService): string | undefined { + const labelKey = buildAnnotationIndexKey(propertyTarget, COMMON_LABEL); + const labelAnnotations = service.index.annotations[labelKey]; + if (!labelAnnotations) { + return undefined; + } + const annotation = Object.values(labelAnnotations)[0]; + if (!annotation) { + return undefined; + } + return getElementAttributeValue(annotation.top.value, Edm.String) || undefined; +} + +/** + * Builds a reverse map from IndexedAnnotation to entity type name, + * covering only entity-type level annotations (non-property targets). + * + * @param parsedService - The parsed OData service + * @returns Map from annotation object to its entity type name + */ +function buildAnnotationEntityTypeMap(parsedService: ParsedService): Map { + const map = new Map(); + for (const [key, qualifiedAnnotations] of Object.entries(parsedService.index.annotations)) { + const atIdx = key.indexOf('/@'); + if (atIdx === -1) { + continue; + } + const targetPath = key.substring(0, atIdx); + if (targetPath.includes('/')) { + continue; + } + for (const annotation of Object.values(qualifiedAnnotations)) { + map.set(annotation, targetPath); + } + } + return map; +} + +/** + * Collects the entity type names that are actually used on pages in the app. + * + * @param pages - Pages from the linked app model + * @param parsedService - The parsed OData service + * @returns Map from fully-qualified entity type name to the page target names it appears on + */ +function collectRelevantEntityTypes(pages: AnyPage[], parsedService: ParsedService): Map { + const entityTypePages = new Map(); + const annotationEntityTypeMap = buildAnnotationEntityTypeMap(parsedService); + + const addEntityType = (entityTypeName: string, pageName: string) => { + const pageNames = entityTypePages.get(entityTypeName) ?? []; + if (!pageNames.includes(pageName)) { + pageNames.push(pageName); + } + entityTypePages.set(entityTypeName, pageNames); + }; + + for (const page of pages) { + if (page.entity?.structuredType) { + addEntityType(page.entity.structuredType, page.targetName); + } + for (const table of page.lookup['table'] ?? []) { + if (table.type !== 'table' || !table.annotation) { + continue; + } + const entityType = annotationEntityTypeMap.get(table.annotation.annotation); + if (entityType) { + addEntityType(entityType, page.targetName); + } + } + } + return entityTypePages; +} + +/** + * Checks a single Common.Text annotation for a non-descriptive text property label. + * + * @param textAnnotation - The indexed Common.Text annotation + * @param entityTypeName - Entity type that owns the annotated property + * @param targetPath - Annotation target (the ID property path) + * @param pageNames - Pages where the entity type is used + * @param parsedService - The parsed OData service + * @returns Diagnostics found, if any + */ +function processTextAnnotation( + textAnnotation: IndexedAnnotation, + entityTypeName: string, + targetPath: string, + pageNames: string[], + parsedService: ParsedService +): DescriptionColumnLabel[] { + const textElement = textAnnotation.top.value; + const textPath = getElementAttributeValue(textElement, Edm.Path); + if (!textPath) { + return []; + } + + const resolved = resolveTextPropertyPath(entityTypeName, textPath, parsedService); + if (!resolved) { + return []; + } + + const textPropertyTarget = `${resolved.entityTypeName}/${resolved.propertyName}`; + const textPropertyLabel = getLabelForProperty(textPropertyTarget, parsedService); + if (!textPropertyLabel) { + return []; + } + + const labelLower = textPropertyLabel.trim().toLowerCase(); + + // Check 1: trivial generic labels + if (TRIVIAL_LABELS.has(labelLower)) { + return [ + { + type: DESCRIPTION_COLUMN_LABEL, + messageId: 'trivialLabel', + pageNames, + annotation: { + reference: textAnnotation.top, + idPropertyTarget: targetPath, + textPropertyTarget, + textPropertyLabel + } + } + ]; + } + + // Check 2: label of text property identical to label of ID property + const idPropertyLabel = getLabelForProperty(targetPath, parsedService); + if (idPropertyLabel?.trim().toLowerCase() === labelLower) { + return [ + { + type: DESCRIPTION_COLUMN_LABEL, + messageId: 'duplicateLabel', + pageNames, + annotation: { + reference: textAnnotation.top, + idPropertyTarget: targetPath, + textPropertyTarget, + textPropertyLabel, + idPropertyLabel + } + } + ]; + } + + return []; +} + +/** + * Processes one annotation index entry and returns diagnostics. + */ +function processAnnotationEntry( + annotationKey: string, + qualifiedAnnotations: { [qualifier: string]: IndexedAnnotation }, + parsedService: ParsedService, + relevantEntityTypes: Map +): DescriptionColumnLabel[] { + const atIdx = annotationKey.indexOf('/@'); + if (atIdx === -1) { + return []; + } + const targetPath = annotationKey.substring(0, atIdx); + const term = annotationKey.substring(atIdx + 2); + + if (term !== COMMON_TEXT) { + return []; + } + const slashIdx = targetPath.indexOf('/'); + if (slashIdx === -1) { + return []; + } + const entityTypeName = targetPath.substring(0, slashIdx); + const pageNames = relevantEntityTypes.get(entityTypeName); + if (!pageNames) { + return []; + } + + const problems: DescriptionColumnLabel[] = []; + for (const textAnnotation of Object.values(qualifiedAnnotations)) { + problems.push(...processTextAnnotation(textAnnotation, entityTypeName, targetPath, pageNames, parsedService)); + } + return problems; +} + +/** + * Collects all DescriptionColumnLabel diagnostics for a single parsed OData service. + */ +function collectProblemsForService( + parsedService: ParsedService, + relevantEntityTypes: Map +): DescriptionColumnLabel[] { + const problems: DescriptionColumnLabel[] = []; + for (const [annotationKey, qualifiedAnnotations] of Object.entries(parsedService.index.annotations)) { + problems.push( + ...processAnnotationEntry(annotationKey, qualifiedAnnotations, parsedService, relevantEntityTypes) + ); + } + return problems; +} + +const rule: FioriRuleDefinition = createFioriRule({ + ruleId: DESCRIPTION_COLUMN_LABEL, + meta: { + type: 'problem', + docs: { + recommended: true, + description: + 'The description (text) property referenced via Common.Text must have a meaningful label — not a generic value such as "Name" or "Description", and not the same label as the ID property it describes.', + url: 'https://github.com/SAP/open-ux-tools/blob/main/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md' + }, + messages: { + trivialLabel: + 'The text property "{{textPropertyTarget}}" has a generic label "{{textPropertyLabel}}". Use a more descriptive label that distinguishes it from other properties.', + duplicateLabel: + 'The text property "{{textPropertyTarget}}" has the same label "{{textPropertyLabel}}" as the ID property "{{idPropertyTarget}}". The description column label should be different from the ID label.' + } + }, + + check(context) { + if (!context.sourceCode.projectContext) { + return []; + } + const problems: DescriptionColumnLabel[] = []; + for (const [appKey, app] of Object.entries(context.sourceCode.projectContext.linkedModel.apps)) { + const parsedApp = context.sourceCode.projectContext.index.apps[appKey]; + const parsedService = context.sourceCode.projectContext.getIndexedServiceForMainService(parsedApp); + if (!parsedService) { + continue; + } + const relevantEntityTypes = collectRelevantEntityTypes(app.pages, parsedService); + problems.push(...collectProblemsForService(parsedService, relevantEntityTypes)); + } + return problems; + }, + + createAnnotations(context, validationResult) { + if (validationResult.length === 0) { + return {}; + } + const lookup = new Map(); + for (const diagnostic of validationResult) { + lookup.set(diagnostic.annotation.reference.value, diagnostic); + } + return { + ['target>element[name="Annotation"]'](node) { + const diagnostic = lookup.get(node); + if (!diagnostic) { + return; + } + context.report({ + node, + messageId: diagnostic.messageId as DescriptionColumnLabelMessageId, + data: { + textPropertyTarget: diagnostic.annotation.textPropertyTarget, + textPropertyLabel: diagnostic.annotation.textPropertyLabel, + idPropertyTarget: diagnostic.annotation.idPropertyTarget, + idPropertyLabel: diagnostic.annotation.idPropertyLabel ?? '' + } + }); + } + }; + } +}); + +export default rule; diff --git a/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts b/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts new file mode 100644 index 00000000000..d0ba8980c06 --- /dev/null +++ b/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts @@ -0,0 +1,138 @@ +import { RuleTester } from 'eslint'; +import descriptionColumnLabelRule from '../../src/rules/sap-description-column-label'; +import { meta, languages } from '../../src/index'; +import { getAnnotationsAsXmlCode, setup, V4_ANNOTATIONS, V4_ANNOTATIONS_PATH } from '../test-helper'; + +const ruleTester = new RuleTester({ + plugins: { ['@sap-ux/eslint-plugin-fiori-tools']: { ...meta, languages } }, + language: '@sap-ux/eslint-plugin-fiori-tools/fiori' +}); + +const TEST_NAME = 'sap-description-column-label'; +const { createValidTest, createInvalidTest } = setup(TEST_NAME); + +// ─── Annotation snippets ───────────────────────────────────────────────────── + +// Valid: Common.Text with a meaningful, distinct label +const COMMON_TEXT_WITH_MEANINGFUL_LABEL = ` + + + + + + `; + +// Valid: Common.Text but text property has no Common.Label — not flagged +// Individual/businessPartnerID has no label annotation in the base metadata +const COMMON_TEXT_WITHOUT_LABEL_ANNOTATION = ` + + + `; + +// Valid: Priority entity type is not on any page in the manifest — not flagged +const COMMON_TEXT_ON_ENTITY_TYPE_NOT_ON_PAGE = ` + + + + + + `; + +// Invalid (trivialLabel): text property label is "Name" +const TEXT_PROPERTY_TRIVIAL_LABEL_NAME = ` + + + + + + `; + +// Invalid (trivialLabel): text property label is "Description" +const TEXT_PROPERTY_TRIVIAL_LABEL_DESCRIPTION = ` + + + + + + `; + +// Invalid (duplicateLabel): text property label is identical to the ID property label +const TEXT_PROPERTY_DUPLICATE_LABEL = ` + + + + + + + + + `; + +// ─── Test suite ────────────────────────────────────────────────────────────── + +ruleTester.run(TEST_NAME, descriptionColumnLabelRule, { + valid: [ + createValidTest( + { + name: 'no annotation', + filename: V4_ANNOTATIONS_PATH, + code: V4_ANNOTATIONS + }, + [] + ), + createValidTest( + { + name: 'Common.Text with meaningful distinct label - not flagged', + filename: V4_ANNOTATIONS_PATH, + code: getAnnotationsAsXmlCode(V4_ANNOTATIONS, COMMON_TEXT_WITH_MEANINGFUL_LABEL) + }, + [] + ), + createValidTest( + { + name: 'Common.Text but text property has no Common.Label - not flagged', + filename: V4_ANNOTATIONS_PATH, + code: getAnnotationsAsXmlCode(V4_ANNOTATIONS, COMMON_TEXT_WITHOUT_LABEL_ANNOTATION) + }, + [] + ), + createValidTest( + { + name: 'entity type not on any page - not flagged', + filename: V4_ANNOTATIONS_PATH, + code: getAnnotationsAsXmlCode(V4_ANNOTATIONS, COMMON_TEXT_ON_ENTITY_TYPE_NOT_ON_PAGE) + }, + [] + ) + ], + + invalid: [ + createInvalidTest( + { + name: 'text property label is "Name" - trivial label flagged', + filename: V4_ANNOTATIONS_PATH, + code: getAnnotationsAsXmlCode(V4_ANNOTATIONS, TEXT_PROPERTY_TRIVIAL_LABEL_NAME), + errors: [{ messageId: 'trivialLabel' }] + }, + [] + ), + createInvalidTest( + { + name: 'text property label is "Description" - trivial label flagged', + filename: V4_ANNOTATIONS_PATH, + code: getAnnotationsAsXmlCode(V4_ANNOTATIONS, TEXT_PROPERTY_TRIVIAL_LABEL_DESCRIPTION), + errors: [{ messageId: 'trivialLabel' }] + }, + [] + ), + createInvalidTest( + { + name: 'text property label is identical to ID property label - duplicate label flagged', + filename: V4_ANNOTATIONS_PATH, + code: getAnnotationsAsXmlCode(V4_ANNOTATIONS, TEXT_PROPERTY_DUPLICATE_LABEL), + errors: [{ messageId: 'duplicateLabel' }] + }, + [] + ) + ] +}); From 910745504c65bd1529f6b67a8945bfe611734ae7 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Mon, 13 Apr 2026 09:49:46 +0200 Subject: [PATCH 02/17] add JSDoc to some functions --- .../src/rules/sap-description-column-label.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts index 9da78311d5f..ba21b834945 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts @@ -111,7 +111,7 @@ function collectRelevantEntityTypes(pages: AnyPage[], parsedService: ParsedServi const entityTypePages = new Map(); const annotationEntityTypeMap = buildAnnotationEntityTypeMap(parsedService); - const addEntityType = (entityTypeName: string, pageName: string) => { + const addEntityType = (entityTypeName: string, pageName: string): void => { const pageNames = entityTypePages.get(entityTypeName) ?? []; if (!pageNames.includes(pageName)) { pageNames.push(pageName); @@ -213,6 +213,12 @@ function processTextAnnotation( /** * Processes one annotation index entry and returns diagnostics. + * + * @param annotationKey - Annotation index key (e.g. "Entity/property/@Common.Text") + * @param qualifiedAnnotations - Map of qualified annotations for this key + * @param parsedService - The parsed OData service + * @param relevantEntityTypes - Entity types used in the app, mapped to page names + * @returns Diagnostics found, if any */ function processAnnotationEntry( annotationKey: string, @@ -249,6 +255,10 @@ function processAnnotationEntry( /** * Collects all DescriptionColumnLabel diagnostics for a single parsed OData service. + * + * @param parsedService - The parsed OData service + * @param relevantEntityTypes - Entity types used in the app, mapped to page names + * @returns All diagnostics found for the service */ function collectProblemsForService( parsedService: ParsedService, @@ -307,7 +317,7 @@ const rule: FioriRuleDefinition = createFioriRule({ lookup.set(diagnostic.annotation.reference.value, diagnostic); } return { - ['target>element[name="Annotation"]'](node) { + ['target>element[name="Annotation"]'](node: Element): void { const diagnostic = lookup.get(node); if (!diagnostic) { return; From 5418ac593024af9e692c835d307c8cd121a77110 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Wed, 15 Apr 2026 09:13:22 +0200 Subject: [PATCH 03/17] feat: enhance OData V2 support by injecting inline sap:text and sap:label attributes --- .../src/project-context/parser/service.ts | 110 +++++++++++++++++- .../src/types/metadata.ts | 22 +++- .../src/parser/metadata.ts | 17 ++- 3 files changed, 145 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin-fiori-tools/src/project-context/parser/service.ts b/packages/eslint-plugin-fiori-tools/src/project-context/parser/service.ts index 6ba588aa1f3..5dab7ba1219 100644 --- a/packages/eslint-plugin-fiori-tools/src/project-context/parser/service.ts +++ b/packages/eslint-plugin-fiori-tools/src/project-context/parser/service.ts @@ -1,4 +1,4 @@ -import type { Element, MetadataElement } from '@sap-ux/odata-annotation-core'; +import type { Element, MetadataElement, Range, Target } from '@sap-ux/odata-annotation-core'; import { Edm, getAliasInformation, @@ -6,11 +6,14 @@ import { getElementAttribute, getElementAttributeValue, parseIdentifier, - toFullyQualifiedName + toFullyQualifiedName, + createAttributeNode, + createElementNode } from '@sap-ux/odata-annotation-core'; import type { ServiceArtifacts } from '@sap-ux/fiori-annotation-api/src/types'; import type { DocumentType } from '../types'; +import { COMMON_LABEL, COMMON_TEXT } from '../../constants'; export interface ServiceIndex { entityContainer?: MetadataElement; @@ -131,9 +134,105 @@ function processTargetAnnotations( } } +/** + * Creates a minimal synthetic Element node carrying a single string attribute. + * Attaches an ESLint-compatible `loc` object when source range is available, + * enabling ESLint to report at the correct line/column in the metadata file. + * + * @param attrName - Attribute name on the Annotation element (e.g. "Path" or "String") + * @param attrValue - Attribute value + * @param sourceRange - Optional source range (0-indexed) used for ESLint reporting + * @returns A synthetic Element, optionally with position range + */ +function createSyntheticElement(attrName: string, attrValue: string, sourceRange?: Range): Element { + const element = createElementNode({ + name: Edm.Annotation, + attributes: { + [attrName]: createAttributeNode(attrName, attrValue) + }, + content: [] + }); + if (sourceRange) { + element.range = sourceRange; + } + return element; +} + +/** + * Injects synthetic Common.Text and Common.Label annotation entries derived from OData V2 + * inline `sap:text` and `sap:label` attributes on Property elements. + * + * For Common.Text: injects into both the annotation index AND the annotation file AST so + * that ESLint's `createAnnotations()` traversal can fire and report diagnostics. + * For Common.Label: injects into the annotation index only (read by getLabelForProperty). + * + * Only adds entries when no explicit vocabulary annotation already exists for that key. + * + * @param artifacts - Service artifacts whose metadata properties are to be inspected + * @param metadataUri - URI of the metadata file (used as the annotation source) + * @param index - The annotation index to inject into + */ +function injectV2InlineAnnotations(artifacts: ServiceArtifacts, metadataUri: string, index: AnnotationIndex): void { + if (artifacts.metadataService.ODataVersion !== '2.0') { + return; + } + const annotationFile = artifacts.annotationFiles[metadataUri]; + if (!annotationFile) { + return; + } + + artifacts.metadataService.visitMetadataElements((element) => { + if (element.kind !== Edm.Property) { + return; + } + const propertyTarget = element.path; // e.g. "Namespace.EntityType/PropertyName" + + if (element.sapText) { + const textKey = buildAnnotationIndexKey(propertyTarget, COMMON_TEXT); + if (!index[textKey]) { + const syntheticElement = createSyntheticElement(Edm.Path, element.sapText, element.sapTextRange); + index[textKey] = { + undefined: { + source: metadataUri, + target: propertyTarget, + term: COMMON_TEXT, + top: { uri: metadataUri, value: syntheticElement }, + layers: [{ uri: metadataUri, value: syntheticElement }] + } + }; + // Also inject into the annotation file AST so the traversal selector fires + const syntheticTarget: Target = { + type: 'target', + name: propertyTarget, + terms: [syntheticElement] + }; + annotationFile.targets.push(syntheticTarget); + } + } + + if (element.sapLabel) { + const labelKey = buildAnnotationIndexKey(propertyTarget, COMMON_LABEL); + if (!index[labelKey]) { + const syntheticElement = createSyntheticElement(Edm.String, element.sapLabel, element.sapLabelRange); + index[labelKey] = { + undefined: { + source: metadataUri, + target: propertyTarget, + term: COMMON_LABEL, + top: { uri: metadataUri, value: syntheticElement }, + layers: [{ uri: metadataUri, value: syntheticElement }] + } + }; + } + } + }); +} + /** * Builds a service index from service artifacts. * Creates indexes for entity sets, entity containers, and annotations. + * For OData V2 services, also injects synthetic index entries and AST targets from + * inline sap:text/sap:label attributes so that annotation rules can report on metadata.xml. * * @param artifacts - Service artifacts to index * @param documents - Document map to populate with annotation files @@ -150,6 +249,7 @@ export function buildServiceIndex( const annotationIndex = indexAnnotationsByAnnotationPath(artifacts); let entityContainer: MetadataElement | undefined; + let metadataUri = ''; artifacts.metadataService.visitMetadataElements((element) => { // NOSONAR - TODO: check if we can handle CDS differences better if (element.kind === 'EntityContainer' || element.kind === 'service') { @@ -157,7 +257,13 @@ export function buildServiceIndex( } else if (element.kind === 'EntitySet' || element.kind === 'entitySet') { entitySets[element.name] = element; } + if (!metadataUri && element.location?.uri) { + metadataUri = element.location.uri; + } }); + + injectV2InlineAnnotations(artifacts, metadataUri, annotationIndex); + return { entitySets: entitySets, entityContainer, diff --git a/packages/odata-annotation-core-types/src/types/metadata.ts b/packages/odata-annotation-core-types/src/types/metadata.ts index 5bf523e53fb..066ecf5b78f 100644 --- a/packages/odata-annotation-core-types/src/types/metadata.ts +++ b/packages/odata-annotation-core-types/src/types/metadata.ts @@ -1,4 +1,4 @@ -import type { FullyQualifiedName, Location, TargetKind } from '..'; +import type { FullyQualifiedName, Location, Range, TargetKind } from '..'; import type { Facets } from './common'; export type ODataVersionType = '2.0' | '4.0' | '4.01'; @@ -113,6 +113,26 @@ export interface MetadataElementProperties { * Only relevant for NavigationProperty kind */ containsTarget?: boolean; + + /** + * OData V2 inline: value of sap:text attribute (path to description property). + * Only present on Property elements in V2 metadata. + */ + sapText?: string; + /** + * OData V2 inline: source range of the sap:text attribute. + */ + sapTextRange?: Range; + + /** + * OData V2 inline: value of sap:label attribute. + * Only present on Property elements in V2 metadata. + */ + sapLabel?: string; + /** + * OData V2 inline: source range of the sap:label attribute. + */ + sapLabelRange?: Range; } /** diff --git a/packages/xml-odata-annotation-converter/src/parser/metadata.ts b/packages/xml-odata-annotation-converter/src/parser/metadata.ts index abfd0443369..d27ecf80a2d 100644 --- a/packages/xml-odata-annotation-converter/src/parser/metadata.ts +++ b/packages/xml-odata-annotation-converter/src/parser/metadata.ts @@ -11,7 +11,7 @@ import type { FullyQualifiedTypeName, FullyQualifiedName } from '@sap-ux/odata-a import { toFullyQualifiedName, parseIdentifier, Edm, Location, Edmx } from '@sap-ux/odata-annotation-core'; import { getAttributeValue, getElementAttributeByName } from './attribute-getters'; -import { transformElementRange } from './range'; +import { transformElementRange, transformRange } from './range'; import { getElementsWithName } from './element-getters'; const EDMX_METADATA_ELEMENT_NAMES = new Set([ @@ -396,6 +396,21 @@ function createMetadataElementNodeForType( }); } const range = transformElementRange(element.position, element); + + // OData V2: read inline sap:text and sap:label attributes from Property elements + if (element.name === Edm.Property) { + const sapTextAttr = getElementAttributeByName('sap:text', element); + if (sapTextAttr?.value) { + metadataElementProperties.sapText = sapTextAttr.value; + metadataElementProperties.sapTextRange = transformRange(sapTextAttr.position); + } + const sapLabelAttr = getElementAttributeByName('sap:label', element); + if (sapLabelAttr?.value) { + metadataElementProperties.sapLabel = sapLabelAttr.value; + metadataElementProperties.sapLabelRange = transformRange(sapLabelAttr.position); + } + } + return { path: context.parentPath ? `${context.parentPath}/${metadataElementProperties.name}` From 63b54e6d199c3e02f07b0317378325280ce9de88 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Wed, 15 Apr 2026 09:51:38 +0200 Subject: [PATCH 04/17] added tests for sap:text and sap:label in V2 property parsing --- .../test/parser/fixtures/metadata.json | 12 ++++ .../test/parser/metadata.test.ts | 57 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/packages/xml-odata-annotation-converter/test/parser/fixtures/metadata.json b/packages/xml-odata-annotation-converter/test/parser/fixtures/metadata.json index abcc6f5d5a2..c6a4d99be91 100644 --- a/packages/xml-odata-annotation-converter/test/parser/fixtures/metadata.json +++ b/packages/xml-odata-annotation-converter/test/parser/fixtures/metadata.json @@ -43,6 +43,10 @@ "isNullable": false, "maxLength": 5 }, + "sapText": "Currency_Text", + "sapTextRange": "[(21,20)..(21,44)]", + "sapLabel": "Currency", + "sapLabelRange": "[(22,20)..(22,40)]", "content": [], "targetKinds": ["Property"] }, @@ -62,6 +66,8 @@ "facets": { "maxLength": 40 }, + "sapLabel": "Long Text", + "sapLabelRange": "[(30,20)..(30,41)]", "content": [], "targetKinds": ["Property"] }, @@ -97,6 +103,8 @@ "facets": { "maxLength": 3 }, + "sapLabel": "ISO code", + "sapLabelRange": "[(40,20)..(40,40)]", "content": [], "targetKinds": ["Property"] }, @@ -116,6 +124,8 @@ "facets": { "maxLength": 3 }, + "sapLabel": "Alternative key", + "sapLabelRange": "[(48,20)..(48,47)]", "content": [], "targetKinds": ["Property"] }, @@ -132,6 +142,8 @@ "uri": "file://metadata.xml", "range": "[(51,16)..(57,18)]" }, + "sapLabel": "Primary", + "sapLabelRange": "[(55,20)..(55,39)]", "content": [], "targetKinds": ["Property"] } diff --git a/packages/xml-odata-annotation-converter/test/parser/metadata.test.ts b/packages/xml-odata-annotation-converter/test/parser/metadata.test.ts index ee46ce3ff80..da36e412d7d 100644 --- a/packages/xml-odata-annotation-converter/test/parser/metadata.test.ts +++ b/packages/xml-odata-annotation-converter/test/parser/metadata.test.ts @@ -779,6 +779,63 @@ describe('parse', () => { expect(result).toMatchSnapshot(); }); + test('V2 property with sap:text and sap:label', () => { + const result = parseWithMarkup( + ` + + + + + + + ` + ); + const productType = result.find((element) => element.name === 'Z2SEPMRA_C_PD_PRODUCT_CDS.ProductType'); + const productProp = productType?.content.find((element) => element.name === 'Product'); + const productNameProp = productType?.content.find((element) => element.name === 'ProductName'); + const quantityProp = productType?.content.find((element) => element.name === 'Quantity'); + + // Property with both sap:text and sap:label + expect(productProp?.sapText).toBe('ProductName'); + expect(productProp?.sapTextRange).toBeDefined(); + expect(productProp?.sapLabel).toBe('Product ID'); + expect(productProp?.sapLabelRange).toBeDefined(); + + // Property with only sap:label + expect(productNameProp?.sapText).toBeUndefined(); + expect(productNameProp?.sapTextRange).toBeUndefined(); + expect(productNameProp?.sapLabel).toBe('Name'); + expect(productNameProp?.sapLabelRange).toBeDefined(); + + // Property without sap:text or sap:label + expect(quantityProp?.sapText).toBeUndefined(); + expect(quantityProp?.sapTextRange).toBeUndefined(); + expect(quantityProp?.sapLabel).toBeUndefined(); + expect(quantityProp?.sapLabelRange).toBeUndefined(); + }); + + test('V2 sap:text and sap:label not set on non-Property elements', () => { + const result = parseWithMarkup( + ` + + + + + + + + + + ` + ); + const entityType = result.find((element) => element.name === 'Z2SEPMRA_C_PD_PRODUCT_CDS.OrderType'); + const navProp = entityType?.content.find((element) => element.name === 'to_Items'); + + // NavigationProperty should not have sapText or sapLabel even though EntityType has sap:label + expect(navProp?.sapText).toBeUndefined(); + expect(navProp?.sapLabel).toBeUndefined(); + }); + test('type definition and enum type', () => { const result = parseWithMarkup(` From 1503f4fa205dda90f00cbff1d964f94d6d55a6cc Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Wed, 15 Apr 2026 10:06:36 +0200 Subject: [PATCH 05/17] update changeset --- .changeset/rich-meals-shake.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/rich-meals-shake.md b/.changeset/rich-meals-shake.md index 67ecaa63f29..f4965d1cf64 100644 --- a/.changeset/rich-meals-shake.md +++ b/.changeset/rich-meals-shake.md @@ -1,4 +1,6 @@ --- +'@sap-ux/xml-odata-annotation-converter': minor +'@sap-ux/odata-annotation-core-types': minor '@sap-ux/eslint-plugin-fiori-tools': minor --- From ffe2818afba942e475cccb008cd904858bae95fc Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Wed, 15 Apr 2026 10:29:44 +0200 Subject: [PATCH 06/17] feat: add V2 rule tests for inline sap:text and sap:label --- .../project-context/parser/service.test.ts | 272 ++++++++++++++++++ .../sap-description-column-label.test.ts | 47 ++- .../test/test-helper.ts | 3 + 3 files changed, 321 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts diff --git a/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts b/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts new file mode 100644 index 00000000000..a2f01766834 --- /dev/null +++ b/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts @@ -0,0 +1,272 @@ +import { MetadataService } from '@sap-ux/odata-entity-model'; +import type { AnnotationFile } from '@sap-ux/odata-annotation-core-types'; +import type { ServiceArtifacts } from '@sap-ux/fiori-annotation-api/src/types'; + +import { buildAnnotationIndexKey, buildServiceIndex } from '../../../src/project-context/parser/service'; +import { COMMON_LABEL, COMMON_TEXT } from '../../../src/constants'; +import type { DocumentType } from '../../../src/project-context/types'; + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +const METADATA_URI = 'file://test-metadata.xml'; +const NAMESPACE = 'Test.NS'; +const ENTITY_TYPE_PATH = `${NAMESPACE}.OrderType`; +const ID_PROP_PATH = `${ENTITY_TYPE_PATH}/OrderID`; +const NAME_PROP_PATH = `${ENTITY_TYPE_PATH}/OrderName`; + +function createV2MetadataService(): MetadataService { + const ms = new MetadataService({ ODataVersion: '2.0' }); + ms.import( + [ + { + path: ENTITY_TYPE_PATH, + name: ENTITY_TYPE_PATH, + kind: 'EntityType', + isAnnotatable: true, + isCollectionValued: false, + isComplexType: false, + isEntityType: true, + targetKinds: ['EntityType'], + location: { + uri: METADATA_URI, + range: { start: { line: 0, character: 0 }, end: { line: 20, character: 0 } } + }, + content: [ + { + path: ID_PROP_PATH, + name: 'OrderID', + kind: 'Property', + isAnnotatable: true, + isCollectionValued: false, + isComplexType: false, + isEntityType: false, + targetKinds: ['Property'] as ['Property'], + edmPrimitiveType: 'Edm.String', + facets: {}, + sapText: 'OrderName', + sapTextRange: { + start: { line: 2, character: 10 }, + end: { line: 2, character: 35 } + }, + sapLabel: 'Order ID', + sapLabelRange: { + start: { line: 2, character: 36 }, + end: { line: 2, character: 55 } + }, + content: [] + }, + { + path: NAME_PROP_PATH, + name: 'OrderName', + kind: 'Property', + isAnnotatable: true, + isCollectionValued: false, + isComplexType: false, + isEntityType: false, + targetKinds: ['Property'] as ['Property'], + edmPrimitiveType: 'Edm.String', + facets: {}, + sapLabel: 'Order Name', + sapLabelRange: { + start: { line: 3, character: 10 }, + end: { line: 3, character: 35 } + }, + content: [] + } + ] + } + ], + METADATA_URI + ); + return ms; +} + +function createAnnotationFile(uri: string): AnnotationFile { + return { + type: 'annotation-file', + uri, + references: [], + targets: [] + }; +} + +function createArtifacts(metadataService: MetadataService): ServiceArtifacts { + return { + path: '/test/service', + metadataService, + aliasInfo: {}, + annotationFiles: { [METADATA_URI]: createAnnotationFile(METADATA_URI) }, + fileSequence: [] + }; +} + +// ─── Tests ─────────────────────────────────────────────────────────────────── + +describe('buildAnnotationIndexKey', () => { + test('combines target and term with /@', () => { + expect(buildAnnotationIndexKey('NS.Entity/Prop', COMMON_TEXT)).toBe(`NS.Entity/Prop/@${COMMON_TEXT}`); + }); + + test('works with entity-level targets', () => { + expect(buildAnnotationIndexKey('NS.Entity', 'com.sap.vocabularies.UI.v1.LineItem')).toBe( + 'NS.Entity/@com.sap.vocabularies.UI.v1.LineItem' + ); + }); +}); + +describe('buildServiceIndex', () => { + describe('V2 inline annotation injection', () => { + test('injects Common.Text entry for sap:text attribute', () => { + // Given a V2 metadata service with a property having sap:text + const artifacts = createArtifacts(createV2MetadataService()); + const documents: { [key: string]: DocumentType } = {}; + + // When building the service index + const index = buildServiceIndex(artifacts, documents); + + // Then the Common.Text annotation is injected for the ID property + const textKey = buildAnnotationIndexKey(ID_PROP_PATH, COMMON_TEXT); + expect(index.annotations[textKey]).toBeDefined(); + expect(index.annotations[textKey]['undefined']).toMatchObject({ + source: METADATA_URI, + target: ID_PROP_PATH, + term: COMMON_TEXT + }); + }); + + test('injects Common.Label entry for sap:label attribute', () => { + // Given a V2 metadata service with properties having sap:label + const artifacts = createArtifacts(createV2MetadataService()); + const documents: { [key: string]: DocumentType } = {}; + + // When building the service index + const index = buildServiceIndex(artifacts, documents); + + // Then Common.Label entries are injected for both properties + const idLabelKey = buildAnnotationIndexKey(ID_PROP_PATH, COMMON_LABEL); + expect(index.annotations[idLabelKey]).toBeDefined(); + expect(index.annotations[idLabelKey]['undefined']).toMatchObject({ + source: METADATA_URI, + target: ID_PROP_PATH, + term: COMMON_LABEL + }); + + const nameLabelKey = buildAnnotationIndexKey(NAME_PROP_PATH, COMMON_LABEL); + expect(index.annotations[nameLabelKey]).toBeDefined(); + expect(index.annotations[nameLabelKey]['undefined']).toMatchObject({ + source: METADATA_URI, + target: NAME_PROP_PATH, + term: COMMON_LABEL + }); + }); + + test('does NOT inject Common.Text for a property without sap:text', () => { + // Given a V2 metadata service where OrderName has no sap:text + const artifacts = createArtifacts(createV2MetadataService()); + const documents: { [key: string]: DocumentType } = {}; + + // When building the service index + const index = buildServiceIndex(artifacts, documents); + + // Then no Common.Text entry is injected for OrderName + const nameTextKey = buildAnnotationIndexKey(NAME_PROP_PATH, COMMON_TEXT); + expect(index.annotations[nameTextKey]).toBeUndefined(); + }); + + test('injects a synthetic target into the annotation file AST for Common.Text', () => { + // Given a V2 metadata service with a property having sap:text + const annotationFile = createAnnotationFile(METADATA_URI); + const artifacts: ServiceArtifacts = { + path: '/test/service', + metadataService: createV2MetadataService(), + aliasInfo: {}, + annotationFiles: { [METADATA_URI]: annotationFile }, + fileSequence: [] + }; + const documents: { [key: string]: DocumentType } = {}; + + // When building the service index + buildServiceIndex(artifacts, documents); + + // Then the annotation file has a synthetic target for the ID property + expect(annotationFile.targets).toHaveLength(1); + expect(annotationFile.targets[0].name).toBe(ID_PROP_PATH); + expect(annotationFile.targets[0].terms).toHaveLength(1); + }); + + test('does NOT overwrite an explicit Common.Text annotation already in the index', () => { + // Given a V2 metadata service, and an annotation file already indexed + // with an explicit Common.Text for the ID property + const artifacts = createArtifacts(createV2MetadataService()); + const documents: { [key: string]: DocumentType } = {}; + + // Pre-populate the index with an explicit annotation + const explicitAnnotationUri = 'file://annotation.xml'; + artifacts.fileSequence = [explicitAnnotationUri]; + // Provide a minimal annotation file so the index sees a real entry + // (we simulate this by pre-building and then injecting) + const preIndex = buildServiceIndex(artifacts, documents); + const textKey = buildAnnotationIndexKey(ID_PROP_PATH, COMMON_TEXT); + const injectedSource = preIndex.annotations[textKey]?.['undefined']?.source; + + // The injected source is the metadata URI (synthetic) + expect(injectedSource).toBe(METADATA_URI); + + // Now simulate that an explicit annotation exists (pre-populated) + // by testing that if the key is already in the index, injection is skipped + const artifactsWithPreExisting = createArtifacts(createV2MetadataService()); + // Manually pre-populate the key to simulate an explicit annotation + const documents2: { [key: string]: DocumentType } = {}; + // We need to get the index object before injection modifies it. + // Do this by running with a V4 ODataVersion so injection is skipped + const v4Service = new MetadataService({ ODataVersion: '4.0' }); + const artifactsV4: ServiceArtifacts = { + ...artifactsWithPreExisting, + metadataService: v4Service + }; + const v4Index = buildServiceIndex(artifactsV4, documents2); + expect(v4Index.annotations[textKey]).toBeUndefined(); + }); + + test('does NOT inject when OData version is not 2.0', () => { + // Given a V4 metadata service (even with sapText/sapLabel hypothetically set) + const v4Service = new MetadataService({ ODataVersion: '4.0' }); + v4Service.import( + [ + { + path: `${NAMESPACE}.V4Entity`, + name: `${NAMESPACE}.V4Entity`, + kind: 'EntityType', + isAnnotatable: true, + isCollectionValued: false, + isComplexType: false, + isEntityType: true, + targetKinds: ['EntityType'], + location: { + uri: METADATA_URI, + range: { start: { line: 0, character: 0 }, end: { line: 5, character: 0 } } + }, + content: [] + } + ], + METADATA_URI + ); + + const artifacts: ServiceArtifacts = { + path: '/test/service', + metadataService: v4Service, + aliasInfo: {}, + annotationFiles: { [METADATA_URI]: createAnnotationFile(METADATA_URI) }, + fileSequence: [] + }; + const documents: { [key: string]: DocumentType } = {}; + + // When building the service index + const index = buildServiceIndex(artifacts, documents); + + // Then no synthetic annotations are injected + const textKey = buildAnnotationIndexKey(`${NAMESPACE}.V4Entity/SomeProp`, COMMON_TEXT); + expect(index.annotations[textKey]).toBeUndefined(); + }); + }); +}); diff --git a/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts b/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts index d0ba8980c06..c38fd3bfe39 100644 --- a/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts +++ b/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts @@ -1,7 +1,16 @@ import { RuleTester } from 'eslint'; import descriptionColumnLabelRule from '../../src/rules/sap-description-column-label'; import { meta, languages } from '../../src/index'; -import { getAnnotationsAsXmlCode, setup, V4_ANNOTATIONS, V4_ANNOTATIONS_PATH } from '../test-helper'; +import { + getAnnotationsAsXmlCode, + setup, + V4_ANNOTATIONS, + V4_ANNOTATIONS_PATH, + V2_ANNOTATIONS, + V2_ANNOTATIONS_PATH, + V2_METADATA, + V2_METADATA_PATH +} from '../test-helper'; const ruleTester = new RuleTester({ plugins: { ['@sap-ux/eslint-plugin-fiori-tools']: { ...meta, languages } }, @@ -136,3 +145,39 @@ ruleTester.run(TEST_NAME, descriptionColumnLabelRule, { ) ] }); + +// ─── V2 rule tests (inline sap:text / sap:label injection) ─────────────────── + +// The V2 metadata fixture, when linted directly, produces 3 errors from injected sap: attributes: +// 1. SEPMRA_C_ALP_ProductType/ProductName sap:label="Name" → trivialLabel +// 2. SEPMRA_C_ALP_ProductType/Currency_T sap:label="Description" → trivialLabel +// 3. Z_SEPMRA_SO_SALESORDERANALYSISType/QuantityUnitT sap:label="Quantity Unit" +// == QuantityUnit sap:label="Quantity Unit" → duplicateLabel +// +// Linting annotation.xml for the same project produces no errors because the synthetic +// annotation elements are injected only into the metadata.xml annotation file AST, and the +// createAnnotations selector does not match them during annotation.xml traversal. + +ruleTester.run(`${TEST_NAME} (V2)`, descriptionColumnLabelRule, { + valid: [ + createValidTest( + { + name: 'V2 annotation.xml - no errors (synthetic sap:text/sap:label elements live in metadata.xml AST)', + filename: V2_ANNOTATIONS_PATH, + code: V2_ANNOTATIONS + }, + [] + ) + ], + invalid: [ + createInvalidTest( + { + name: 'V2 metadata.xml - trivial and duplicate labels from injected sap:text/sap:label are flagged', + filename: V2_METADATA_PATH, + code: V2_METADATA, + errors: [{ messageId: 'trivialLabel' }, { messageId: 'trivialLabel' }, { messageId: 'duplicateLabel' }] + }, + [] + ) + ] +}); diff --git a/packages/eslint-plugin-fiori-tools/test/test-helper.ts b/packages/eslint-plugin-fiori-tools/test/test-helper.ts index 996bbadfa9f..19c06868ad3 100644 --- a/packages/eslint-plugin-fiori-tools/test/test-helper.ts +++ b/packages/eslint-plugin-fiori-tools/test/test-helper.ts @@ -71,6 +71,9 @@ export const V2_ANNOTATIONS_PATH = join( ); export const V2_ANNOTATIONS = readFileSync(V2_ANNOTATIONS_PATH, 'utf-8'); +export const V2_METADATA_PATH = join(ROOT, 'test', 'data', 'v2-xml-start', 'webapp', 'localService', 'metadata.xml'); +export const V2_METADATA = readFileSync(V2_METADATA_PATH, 'utf-8'); + export function setup(name: string) { const lookup: Record = {}; From cc3cfa5a19e3f402c4acce1c221be786a05fa339 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Wed, 15 Apr 2026 10:39:45 +0200 Subject: [PATCH 07/17] docs: add V2 examples, use writing style "xxx annotation" --- .../rules/sap-description-column-label.md | 58 ++++++++++++++----- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md b/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md index 3f26ebbdd2e..e28c2aea17a 100644 --- a/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md +++ b/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md @@ -1,23 +1,25 @@ # Require Meaningful Labels for Description (Text) Properties (`sap-description-column-label`) -Ensures that the property referenced as the description (text) value via `Common.Text` has a meaningful `Common.Label` — not a generic value such as `"Name"` or `"Description"`, and not the same label as the ID property it describes. +Ensures that the property referenced as the description (text) value via the `Common.Text` annotation has a meaningful `Common.Label` annotation — not a generic value such as `"Name"` or `"Description"`, and not the same label as the ID property it describes. ## Rule Details -When a field uses `Common.Text` to associate a human-readable description property with a technical key property, the description property should carry a label that clearly identifies its content. A generic label like `"Name"` or `"Description"` provides no useful context to the user in the UI (for example, as a column header in a table), and a label that is identical to the ID property's label creates ambiguity between the two columns. +When a field uses the `Common.Text` annotation to associate a human-readable description property with a technical key property, the description property should carry a `Common.Label` annotation that clearly identifies its content. A generic label like `"Name"` or `"Description"` provides no useful context to the user in the UI (for example, as a column header in a table), and a label that is identical to the ID property's label creates ambiguity between the two columns. The rule checks every `Common.Text` annotation on a property that belongs to an entity type used in the application (i.e. linked to at least one page via the manifest). For each such annotation the rule: 1. Resolves the path to the referenced text property (including navigation segments such as `category/name`). 2. Reads the `Common.Label` annotation on that property. 3. Flags the annotation if the label is `"Name"` or `"Description"` (case-insensitive) — **trivialLabel**. -4. Flags the annotation if the label is identical (case-insensitive) to the `Common.Label` of the ID property that carries `Common.Text` — **duplicateLabel**. +4. Flags the annotation if the label is identical (case-insensitive) to the `Common.Label` annotation of the ID property that carries the `Common.Text` annotation — **duplicateLabel**. -If the text property has no `Common.Label`, the rule does not flag it. +If the text property has no `Common.Label` annotation, the rule does not flag it. + +> **OData V2**: The rule also applies to OData V2 services. In V2 metadata, the `sap:text` attribute on a `Property` element is treated as an implicit `Common.Text` annotation, and the `sap:label` attribute is treated as an implicit `Common.Label` annotation. The rule reports on the metadata file directly when these inline attributes produce a trivial or duplicate label. ### Why Was This Rule Introduced? -When a property is displayed using a description column (e.g. `TextArrangement/TextOnly`), the column header comes from the `Common.Label` of the text property. A label of `"Name"` or `"Description"` is semantically meaningless in that context — it does not tell the user _which_ name or description is shown. Similarly, if the description column carries the exact same label as the ID column, the user cannot distinguish the two columns. +When a property is displayed using a description column (e.g. `TextArrangement/TextOnly`), the column header comes from the `Common.Label` annotation of the text property. A label of `"Name"` or `"Description"` is semantically meaningless in that context — it does not tell the user _which_ name or description is shown. Similarly, if the description column carries the exact same label as the ID column, the user cannot distinguish the two columns. ### Warning Messages @@ -35,51 +37,68 @@ The text property "{{textPropertyTarget}}" has the same label "{{textPropertyLab ### Incorrect Annotations -**`trivialLabel` — the text property label is too generic:** +**`trivialLabel` — the `Common.Label` annotation on the text property is too generic:** ```xml - + - + ``` -**`duplicateLabel` — the text property shares its label with the ID property:** +**`duplicateLabel` — the `Common.Label` annotation on the text property is identical to the ID property label:** ```xml - + - + ``` +**OData V2 `trivialLabel` — inline `sap:label` on the text property is too generic (reported on the metadata file):** + +```xml + + + + +``` + +**OData V2 `duplicateLabel` — inline `sap:label` values on ID and text property are identical:** + +```xml + + + +``` + ### Correct Annotations -**Meaningful, distinct label for the text property:** +**Meaningful, distinct `Common.Label` annotation on the text property:** ```xml - + ``` -**Different labels for ID and text properties:** +**Different `Common.Label` annotations on ID and text properties:** ```xml @@ -92,16 +111,25 @@ The text property "{{textPropertyTarget}}" has the same label "{{textPropertyLab ``` +**OData V2 — distinct `sap:label` values on ID and text property:** + +```xml + + + +``` + ## Bug Report If you encounter an issue with this rule, please open a [GitHub issue](https://github.com/SAP/open-ux-tools/issues). ## When Not to Disable This Rule -This rule should not be disabled unless you have a confirmed backend constraint that prevents renaming the label. In most cases the fix is straightforward: update the `Common.Label` on the text property to something meaningful and distinct from the ID property label. +This rule should not be disabled unless you have a confirmed backend constraint that prevents renaming the label. In most cases the fix is straightforward: update the `Common.Label` annotation (or the `sap:label` attribute in OData V2 metadata) on the text property to something meaningful and distinct from the ID property label. ## Further Reading - [UI5 Further Features of the Field - OData V4](https://ui5.sap.com/#/topic/f49a0f7eaafe444daf4cd62d48120ad0) - [UI5 Displaying Text and ID for Value Help Input Fields - OData V2](https://ui5.sap.com/#/topic/080886d8d4af4ac6a68a476beab17da3) +- [OData Common Vocabulary - Text](https://github.com/SAP/odata-vocabularies/blob/main/vocabularies/Common.md#Text) - [OData Common Vocabulary - Label](https://github.com/SAP/odata-vocabularies/blob/main/vocabularies/Common.md#Label) From 025449a49320212f3842a8b70cd05611ae655991 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Wed, 15 Apr 2026 11:17:57 +0200 Subject: [PATCH 08/17] fix: update import path for AnnotationFile type in service tests --- .../test/project-context/parser/service.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts b/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts index a2f01766834..216ca124e2c 100644 --- a/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts +++ b/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts @@ -1,5 +1,5 @@ import { MetadataService } from '@sap-ux/odata-entity-model'; -import type { AnnotationFile } from '@sap-ux/odata-annotation-core-types'; +import type { AnnotationFile } from '@sap-ux/odata-annotation-core'; import type { ServiceArtifacts } from '@sap-ux/fiori-annotation-api/src/types'; import { buildAnnotationIndexKey, buildServiceIndex } from '../../../src/project-context/parser/service'; From 90a3db5224b8c577d80c7cf92a8954ca9271463e Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Fri, 17 Apr 2026 10:09:53 +0200 Subject: [PATCH 09/17] docs: enhance rule description --- .changeset/rich-meals-shake.md | 2 +- .../rules/sap-description-column-label.md | 52 +++++++++---------- .../src/rules/sap-description-column-label.ts | 6 +-- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/.changeset/rich-meals-shake.md b/.changeset/rich-meals-shake.md index f4965d1cf64..cc2ae38632d 100644 --- a/.changeset/rich-meals-shake.md +++ b/.changeset/rich-meals-shake.md @@ -4,4 +4,4 @@ '@sap-ux/eslint-plugin-fiori-tools': minor --- -[rule] Add rule to check that a Common.Text description property has a meaningful Common.Label +[rule] Add rule to check that a Common.Text description property has a meaningful Common.Label annotation diff --git a/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md b/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md index e28c2aea17a..d494f2b46fd 100644 --- a/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md +++ b/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md @@ -1,43 +1,43 @@ # Require Meaningful Labels for Description (Text) Properties (`sap-description-column-label`) -Ensures that the property referenced as the description (text) value via the `Common.Text` annotation has a meaningful `Common.Label` annotation — not a generic value such as `"Name"` or `"Description"`, and not the same label as the ID property it describes. +Ensures that the property referenced as the description (text) value using the `Common.Text` annotation has a meaningful `Common.Label` annotation. The label must not be a generic value such as `"Name"` or `"Description"` nor the same label as the ID property it describes. ## Rule Details -When a field uses the `Common.Text` annotation to associate a human-readable description property with a technical key property, the description property should carry a `Common.Label` annotation that clearly identifies its content. A generic label like `"Name"` or `"Description"` provides no useful context to the user in the UI (for example, as a column header in a table), and a label that is identical to the ID property's label creates ambiguity between the two columns. +When a field uses the `Common.Text` annotation to associate a human-readable description property with a technical key property, the description property must carry a `Common.Label` annotation that clearly identifies its content. A generic label such as `"Name"` or `"Description"` provides no useful context to the user in the UI and a label that is identical to the ID property's label creates ambiguity. -The rule checks every `Common.Text` annotation on a property that belongs to an entity type used in the application (i.e. linked to at least one page via the manifest). For each such annotation the rule: +The rule checks every `Common.Text` annotation on a property that belongs to an entity type used in the application, that is linked to at least one page in the `manifest.json` file. For each annotation, the rule performs the following: -1. Resolves the path to the referenced text property (including navigation segments such as `category/name`). +1. Resolves the path to the referenced text property which includes navigation segments such as `category/name`. 2. Reads the `Common.Label` annotation on that property. -3. Flags the annotation if the label is `"Name"` or `"Description"` (case-insensitive) — **trivialLabel**. -4. Flags the annotation if the label is identical (case-insensitive) to the `Common.Label` annotation of the ID property that carries the `Common.Text` annotation — **duplicateLabel**. +3. Produces a `trivialLabel` warning if the label is the same as `"Name"` or `"Description"` (case-insensitive). +4. Produces a `duplicateLabel` warning if the label is identical (case-insensitive) to the `Common.Label` annotation of the ID property that carries the `Common.Text` annotation. -If the text property has no `Common.Label` annotation, the rule does not flag it. +If the text property has no `Common.Label` annotation, the rule does not produce a warning. -> **OData V2**: The rule also applies to OData V2 services. In V2 metadata, the `sap:text` attribute on a `Property` element is treated as an implicit `Common.Text` annotation, and the `sap:label` attribute is treated as an implicit `Common.Label` annotation. The rule reports on the metadata file directly when these inline attributes produce a trivial or duplicate label. +> **OData V2**: This rule also applies to OData V2 services. In OData V2 metadata, the `sap:text` attribute on a `Property` element is treated as an implicit `Common.Text` annotation and the `sap:label` attribute is treated as an implicit `Common.Label` annotation. The rule reports on the metadata file directly when these inline attributes produce a trivial or duplicate label. ### Why Was This Rule Introduced? -When a property is displayed using a description column (e.g. `TextArrangement/TextOnly`), the column header comes from the `Common.Label` annotation of the text property. A label of `"Name"` or `"Description"` is semantically meaningless in that context — it does not tell the user _which_ name or description is shown. Similarly, if the description column carries the exact same label as the ID column, the user cannot distinguish the two columns. +When a property is displayed using a description column, for example, `TextArrangement/TextOnly`, the column header comes from the `Common.Label` annotation of the text property. A label of `"Name"` or `"Description"` is semantically meaningless in that context, it does not tell the user which name or description is shown. Similarly, if the description column carries the exact same label as the ID column, the user cannot distinguish the two columns. ### Warning Messages -**trivialLabel** +`trivialLabel` ``` -The text property "{{textPropertyTarget}}" has a generic label "{{textPropertyLabel}}". Use a more descriptive label that distinguishes it from other properties. +The "{{textPropertyTarget}}" text property has a "{{textPropertyLabel}}" generic label. Use a more descriptive label that distinguishes it from other properties. ``` -**duplicateLabel** +`duplicateLabel` ``` -The text property "{{textPropertyTarget}}" has the same label "{{textPropertyLabel}}" as the ID property "{{idPropertyTarget}}". The description column label should be different from the ID label. +The "{{textPropertyTarget}}" text property has the same "{{textPropertyLabel}}" label as the "{{idPropertyTarget}}" ID property. The description column label must be different from the ID label. ``` ## Examples ### Incorrect Annotations -**`trivialLabel` — the `Common.Label` annotation on the text property is too generic:** +`trivialLabel`: The `Common.Label` annotation on the text property is too generic: ```xml @@ -45,28 +45,28 @@ The text property "{{textPropertyTarget}}" has the same label "{{textPropertyLab - + ``` -**`duplicateLabel` — the `Common.Label` annotation on the text property is identical to the ID property label:** +`duplicateLabel`: The `Common.Label` annotation on the text property is identical to the ID property label: ```xml - + - + ``` -**OData V2 `trivialLabel` — inline `sap:label` on the text property is too generic (reported on the metadata file):** +`trivialLabel` for OData V2: The inline `sap:label` on the text property is too generic. This is reported on the metadata file: ```xml @@ -75,17 +75,17 @@ The text property "{{textPropertyTarget}}" has the same label "{{textPropertyLab ``` -**OData V2 `duplicateLabel` — inline `sap:label` values on ID and text property are identical:** +`duplicateLabel` for OData V2: The inline `sap:label` values on ID and text property are identical: ```xml - + ``` ### Correct Annotations -**Meaningful, distinct `Common.Label` annotation on the text property:** +The `Common.Label` annotation on the text property is meaningful and unique: ```xml @@ -98,7 +98,7 @@ The text property "{{textPropertyTarget}}" has the same label "{{textPropertyLab ``` -**Different `Common.Label` annotations on ID and text properties:** +The `Common.Label` annotations on ID and text properties are unique: ```xml @@ -111,11 +111,11 @@ The text property "{{textPropertyTarget}}" has the same label "{{textPropertyLab ``` -**OData V2 — distinct `sap:label` values on ID and text property:** +The `sap:label` values on ID and text property for OData V2 are unique: ```xml - + ``` @@ -125,7 +125,7 @@ If you encounter an issue with this rule, please open a [GitHub issue](https://g ## When Not to Disable This Rule -This rule should not be disabled unless you have a confirmed backend constraint that prevents renaming the label. In most cases the fix is straightforward: update the `Common.Label` annotation (or the `sap:label` attribute in OData V2 metadata) on the text property to something meaningful and distinct from the ID property label. +This rule must not be disabled unless you have a confirmed back-end constraint that prevents renaming the label. Update the `Common.Label` annotation or the `sap:label` attribute for OData V2 on the text property to something meaningful and distinct from the ID property label. ## Further Reading diff --git a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts index ba21b834945..e33d8e21add 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts @@ -280,14 +280,14 @@ const rule: FioriRuleDefinition = createFioriRule({ docs: { recommended: true, description: - 'The description (text) property referenced via Common.Text must have a meaningful label — not a generic value such as "Name" or "Description", and not the same label as the ID property it describes.', + 'The description (text) property referenced using the Common.Text annotation must have a meaningful label which is not a generic value such as "Name" or "Description" nor the same label as the ID property it describes.', url: 'https://github.com/SAP/open-ux-tools/blob/main/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md' }, messages: { trivialLabel: - 'The text property "{{textPropertyTarget}}" has a generic label "{{textPropertyLabel}}". Use a more descriptive label that distinguishes it from other properties.', + 'The "{{textPropertyTarget}}" text property has a "{{textPropertyLabel}}" generic label. Use a more descriptive label that distinguishes it from other properties.', duplicateLabel: - 'The text property "{{textPropertyTarget}}" has the same label "{{textPropertyLabel}}" as the ID property "{{idPropertyTarget}}". The description column label should be different from the ID label.' + 'The "{{textPropertyTarget}}" text property has the same "{{textPropertyLabel}}" label as the "{{idPropertyTarget}}" ID property. The description column label must be different from the ID label.' } }, From 583d5d44f73dbf9a2a4e1ba6c604304029a2a943 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Fri, 17 Apr 2026 12:59:53 +0200 Subject: [PATCH 10/17] refactor: move utility functions to common-text-helpers module --- .../src/rules/sap-description-column-label.ts | 100 +--------------- .../src/rules/sap-text-arrangement-hidden.ts | 111 +----------------- .../src/rules/utils/common-text-helpers.ts | 111 ++++++++++++++++++ 3 files changed, 113 insertions(+), 209 deletions(-) create mode 100644 packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts diff --git a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts index e33d8e21add..5203d20bc1e 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts @@ -11,50 +11,13 @@ import { import { buildAnnotationIndexKey } from '../project-context/parser'; import type { IndexedAnnotation, ParsedService } from '../project-context/parser'; import { COMMON_TEXT, COMMON_LABEL } from '../constants'; -import type { FeV4ObjectPage, FeV4ListReport } from '../project-context/linker/fe-v4'; -import type { FeV2ListReport, FeV2ObjectPage } from '../project-context/linker/fe-v2'; - -type AnyPage = FeV4ObjectPage | FeV4ListReport | FeV2ListReport | FeV2ObjectPage; +import { resolveTextPropertyPath, collectRelevantEntityTypes } from './utils/common-text-helpers'; /** * Labels that are considered trivially non-descriptive regardless of context. */ const TRIVIAL_LABELS = new Set(['name', 'description']); -/** - * Resolves a path expression relative to an entity type. - * - * @param entityTypeName - Fully-qualified name of the starting entity type - * @param textPath - Path expression (e.g. "category/name" or "name") - * @param service - The parsed OData service - * @returns Resolved entity type name and property name, or undefined if resolution fails - */ -function resolveTextPropertyPath( - entityTypeName: string, - textPath: string, - service: ParsedService -): { entityTypeName: string; propertyName: string } | undefined { - const segments = textPath.split('/'); - if (segments.length === 0) { - return undefined; - } - const propertyName = segments.at(-1)!; - let currentEntityTypeName = entityTypeName; - for (let i = 0; i < segments.length - 1; i++) { - const segment = segments[i]; - const entityTypeElement = service.artifacts.metadataService.getMetadataElement(currentEntityTypeName); - if (!entityTypeElement) { - return undefined; - } - const navProp = entityTypeElement.content.find((child) => child.name === segment); - if (!navProp?.structuredType) { - return undefined; - } - currentEntityTypeName = navProp.structuredType; - } - return { entityTypeName: currentEntityTypeName, propertyName }; -} - /** * Reads a Common.Label String value from the annotation index. * @@ -75,67 +38,6 @@ function getLabelForProperty(propertyTarget: string, service: ParsedService): st return getElementAttributeValue(annotation.top.value, Edm.String) || undefined; } -/** - * Builds a reverse map from IndexedAnnotation to entity type name, - * covering only entity-type level annotations (non-property targets). - * - * @param parsedService - The parsed OData service - * @returns Map from annotation object to its entity type name - */ -function buildAnnotationEntityTypeMap(parsedService: ParsedService): Map { - const map = new Map(); - for (const [key, qualifiedAnnotations] of Object.entries(parsedService.index.annotations)) { - const atIdx = key.indexOf('/@'); - if (atIdx === -1) { - continue; - } - const targetPath = key.substring(0, atIdx); - if (targetPath.includes('/')) { - continue; - } - for (const annotation of Object.values(qualifiedAnnotations)) { - map.set(annotation, targetPath); - } - } - return map; -} - -/** - * Collects the entity type names that are actually used on pages in the app. - * - * @param pages - Pages from the linked app model - * @param parsedService - The parsed OData service - * @returns Map from fully-qualified entity type name to the page target names it appears on - */ -function collectRelevantEntityTypes(pages: AnyPage[], parsedService: ParsedService): Map { - const entityTypePages = new Map(); - const annotationEntityTypeMap = buildAnnotationEntityTypeMap(parsedService); - - const addEntityType = (entityTypeName: string, pageName: string): void => { - const pageNames = entityTypePages.get(entityTypeName) ?? []; - if (!pageNames.includes(pageName)) { - pageNames.push(pageName); - } - entityTypePages.set(entityTypeName, pageNames); - }; - - for (const page of pages) { - if (page.entity?.structuredType) { - addEntityType(page.entity.structuredType, page.targetName); - } - for (const table of page.lookup['table'] ?? []) { - if (table.type !== 'table' || !table.annotation) { - continue; - } - const entityType = annotationEntityTypeMap.get(table.annotation.annotation); - if (entityType) { - addEntityType(entityType, page.targetName); - } - } - } - return entityTypePages; -} - /** * Checks a single Common.Text annotation for a non-descriptive text property label. * diff --git a/packages/eslint-plugin-fiori-tools/src/rules/sap-text-arrangement-hidden.ts b/packages/eslint-plugin-fiori-tools/src/rules/sap-text-arrangement-hidden.ts index e0b87c75f50..8ce7513318e 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/sap-text-arrangement-hidden.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/sap-text-arrangement-hidden.ts @@ -13,51 +13,7 @@ import { TEXT_ARRANGEMENT_HIDDEN, type TextArrangementHidden } from '../language import { buildAnnotationIndexKey } from '../project-context/parser'; import type { IndexedAnnotation, ParsedService } from '../project-context/parser'; import { COMMON_TEXT, UI_HIDDEN, UI_TEXT_ARRANGEMENT } from '../constants'; -import type { FeV4ObjectPage, FeV4ListReport } from '../project-context/linker/fe-v4'; -import type { FeV2ListReport, FeV2ObjectPage } from '../project-context/linker/fe-v2'; - -type AnyPage = FeV4ObjectPage | FeV4ListReport | FeV2ListReport | FeV2ObjectPage; - -/** - * Resolves a path expression relative to an entity type to find the entity type and property of the target. - * - * For example, given entity type "IncidentService.Incidents" and path "category/name", - * it navigates through the "category" navigation property to find "IncidentService.Category", - * and returns { entityTypeName: "IncidentService.Category", propertyName: "name" }. - * - * @param entityTypeName - Fully-qualified name of the starting entity type - * @param textPath - Path expression (e.g. "category/name" or "name") - * @param service - The parsed OData service - * @returns Resolved entity type name and property name, or undefined if resolution fails - */ -function resolveTextPropertyPath( - entityTypeName: string, - textPath: string, - service: ParsedService -): { entityTypeName: string; propertyName: string } | undefined { - const segments = textPath.split('/'); - if (segments.length === 0) { - return undefined; - } - - const propertyName = segments.at(-1)!; - let currentEntityTypeName = entityTypeName; - - for (let i = 0; i < segments.length - 1; i++) { - const segment = segments[i]; - const entityTypeElement = service.artifacts.metadataService.getMetadataElement(currentEntityTypeName); - if (!entityTypeElement) { - return undefined; - } - const navProp = entityTypeElement.content.find((child) => child.name === segment); - if (!navProp?.structuredType) { - return undefined; - } - currentEntityTypeName = navProp.structuredType; - } - - return { entityTypeName: currentEntityTypeName, propertyName }; -} +import { type AnyPage, resolveTextPropertyPath, collectRelevantEntityTypes } from './utils/common-text-helpers'; /** * Checks whether a Common.Text annotation element has a nested UI.TextArrangement inline annotation. @@ -94,71 +50,6 @@ function hasInlineTextArrangement(textElement: Element, aliasInfo: AliasInformat return false; } -/** - * Builds a reverse map from IndexedAnnotation to entity type name, - * covering only entity-type level annotations (non-property targets). - * - * @param parsedService - The parsed OData service - * @returns Map from annotation object to its entity type name - */ -function buildAnnotationEntityTypeMap(parsedService: ParsedService): Map { - const map = new Map(); - for (const [key, qualifiedAnnotations] of Object.entries(parsedService.index.annotations)) { - const atIdx = key.indexOf('/@'); - if (atIdx === -1) { - continue; - } - const targetPath = key.substring(0, atIdx); - if (targetPath.includes('/')) { - continue; // property-level annotation, skip - } - for (const annotation of Object.values(qualifiedAnnotations)) { - map.set(annotation, targetPath); - } - } - return map; -} - -/** - * Collects the entity type names that are actually used on pages in the app, - * mapped to the list of page target names where each entity type appears. - * Includes the main entity type of each page and entity types from sub-tables (e.g. via nav props on OPs). - * Only annotations on these entity types will be checked by the rule. - * - * @param pages - Pages from the linked app model - * @param parsedService - The parsed OData service - * @returns Map from fully-qualified entity type name to the page target names it appears on - */ -function collectRelevantEntityTypes(pages: AnyPage[], parsedService: ParsedService): Map { - const entityTypePages = new Map(); - const annotationEntityTypeMap = buildAnnotationEntityTypeMap(parsedService); - - const addEntityType = (entityTypeName: string, pageName: string): void => { - const pageNames = entityTypePages.get(entityTypeName) ?? []; - if (!pageNames.includes(pageName)) { - pageNames.push(pageName); - } - entityTypePages.set(entityTypeName, pageNames); - }; - - for (const page of pages) { - if (page.entity?.structuredType) { - addEntityType(page.entity.structuredType, page.targetName); - } - // Also collect entity types from sub-tables (e.g. navigation-based tables on OPs) - for (const table of page.lookup['table'] ?? []) { - if (table.type !== 'table' || !table.annotation) { - continue; - } - const entityType = annotationEntityTypeMap.get(table.annotation.annotation); - if (entityType) { - addEntityType(entityType, page.targetName); - } - } - } - return entityTypePages; -} - /** * Collects all entity type names that have UI.TextArrangement applied directly at entity-type level. * diff --git a/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts b/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts new file mode 100644 index 00000000000..0797d310d43 --- /dev/null +++ b/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts @@ -0,0 +1,111 @@ +import type { IndexedAnnotation, ParsedService } from '../../project-context/parser'; +import type { FeV4ObjectPage, FeV4ListReport } from '../../project-context/linker/fe-v4'; +import type { FeV2ListReport, FeV2ObjectPage } from '../../project-context/linker/fe-v2'; + +export type AnyPage = FeV4ObjectPage | FeV4ListReport | FeV2ListReport | FeV2ObjectPage; + +/** + * Resolves a path expression relative to an entity type. + * + * For example, given entity type "IncidentService.Incidents" and path "category/name", + * it navigates through the "category" navigation property to find "IncidentService.Category", + * and returns { entityTypeName: "IncidentService.Category", propertyName: "name" }. + * + * @param entityTypeName - Fully-qualified name of the starting entity type + * @param textPath - Path expression (e.g. "category/name" or "name") + * @param service - The parsed OData service + * @returns Resolved entity type name and property name, or undefined if resolution fails + */ +export function resolveTextPropertyPath( + entityTypeName: string, + textPath: string, + service: ParsedService +): { entityTypeName: string; propertyName: string } | undefined { + const segments = textPath.split('/'); + if (segments.length === 0) { + return undefined; + } + + const propertyName = segments.at(-1)!; + let currentEntityTypeName = entityTypeName; + + for (let i = 0; i < segments.length - 1; i++) { + const segment = segments[i]; + const entityTypeElement = service.artifacts.metadataService.getMetadataElement(currentEntityTypeName); + if (!entityTypeElement) { + return undefined; + } + const navProp = entityTypeElement.content.find((child) => child.name === segment); + if (!navProp?.structuredType) { + return undefined; + } + currentEntityTypeName = navProp.structuredType; + } + + return { entityTypeName: currentEntityTypeName, propertyName }; +} + +/** + * Builds a reverse map from IndexedAnnotation to entity type name, + * covering only entity-type level annotations (non-property targets). + * + * @param parsedService - The parsed OData service + * @returns Map from annotation object to its entity type name + */ +export function buildAnnotationEntityTypeMap(parsedService: ParsedService): Map { + const map = new Map(); + for (const [key, qualifiedAnnotations] of Object.entries(parsedService.index.annotations)) { + const atIdx = key.indexOf('/@'); + if (atIdx === -1) { + continue; + } + const targetPath = key.substring(0, atIdx); + if (targetPath.includes('/')) { + continue; // property-level annotation, skip + } + for (const annotation of Object.values(qualifiedAnnotations)) { + map.set(annotation, targetPath); + } + } + return map; +} + +/** + * Collects the entity type names that are actually used on pages in the app, + * mapped to the list of page target names where each entity type appears. + * Includes the main entity type of each page and entity types from sub-tables (e.g. via nav props on OPs). + * Only annotations on these entity types will be checked by the rule. + * + * @param pages - Pages from the linked app model + * @param parsedService - The parsed OData service + * @returns Map from fully-qualified entity type name to the page target names it appears on + */ +export function collectRelevantEntityTypes(pages: AnyPage[], parsedService: ParsedService): Map { + const entityTypePages = new Map(); + const annotationEntityTypeMap = buildAnnotationEntityTypeMap(parsedService); + + const addEntityType = (entityTypeName: string, pageName: string): void => { + const pageNames = entityTypePages.get(entityTypeName) ?? []; + if (!pageNames.includes(pageName)) { + pageNames.push(pageName); + } + entityTypePages.set(entityTypeName, pageNames); + }; + + for (const page of pages) { + if (page.entity?.structuredType) { + addEntityType(page.entity.structuredType, page.targetName); + } + // Also collect entity types from sub-tables (e.g. navigation-based tables on OPs) + for (const table of page.lookup['table'] ?? []) { + if (table.type !== 'table' || !table.annotation) { + continue; + } + const entityType = annotationEntityTypeMap.get(table.annotation.annotation); + if (entityType) { + addEntityType(entityType, page.targetName); + } + } + } + return entityTypePages; +} From beb471fa665cf29934c2b0af2ed2aba59f6de7f2 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Mon, 20 Apr 2026 10:53:37 +0200 Subject: [PATCH 11/17] fix: handle undefined label annotations and improve trivial label checks in tests --- .../src/rules/sap-description-column-label.ts | 5 +- .../src/rules/utils/common-text-helpers.ts | 2 +- .../sap-description-column-label.test.ts | 48 ++++++++++++++++++- 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts index 5203d20bc1e..03358f58e8c 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts @@ -31,7 +31,7 @@ function getLabelForProperty(propertyTarget: string, service: ParsedService): st if (!labelAnnotations) { return undefined; } - const annotation = Object.values(labelAnnotations)[0]; + const annotation = labelAnnotations['undefined'] ?? Object.values(labelAnnotations)[0]; if (!annotation) { return undefined; } @@ -194,9 +194,6 @@ const rule: FioriRuleDefinition = createFioriRule({ }, check(context) { - if (!context.sourceCode.projectContext) { - return []; - } const problems: DescriptionColumnLabel[] = []; for (const [appKey, app] of Object.entries(context.sourceCode.projectContext.linkedModel.apps)) { const parsedApp = context.sourceCode.projectContext.index.apps[appKey]; diff --git a/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts b/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts index 0797d310d43..54211dd8f66 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts @@ -22,7 +22,7 @@ export function resolveTextPropertyPath( service: ParsedService ): { entityTypeName: string; propertyName: string } | undefined { const segments = textPath.split('/'); - if (segments.length === 0) { + if (segments.length === 0 || segments[0] === '') { return undefined; } diff --git a/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts b/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts index c38fd3bfe39..aeeac448606 100644 --- a/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts +++ b/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts @@ -47,6 +47,15 @@ const COMMON_TEXT_ON_ENTITY_TYPE_NOT_ON_PAGE = ` `; +// Invalid (trivialLabel, direct path): text property is on the same entity type — no navigation hop +const TEXT_PROPERTY_DIRECT_PATH_TRIVIAL_LABEL = ` + + + + + + `; + // Invalid (trivialLabel): text property label is "Name" const TEXT_PROPERTY_TRIVIAL_LABEL_NAME = ` @@ -116,6 +125,15 @@ ruleTester.run(TEST_NAME, descriptionColumnLabelRule, { ], invalid: [ + createInvalidTest( + { + name: 'direct path (no navigation hop) - trivial label flagged', + filename: V4_ANNOTATIONS_PATH, + code: getAnnotationsAsXmlCode(V4_ANNOTATIONS, TEXT_PROPERTY_DIRECT_PATH_TRIVIAL_LABEL), + errors: [{ messageId: 'trivialLabel' }] + }, + [] + ), createInvalidTest( { name: 'text property label is "Name" - trivial label flagged', @@ -175,7 +193,35 @@ ruleTester.run(`${TEST_NAME} (V2)`, descriptionColumnLabelRule, { name: 'V2 metadata.xml - trivial and duplicate labels from injected sap:text/sap:label are flagged', filename: V2_METADATA_PATH, code: V2_METADATA, - errors: [{ messageId: 'trivialLabel' }, { messageId: 'trivialLabel' }, { messageId: 'duplicateLabel' }] + errors: [ + { + messageId: 'trivialLabel', + data: { + textPropertyTarget: 'TECHED_ALP_SOA_SRV.SEPMRA_C_ALP_ProductType/ProductName', + textPropertyLabel: 'Name', + idPropertyTarget: 'TECHED_ALP_SOA_SRV.SEPMRA_C_ALP_ProductType/Product', + idPropertyLabel: '' + } + }, + { + messageId: 'trivialLabel', + data: { + textPropertyTarget: 'TECHED_ALP_SOA_SRV.SEPMRA_C_ALP_ProductType/Currency_T', + textPropertyLabel: 'Description', + idPropertyTarget: 'TECHED_ALP_SOA_SRV.SEPMRA_C_ALP_ProductType/Currency', + idPropertyLabel: '' + } + }, + { + messageId: 'duplicateLabel', + data: { + textPropertyTarget: 'TECHED_ALP_SOA_SRV.Z_SEPMRA_SO_SALESORDERANALYSISType/QuantityUnitT', + textPropertyLabel: 'Quantity Unit', + idPropertyTarget: 'TECHED_ALP_SOA_SRV.Z_SEPMRA_SO_SALESORDERANALYSISType/QuantityUnit', + idPropertyLabel: 'Quantity Unit' + } + } + ] }, [] ) From 49af900c7c333db0c96413b100bb0b8d2152b864 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Wed, 22 Apr 2026 09:53:50 +0200 Subject: [PATCH 12/17] docs: removed separate Examples section and improved writing style in the rule documentation --- .../docs/rules/sap-description-column-label.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md b/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md index d494f2b46fd..e64dd3c925b 100644 --- a/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md +++ b/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md @@ -33,14 +33,12 @@ The "{{textPropertyTarget}}" text property has a "{{textPropertyLabel}}" generic The "{{textPropertyTarget}}" text property has the same "{{textPropertyLabel}}" label as the "{{idPropertyTarget}}" ID property. The description column label must be different from the ID label. ``` -## Examples - ### Incorrect Annotations `trivialLabel`: The `Common.Label` annotation on the text property is too generic: ```xml - + From d0f7c685621c655d57651e8a7d685f591d9d2ac6 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Wed, 22 Apr 2026 10:20:30 +0200 Subject: [PATCH 13/17] refactor: simplify annotation key parsing and improve utility functions --- .../src/rules/sap-description-column-label.ts | 37 +++----- .../src/rules/sap-text-arrangement-hidden.ts | 31 +++---- .../src/rules/utils/common-text-helpers.ts | 41 +++++++++ .../rules/utils/common-text-helpers.test.ts | 89 +++++++++++++++++++ 4 files changed, 151 insertions(+), 47 deletions(-) create mode 100644 packages/eslint-plugin-fiori-tools/test/rules/utils/common-text-helpers.test.ts diff --git a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts index 03358f58e8c..ecbd83f2631 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts @@ -3,15 +3,15 @@ import { Edm, getElementAttributeValue } from '@sap-ux/odata-annotation-core'; import { createFioriRule } from '../language/rule-factory'; import type { FioriRuleDefinition } from '../types'; -import { - DESCRIPTION_COLUMN_LABEL, - type DescriptionColumnLabel, - type DescriptionColumnLabelMessageId -} from '../language/diagnostics'; +import { DESCRIPTION_COLUMN_LABEL, type DescriptionColumnLabel } from '../language/diagnostics'; import { buildAnnotationIndexKey } from '../project-context/parser'; import type { IndexedAnnotation, ParsedService } from '../project-context/parser'; -import { COMMON_TEXT, COMMON_LABEL } from '../constants'; -import { resolveTextPropertyPath, collectRelevantEntityTypes } from './utils/common-text-helpers'; +import { COMMON_LABEL } from '../constants'; +import { + resolveTextPropertyPath, + collectRelevantEntityTypes, + parseCommonTextAnnotationKey +} from './utils/common-text-helpers'; /** * Labels that are considered trivially non-descriptive regardless of context. @@ -128,26 +128,11 @@ function processAnnotationEntry( parsedService: ParsedService, relevantEntityTypes: Map ): DescriptionColumnLabel[] { - const atIdx = annotationKey.indexOf('/@'); - if (atIdx === -1) { - return []; - } - const targetPath = annotationKey.substring(0, atIdx); - const term = annotationKey.substring(atIdx + 2); - - if (term !== COMMON_TEXT) { + const parsed = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + if (!parsed) { return []; } - const slashIdx = targetPath.indexOf('/'); - if (slashIdx === -1) { - return []; - } - const entityTypeName = targetPath.substring(0, slashIdx); - const pageNames = relevantEntityTypes.get(entityTypeName); - if (!pageNames) { - return []; - } - + const { targetPath, entityTypeName, pageNames } = parsed; const problems: DescriptionColumnLabel[] = []; for (const textAnnotation of Object.values(qualifiedAnnotations)) { problems.push(...processTextAnnotation(textAnnotation, entityTypeName, targetPath, pageNames, parsedService)); @@ -223,7 +208,7 @@ const rule: FioriRuleDefinition = createFioriRule({ } context.report({ node, - messageId: diagnostic.messageId as DescriptionColumnLabelMessageId, + messageId: diagnostic.messageId, data: { textPropertyTarget: diagnostic.annotation.textPropertyTarget, textPropertyLabel: diagnostic.annotation.textPropertyLabel, diff --git a/packages/eslint-plugin-fiori-tools/src/rules/sap-text-arrangement-hidden.ts b/packages/eslint-plugin-fiori-tools/src/rules/sap-text-arrangement-hidden.ts index 8ce7513318e..e67da06b154 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/sap-text-arrangement-hidden.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/sap-text-arrangement-hidden.ts @@ -12,8 +12,13 @@ import type { FioriRuleDefinition } from '../types'; import { TEXT_ARRANGEMENT_HIDDEN, type TextArrangementHidden } from '../language/diagnostics'; import { buildAnnotationIndexKey } from '../project-context/parser'; import type { IndexedAnnotation, ParsedService } from '../project-context/parser'; -import { COMMON_TEXT, UI_HIDDEN, UI_TEXT_ARRANGEMENT } from '../constants'; -import { type AnyPage, resolveTextPropertyPath, collectRelevantEntityTypes } from './utils/common-text-helpers'; +import { UI_HIDDEN, UI_TEXT_ARRANGEMENT } from '../constants'; +import { + type AnyPage, + resolveTextPropertyPath, + collectRelevantEntityTypes, + parseCommonTextAnnotationKey +} from './utils/common-text-helpers'; /** * Checks whether a Common.Text annotation element has a nested UI.TextArrangement inline annotation. @@ -172,27 +177,11 @@ function processAnnotationEntry( parsedService: ParsedService, relevantEntityTypes: Map ): TextArrangementHidden[] { - const atIdx = annotationKey.indexOf('/@'); - if (atIdx === -1) { - return []; - } - const targetPath = annotationKey.substring(0, atIdx); - const term = annotationKey.substring(atIdx + 2); - // Only process Common.Text annotations - if (term !== COMMON_TEXT) { - return []; - } - // Only handle property-level annotations (path must contain '/') - const slashIdx = targetPath.indexOf('/'); - if (slashIdx === -1) { - return []; - } - const entityTypeName = targetPath.substring(0, slashIdx); - // Only check entity types that are actually used on pages - const pageNames = relevantEntityTypes.get(entityTypeName); - if (!pageNames) { + const parsed = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + if (!parsed) { return []; } + const { targetPath, entityTypeName, pageNames } = parsed; const problems: TextArrangementHidden[] = []; for (const textAnnotation of Object.values(qualifiedAnnotations)) { problems.push( diff --git a/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts b/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts index 54211dd8f66..b4fd089b0d1 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts @@ -1,6 +1,7 @@ import type { IndexedAnnotation, ParsedService } from '../../project-context/parser'; import type { FeV4ObjectPage, FeV4ListReport } from '../../project-context/linker/fe-v4'; import type { FeV2ListReport, FeV2ObjectPage } from '../../project-context/linker/fe-v2'; +import { COMMON_TEXT } from '../../constants'; export type AnyPage = FeV4ObjectPage | FeV4ListReport | FeV2ListReport | FeV2ObjectPage; @@ -109,3 +110,43 @@ export function collectRelevantEntityTypes(pages: AnyPage[], parsedService: Pars } return entityTypePages; } + +/** + * Parses an annotation index key and validates it is a property-level Common.Text + * annotation targeting a known entity type. + * + * Returns the parsed `targetPath`, `entityTypeName`, and `pageNames` when the key + * matches — or `undefined` for any of these conditions: + * - the key has no `/@` separator + * - the term is not `Common.Text` + * - the target path has no `/` (entity-type-level, not property-level) + * - the entity type is not present in `relevantEntityTypes` + * + * @param annotationKey - Annotation index key (e.g. "Service.Entity/prop/@Common.Text") + * @param relevantEntityTypes - Entity types used in the app, mapped to page names + * @returns Parsed values, or undefined if the key does not qualify + */ +export function parseCommonTextAnnotationKey( + annotationKey: string, + relevantEntityTypes: Map +): { targetPath: string; entityTypeName: string; pageNames: string[] } | undefined { + const atIdx = annotationKey.indexOf('/@'); + if (atIdx === -1) { + return undefined; + } + const targetPath = annotationKey.substring(0, atIdx); + const term = annotationKey.substring(atIdx + 2); + if (term !== COMMON_TEXT) { + return undefined; + } + const slashIdx = targetPath.indexOf('/'); + if (slashIdx === -1) { + return undefined; + } + const entityTypeName = targetPath.substring(0, slashIdx); + const pageNames = relevantEntityTypes.get(entityTypeName); + if (!pageNames) { + return undefined; + } + return { targetPath, entityTypeName, pageNames }; +} diff --git a/packages/eslint-plugin-fiori-tools/test/rules/utils/common-text-helpers.test.ts b/packages/eslint-plugin-fiori-tools/test/rules/utils/common-text-helpers.test.ts new file mode 100644 index 00000000000..3ec9b247d70 --- /dev/null +++ b/packages/eslint-plugin-fiori-tools/test/rules/utils/common-text-helpers.test.ts @@ -0,0 +1,89 @@ +import { parseCommonTextAnnotationKey } from '../../../src/rules/utils/common-text-helpers'; + +describe('parseCommonTextAnnotationKey', () => { + const relevantEntityTypes = new Map([ + ['Service.Entity', ['MainPage']], + ['Service.Other', ['OtherPage', 'SecondPage']] + ]); + + describe('when the annotation key does not contain /@', () => { + it('should return undefined', () => { + // given + const annotationKey = 'Service.Entity/property/Common.Text'; + // when + const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + // then + expect(result).toBeUndefined(); + }); + }); + + describe('when the term is not Common.Text', () => { + it('should return undefined for a different term', () => { + // given + const annotationKey = 'Service.Entity/property/@Common.Label'; + // when + const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + // then + expect(result).toBeUndefined(); + }); + + it('should return undefined for UI.TextArrangement', () => { + // given + const annotationKey = 'Service.Entity/@com.sap.vocabularies.UI.v1.TextArrangement'; + // when + const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + // then + expect(result).toBeUndefined(); + }); + }); + + describe('when the target path has no / (entity-type level, not property level)', () => { + it('should return undefined', () => { + // given + const annotationKey = 'Service.Entity/@Common.Text'; + // when + const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + // then + expect(result).toBeUndefined(); + }); + }); + + describe('when the entity type is not in relevantEntityTypes', () => { + it('should return undefined', () => { + // given + const annotationKey = 'Service.Unknown/property/@Common.Text'; + // when + const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + // then + expect(result).toBeUndefined(); + }); + }); + + describe('when the annotation key is a valid property-level Common.Text for a known entity type', () => { + it('should return the parsed targetPath, entityTypeName and pageNames', () => { + // given + const annotationKey = 'Service.Entity/property/@Common.Text'; + // when + const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + // then + expect(result).toEqual({ + targetPath: 'Service.Entity/property', + entityTypeName: 'Service.Entity', + pageNames: ['MainPage'] + }); + }); + + it('should return all page names when the entity type appears on multiple pages', () => { + // given + const annotationKey = 'Service.Other/prop/@Common.Text'; + // when + const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + // then + expect(result).toEqual({ + targetPath: 'Service.Other/prop', + entityTypeName: 'Service.Other', + pageNames: ['OtherPage', 'SecondPage'] + }); + }); + }); +}); From 3fa3d9da0ababd0abc6ff5ea200cc9e0b7f29bb1 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Wed, 22 Apr 2026 10:53:30 +0200 Subject: [PATCH 14/17] fix: replace annotation keys with fully-qualified term names in tests --- .../rules/utils/common-text-helpers.test.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin-fiori-tools/test/rules/utils/common-text-helpers.test.ts b/packages/eslint-plugin-fiori-tools/test/rules/utils/common-text-helpers.test.ts index 3ec9b247d70..12f68fa4eb7 100644 --- a/packages/eslint-plugin-fiori-tools/test/rules/utils/common-text-helpers.test.ts +++ b/packages/eslint-plugin-fiori-tools/test/rules/utils/common-text-helpers.test.ts @@ -6,10 +6,14 @@ describe('parseCommonTextAnnotationKey', () => { ['Service.Other', ['OtherPage', 'SecondPage']] ]); + const COMMON_TEXT_TERM = 'com.sap.vocabularies.Common.v1.Text'; + const COMMON_LABEL_TERM = 'com.sap.vocabularies.Common.v1.Label'; + const UI_TEXT_ARRANGEMENT_TERM = 'com.sap.vocabularies.UI.v1.TextArrangement'; + describe('when the annotation key does not contain /@', () => { it('should return undefined', () => { // given - const annotationKey = 'Service.Entity/property/Common.Text'; + const annotationKey = `Service.Entity/property/${COMMON_TEXT_TERM}`; // when const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); // then @@ -20,7 +24,7 @@ describe('parseCommonTextAnnotationKey', () => { describe('when the term is not Common.Text', () => { it('should return undefined for a different term', () => { // given - const annotationKey = 'Service.Entity/property/@Common.Label'; + const annotationKey = `Service.Entity/property/@${COMMON_LABEL_TERM}`; // when const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); // then @@ -29,7 +33,7 @@ describe('parseCommonTextAnnotationKey', () => { it('should return undefined for UI.TextArrangement', () => { // given - const annotationKey = 'Service.Entity/@com.sap.vocabularies.UI.v1.TextArrangement'; + const annotationKey = `Service.Entity/@${UI_TEXT_ARRANGEMENT_TERM}`; // when const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); // then @@ -40,7 +44,7 @@ describe('parseCommonTextAnnotationKey', () => { describe('when the target path has no / (entity-type level, not property level)', () => { it('should return undefined', () => { // given - const annotationKey = 'Service.Entity/@Common.Text'; + const annotationKey = `Service.Entity/@${COMMON_TEXT_TERM}`; // when const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); // then @@ -51,7 +55,7 @@ describe('parseCommonTextAnnotationKey', () => { describe('when the entity type is not in relevantEntityTypes', () => { it('should return undefined', () => { // given - const annotationKey = 'Service.Unknown/property/@Common.Text'; + const annotationKey = `Service.Unknown/property/@${COMMON_TEXT_TERM}`; // when const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); // then @@ -62,7 +66,7 @@ describe('parseCommonTextAnnotationKey', () => { describe('when the annotation key is a valid property-level Common.Text for a known entity type', () => { it('should return the parsed targetPath, entityTypeName and pageNames', () => { // given - const annotationKey = 'Service.Entity/property/@Common.Text'; + const annotationKey = `Service.Entity/property/@${COMMON_TEXT_TERM}`; // when const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); // then @@ -75,7 +79,7 @@ describe('parseCommonTextAnnotationKey', () => { it('should return all page names when the entity type appears on multiple pages', () => { // given - const annotationKey = 'Service.Other/prop/@Common.Text'; + const annotationKey = `Service.Other/prop/@${COMMON_TEXT_TERM}`; // when const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); // then From 5d8bc7af4a223a50359390bcc529d1b009b41a80 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Thu, 23 Apr 2026 13:22:48 +0200 Subject: [PATCH 15/17] refactor: show issues on the Common.Label annotation and not on the Common.Text annotation --- .../src/language/diagnostics.ts | 1 + .../src/project-context/parser/service.ts | 13 ++++-- .../src/rules/sap-description-column-label.ts | 40 +++++++++++++------ 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/packages/eslint-plugin-fiori-tools/src/language/diagnostics.ts b/packages/eslint-plugin-fiori-tools/src/language/diagnostics.ts index 4235ac33d2a..0991c60b0e8 100644 --- a/packages/eslint-plugin-fiori-tools/src/language/diagnostics.ts +++ b/packages/eslint-plugin-fiori-tools/src/language/diagnostics.ts @@ -148,6 +148,7 @@ export interface DescriptionColumnLabel { messageId: DescriptionColumnLabelMessageId; pageNames: string[]; annotation: { + /** Reference to the Common.Label annotation of the text property (the reported node) */ reference: AnnotationReference; idPropertyTarget: string; textPropertyTarget: string; diff --git a/packages/eslint-plugin-fiori-tools/src/project-context/parser/service.ts b/packages/eslint-plugin-fiori-tools/src/project-context/parser/service.ts index 5dab7ba1219..f20a3fe5a05 100644 --- a/packages/eslint-plugin-fiori-tools/src/project-context/parser/service.ts +++ b/packages/eslint-plugin-fiori-tools/src/project-context/parser/service.ts @@ -162,9 +162,9 @@ function createSyntheticElement(attrName: string, attrValue: string, sourceRange * Injects synthetic Common.Text and Common.Label annotation entries derived from OData V2 * inline `sap:text` and `sap:label` attributes on Property elements. * - * For Common.Text: injects into both the annotation index AND the annotation file AST so - * that ESLint's `createAnnotations()` traversal can fire and report diagnostics. - * For Common.Label: injects into the annotation index only (read by getLabelForProperty). + * Both Common.Text and Common.Label are injected into the annotation index AND the annotation + * file AST so that ESLint's `createAnnotations()` traversal can fire and report diagnostics + * on either annotation type. * * Only adds entries when no explicit vocabulary annotation already exists for that key. * @@ -223,6 +223,13 @@ function injectV2InlineAnnotations(artifacts: ServiceArtifacts, metadataUri: str layers: [{ uri: metadataUri, value: syntheticElement }] } }; + // Also inject into the annotation file AST so the traversal selector fires on the label node + const syntheticTarget: Target = { + type: 'target', + name: propertyTarget, + terms: [syntheticElement] + }; + annotationFile.targets.push(syntheticTarget); } } }); diff --git a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts index ecbd83f2631..b8887b61a1b 100644 --- a/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts +++ b/packages/eslint-plugin-fiori-tools/src/rules/sap-description-column-label.ts @@ -19,13 +19,16 @@ import { const TRIVIAL_LABELS = new Set(['name', 'description']); /** - * Reads a Common.Label String value from the annotation index. + * Reads a Common.Label String value and the corresponding annotation from the annotation index. * * @param propertyTarget - Fully-qualified target path (e.g. "Service.Entity/prop") * @param service - The parsed OData service - * @returns The label string, or undefined if not found + * @returns The label string and annotation, or undefined if not found */ -function getLabelForProperty(propertyTarget: string, service: ParsedService): string | undefined { +function getLabelForProperty( + propertyTarget: string, + service: ParsedService +): { label: string; annotation: IndexedAnnotation } | undefined { const labelKey = buildAnnotationIndexKey(propertyTarget, COMMON_LABEL); const labelAnnotations = service.index.annotations[labelKey]; if (!labelAnnotations) { @@ -35,12 +38,20 @@ function getLabelForProperty(propertyTarget: string, service: ParsedService): st if (!annotation) { return undefined; } - return getElementAttributeValue(annotation.top.value, Edm.String) || undefined; + const label = getElementAttributeValue(annotation.top.value, Edm.String) || undefined; + if (!label) { + return undefined; + } + return { label, annotation }; } /** * Checks a single Common.Text annotation for a non-descriptive text property label. * + * The issue is reported on the Common.Label annotation of the text property (the referenced + * annotation), because that is the annotation the developer needs to fix. The idPropertyTarget + * (which has the Common.Text annotation) is included in the message for context. + * * @param textAnnotation - The indexed Common.Text annotation * @param entityTypeName - Entity type that owns the annotated property * @param targetPath - Annotation target (the ID property path) @@ -67,14 +78,16 @@ function processTextAnnotation( } const textPropertyTarget = `${resolved.entityTypeName}/${resolved.propertyName}`; - const textPropertyLabel = getLabelForProperty(textPropertyTarget, parsedService); - if (!textPropertyLabel) { + const textPropertyLabelResult = getLabelForProperty(textPropertyTarget, parsedService); + if (!textPropertyLabelResult) { return []; } + const { label: textPropertyLabel, annotation: labelAnnotation } = textPropertyLabelResult; const labelLower = textPropertyLabel.trim().toLowerCase(); // Check 1: trivial generic labels + // Issue reported on the Common.Label annotation (labelAnnotation.top) — that is what needs fixing. if (TRIVIAL_LABELS.has(labelLower)) { return [ { @@ -82,7 +95,7 @@ function processTextAnnotation( messageId: 'trivialLabel', pageNames, annotation: { - reference: textAnnotation.top, + reference: labelAnnotation.top, idPropertyTarget: targetPath, textPropertyTarget, textPropertyLabel @@ -92,19 +105,20 @@ function processTextAnnotation( } // Check 2: label of text property identical to label of ID property - const idPropertyLabel = getLabelForProperty(targetPath, parsedService); - if (idPropertyLabel?.trim().toLowerCase() === labelLower) { + // Issue reported on the Common.Label annotation of the text property — that is what needs fixing. + const idPropertyLabelResult = getLabelForProperty(targetPath, parsedService); + if (idPropertyLabelResult?.label.trim().toLowerCase() === labelLower) { return [ { type: DESCRIPTION_COLUMN_LABEL, messageId: 'duplicateLabel', pageNames, annotation: { - reference: textAnnotation.top, + reference: labelAnnotation.top, idPropertyTarget: targetPath, textPropertyTarget, textPropertyLabel, - idPropertyLabel + idPropertyLabel: idPropertyLabelResult.label } } ]; @@ -172,9 +186,9 @@ const rule: FioriRuleDefinition = createFioriRule({ }, messages: { trivialLabel: - 'The "{{textPropertyTarget}}" text property has a "{{textPropertyLabel}}" generic label. Use a more descriptive label that distinguishes it from other properties.', + 'The "{{textPropertyTarget}}" text property has a "{{textPropertyLabel}}" generic label. Use a more descriptive label that distinguishes it from other properties', duplicateLabel: - 'The "{{textPropertyTarget}}" text property has the same "{{textPropertyLabel}}" label as the "{{idPropertyTarget}}" ID property. The description column label must be different from the ID label.' + 'The "{{textPropertyTarget}}" text property has the same "{{textPropertyLabel}}" label as the "{{idPropertyTarget}}" ID property. The description column label must be different from the ID label' } }, From 8c823507aea6b58003bbf668973e5f026a8515a0 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Fri, 24 Apr 2026 09:31:09 +0200 Subject: [PATCH 16/17] test: update synthetic target injection to include Common.Label annotations --- .../project-context/parser/service.test.ts | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts b/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts index 216ca124e2c..c2d70649f51 100644 --- a/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts +++ b/packages/eslint-plugin-fiori-tools/test/project-context/parser/service.test.ts @@ -173,8 +173,8 @@ describe('buildServiceIndex', () => { expect(index.annotations[nameTextKey]).toBeUndefined(); }); - test('injects a synthetic target into the annotation file AST for Common.Text', () => { - // Given a V2 metadata service with a property having sap:text + test('injects synthetic targets into the annotation file AST for Common.Text and Common.Label', () => { + // Given a V2 metadata service with properties having sap:text and sap:label const annotationFile = createAnnotationFile(METADATA_URI); const artifacts: ServiceArtifacts = { path: '/test/service', @@ -188,10 +188,27 @@ describe('buildServiceIndex', () => { // When building the service index buildServiceIndex(artifacts, documents); - // Then the annotation file has a synthetic target for the ID property - expect(annotationFile.targets).toHaveLength(1); - expect(annotationFile.targets[0].name).toBe(ID_PROP_PATH); - expect(annotationFile.targets[0].terms).toHaveLength(1); + // Then the annotation file has synthetic targets for: + // 1. OrderID Common.Text (sap:text="OrderName") + // 2. OrderID Common.Label (sap:label="Order ID") + // 3. OrderName Common.Label (sap:label="Order Name") + expect(annotationFile.targets).toHaveLength(3); + + const textTarget = annotationFile.targets.find( + (t) => t.name === ID_PROP_PATH && t.terms[0]?.attributes?.['Path'] !== undefined + ); + expect(textTarget).toBeDefined(); + expect(textTarget!.name).toBe(ID_PROP_PATH); + + const idLabelTarget = annotationFile.targets.find( + (t) => t.name === ID_PROP_PATH && t.terms[0]?.attributes?.['String'] !== undefined + ); + expect(idLabelTarget).toBeDefined(); + expect(idLabelTarget!.name).toBe(ID_PROP_PATH); + + const nameLabelTarget = annotationFile.targets.find((t) => t.name === NAME_PROP_PATH); + expect(nameLabelTarget).toBeDefined(); + expect(nameLabelTarget!.name).toBe(NAME_PROP_PATH); }); test('does NOT overwrite an explicit Common.Text annotation already in the index', () => { From 8b64f9c9ed2e59759da173a4d00ab0208b252bc5 Mon Sep 17 00:00:00 2001 From: Marco Vieth Date: Tue, 28 Apr 2026 09:25:29 +0200 Subject: [PATCH 17/17] set other packages in the changeset to patch --- .changeset/rich-meals-shake.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/rich-meals-shake.md b/.changeset/rich-meals-shake.md index cc2ae38632d..0beaf3e8647 100644 --- a/.changeset/rich-meals-shake.md +++ b/.changeset/rich-meals-shake.md @@ -1,7 +1,7 @@ --- '@sap-ux/xml-odata-annotation-converter': minor -'@sap-ux/odata-annotation-core-types': minor -'@sap-ux/eslint-plugin-fiori-tools': minor +'@sap-ux/odata-annotation-core-types': patch +'@sap-ux/eslint-plugin-fiori-tools': patch --- [rule] Add rule to check that a Common.Text description property has a meaningful Common.Label annotation