diff --git a/.changeset/rich-meals-shake.md b/.changeset/rich-meals-shake.md new file mode 100644 index 00000000000..cc2ae38632d --- /dev/null +++ b/.changeset/rich-meals-shake.md @@ -0,0 +1,7 @@ +--- +'@sap-ux/xml-odata-annotation-converter': minor +'@sap-ux/odata-annotation-core-types': minor +'@sap-ux/eslint-plugin-fiori-tools': minor +--- + +[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/README.md b/packages/eslint-plugin-fiori-tools/README.md index 109cee025ae..8777bc4c5e1 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 | |:---------:|------|-------------|:-----------:|:-----------------------:| -| new | [sap-text-arrangement-hidden](docs/rules/sap-text-arrangement-hidden.md) | Ensures that the text property referenced by a `UI.TextArrangement` annotation using the `Common.Text` annotation is not hidden by the `UI.Hidden` annotation | | ✅ | +| 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.12.0 | [sap-text-arrangement-hidden](docs/rules/sap-text-arrangement-hidden.md) | Ensures that the text property referenced by a `UI.TextArrangement` annotation using the `Common.Text` annotation is not hidden by the `UI.Hidden` annotation | | ✅ | | 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 | | ✅ | 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..e64dd3c925b --- /dev/null +++ b/packages/eslint-plugin-fiori-tools/docs/rules/sap-description-column-label.md @@ -0,0 +1,133 @@ +# Require Meaningful Labels for Description (Text) Properties (`sap-description-column-label`) + +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 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, 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 which includes navigation segments such as `category/name`. +2. Reads the `Common.Label` annotation on that property. +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 produce a warning. + +> **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, 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` +``` +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. +``` + +### Incorrect Annotations + +`trivialLabel`: The `Common.Label` annotation on the text property is too generic: + +```xml + + + + + + + + + +``` + +`duplicateLabel`: The `Common.Label` annotation on the text property is identical to the ID property label: + +```xml + + + + + + + + + + +``` + +`trivialLabel` for OData V2: The inline `sap:label` on the text property is too generic. This is reported on the metadata file: + +```xml + + + + +``` + +`duplicateLabel` for OData V2: The inline `sap:label` values on ID and text property are identical: + +```xml + + + +``` + +### Correct Annotations + +The `Common.Label` annotation on the text property is meaningful and unique: + +```xml + + + + + + + + +``` + +The `Common.Label` annotations on ID and text properties are unique: + +```xml + + + + + + + + +``` + +The `sap:label` values on ID and text property for OData V2 are unique: + +```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 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 + +- [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) diff --git a/packages/eslint-plugin-fiori-tools/src/constants.ts b/packages/eslint-plugin-fiori-tools/src/constants.ts index 9e0fd0e9a30..22351c05de5 100644 --- a/packages/eslint-plugin-fiori-tools/src/constants.ts +++ b/packages/eslint-plugin-fiori-tools/src/constants.ts @@ -1,5 +1,6 @@ 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_HIDDEN = 'com.sap.vocabularies.UI.v1.Hidden'; export const UI_TEXT_ARRANGEMENT = 'com.sap.vocabularies.UI.v1.TextArrangement'; 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 443e5d935eb..ac885b1a2a6 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 66bfda5e298..0991c60b0e8 100644 --- a/packages/eslint-plugin-fiori-tools/src/language/diagnostics.ts +++ b/packages/eslint-plugin-fiori-tools/src/language/diagnostics.ts @@ -16,6 +16,7 @@ export const TEXT_ARRANGEMENT_HIDDEN = 'sap-text-arrangement-hidden'; 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; @@ -138,6 +139,24 @@ 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 to the Common.Label annotation of the text property (the reported node) */ + reference: AnnotationReference; + idPropertyTarget: string; + textPropertyTarget: string; + textPropertyLabel: string; + idPropertyLabel?: string; + }; +} + export interface TextArrangementHidden { type: typeof TEXT_ARRANGEMENT_HIDDEN; pageNames: string[]; @@ -154,6 +173,7 @@ export type Diagnostic = | FlexEnabled | CopyToClipboard | CreationModeForTable + | DescriptionColumnLabel | EnableExport | EnablePaste | StatePreservationMode 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..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 @@ -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,112 @@ 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. + * + * 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. + * + * @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 }] + } + }; + // 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); + } + } + }); +} + /** * 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 +256,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 +264,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/eslint-plugin-fiori-tools/src/rules/index.ts b/packages/eslint-plugin-fiori-tools/src/rules/index.ts index 139640b6331..39d525b686c 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, @@ -74,6 +75,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'; @@ -137,6 +139,7 @@ export const rules: Record +): DescriptionColumnLabel[] { + const parsed = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + if (!parsed) { + return []; + } + const { targetPath, entityTypeName, pageNames } = parsed; + 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. + * + * @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, + 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 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 "{{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' + } + }, + + check(context) { + 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: Element): void { + const diagnostic = lookup.get(node); + if (!diagnostic) { + return; + } + context.report({ + node, + messageId: diagnostic.messageId, + 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/src/rules/sap-text-arrangement-hidden.ts b/packages/eslint-plugin-fiori-tools/src/rules/sap-text-arrangement-hidden.ts index e0b87c75f50..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,52 +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 { 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 { 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. @@ -94,71 +55,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. * @@ -281,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 new file mode 100644 index 00000000000..b4fd089b0d1 --- /dev/null +++ b/packages/eslint-plugin-fiori-tools/src/rules/utils/common-text-helpers.ts @@ -0,0 +1,152 @@ +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; + +/** + * 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 || segments[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; +} + +/** + * 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/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..216ca124e2c --- /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'; +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 new file mode 100644 index 00000000000..aeeac448606 --- /dev/null +++ b/packages/eslint-plugin-fiori-tools/test/rules/sap-description-column-label.test.ts @@ -0,0 +1,229 @@ +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, + 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 } }, + 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, 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 = ` + + + + + + `; + +// 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: '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', + 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' }] + }, + [] + ) + ] +}); + +// ─── 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', + 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' + } + } + ] + }, + [] + ) + ] +}); 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..12f68fa4eb7 --- /dev/null +++ b/packages/eslint-plugin-fiori-tools/test/rules/utils/common-text-helpers.test.ts @@ -0,0 +1,93 @@ +import { parseCommonTextAnnotationKey } from '../../../src/rules/utils/common-text-helpers'; + +describe('parseCommonTextAnnotationKey', () => { + const relevantEntityTypes = new Map([ + ['Service.Entity', ['MainPage']], + ['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_TERM}`; + // 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_TERM}`; + // when + const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + // then + expect(result).toBeUndefined(); + }); + + it('should return undefined for UI.TextArrangement', () => { + // given + const annotationKey = `Service.Entity/@${UI_TEXT_ARRANGEMENT_TERM}`; + // 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_TERM}`; + // 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_TERM}`; + // 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_TERM}`; + // 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_TERM}`; + // when + const result = parseCommonTextAnnotationKey(annotationKey, relevantEntityTypes); + // then + expect(result).toEqual({ + targetPath: 'Service.Other/prop', + entityTypeName: 'Service.Other', + pageNames: ['OtherPage', 'SecondPage'] + }); + }); + }); +}); 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 = {}; 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}` 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(`