FEAT: add sap-no-deep-collection-facets eslint rule by vinayhospete · Pull Request #5174 · SAP/open-ux-tools

vinayhospete · 2026-09-11T07:49:59Z

Add ESLint rule to flag deeply nested CollectionFacets

Description

Adds the new sap-no-deep-collection-facets ESLint rule for Fiori annotation sources. The rule reports UI.CollectionFacet records nested at the third level or deeper in UI.Facets, since SAP Fiori elements only considers up to two levels.

This PR includes:

New rule implementation and diagnostics for sap-no-deep-collection-facets
Registration of the new rule in the plugin rule index and recommended configuration
Documentation with valid/invalid examples and remediation guidance
Unit tests covering V2/V4 annotations, qualified annotations, valid two-level nesting, and multiple nested violations
README rule table updates and changeset for a minor release

Type of change

Bug (non-breaking change which fixes an issue)
New feature (non-breaking change which adds a new feature)
Breaking change (Bug or New feature that would cause existing functionality/consumers to not work as expected)
Non-Breaking chores (Changes to tools, libraries, build process, documentation, etc)
None of the above (Reviewers might ask for more clarification)

How have you tested?

Added RuleTester coverage for the new rule, including:

Valid single-level and two-level facet structures
Invalid three-level and four-level nested UI.CollectionFacet structures
V2 and V4 annotation scenarios
Qualified UI.Facets annotations
Object-page-only validation behavior

Checklist:

The code conforms to the general development principles
Supplied as many details as possible on this change
The code is easy to read and maintainable by others
Corresponding changes to the documentation has been done
Already existing and new unit tests pass locally
I have reviewed and addressed all Hyperspace bot findings (or explicitly explained dismissals)
I have done an Agentic review
🔄 Regenerate and Update Summary

PR Bot Information

Version: 1.31.30

Event Trigger: pull_request.opened
File Content Strategy: Full file content
Output Template: Repository PR Template
LLM: gpt-5.5
Summary Prompt: Default Prompt
Correlation ID: 63b2e3f0-adb5-11f1-9bbb-f0333a3e7b8d

changeset-bot · 2026-09-11T07:50:07Z

🦋 Changeset detected

Latest commit: 7ee7ed7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sap-ux/eslint-plugin-fiori-tools	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

hyperspace-pr-bot

I found a few build/type-safety issues that should be addressed before merge, including the missing diagnostic union entries, a registered rule whose implementation is absent, and an incompatible minimatch import for the currently declared dependency version. The new rule is otherwise well covered with focused test scenarios.

PR Bot Information

Version: 1.31.30

Event Trigger: pull_request.opened
File Content Strategy: Full file content
LLM: gpt-5.5
Correlation ID: 63b2e3f0-adb5-11f1-9bbb-f0333a3e7b8d
Agent Instructions:
- AGENTS.md
- CLAUDE.md

hyperspace-pr-bot · 2026-09-11T07:51:05Z

+export interface NoDeepCollectionFacets {
+    type: typeof NO_DEEP_COLLECTION_FACETS;
+    pageNames: string[];
+    annotation: {
+        reference: AnnotationReference;
+        reportedParent: Element;
+    };
+}


Bug: New diagnostic types are not included in the Diagnostic union

createFioriRule constrains rule IDs and validation results to Diagnostic['type'], so the new rules will fail type checking or infer never until their interfaces are added to the union. Consider adding both NoSingleFacetInCollection and NoDeepCollectionFacets to Diagnostic.

Please provide feedback on the review comment by checking the appropriate box:

🌟 Awesome comment, a human might have missed that.

✅ Helpful comment

🤷 Neutral

❌ This comment is not helpful

hyperspace-pr-bot · 2026-09-11T07:51:05Z

 import textArrangementHidden from './sap-text-arrangement-hidden.js';
 import noLiveMode from './sap-no-live-mode.js';
 import cloudDevAdaptationStatus from './sap-cloud-dev-adaptation-status.js';
+import noSingleFacetInCollection from './sap-no-single-facet-in-collection.js';


Bug: Missing rule implementation is imported

This import points to sap-no-single-facet-in-collection.js, but that rule file is not present in the PR, so the package will fail to build when resolving the module. Consider adding the missing rule implementation and docs/tests, or remove its README/config/index registrations from this PR.

Please provide feedback on the review comment by checking the appropriate box:

🌟 Awesome comment, a human might have missed that.

✅ Helpful comment

🤷 Neutral

❌ This comment is not helpful

lfindlaysap · 2026-09-11T10:55:35Z

@@ -0,0 +1,208 @@
+# UI.CollectionFacet should not be nested at third level or deeper (sap-no-deep-collection-facets)


Suggested change

# UI.CollectionFacet should not be nested at third level or deeper (sap-no-deep-collection-facets)

# UI.CollectionFacet UI.collectionfacet Should Not Be Nested at Third Level or Deeper (`sap-no-deep-collection-facets`)

lfindlaysap · 2026-09-11T10:55:56Z

@@ -0,0 +1,208 @@
+# UI.CollectionFacet should not be nested at third level or deeper (sap-no-deep-collection-facets)
+
+SAP Fiori elements does not consider `UI.CollectionFacet` elements that are nested at the third level or deeper within the `UI.Facets` annotation. This rule detects deeply nested collection facets and recommends reorganizing the facet structure to use a maximum of two nesting levels for proper rendering and functionality.


Suggested change

SAP Fiori elements does not consider `UI.CollectionFacet` elements that are nested at the third level or deeper within the `UI.Facets` annotation. This rule detects deeply nested collection facets and recommends reorganizing the facet structure to use a maximum of two nesting levels for proper rendering and functionality.

SAP Fiori elements does not consider `UI.CollectionFacet` elements that are nested at the third level or deeper within the `UI.Facets` annotation. This rule detects deeply nested collection facets and recommends reorganizing the facet structure to use a maximum of two nesting levels.

lfindlaysap · 2026-09-11T10:56:10Z

+
+- **Level 1** (direct children of `UI.Facets`): ✅ Supported
+- **Level 2** (children of level 1 `UI.CollectionFacet`): ✅ Supported  
+- **Level 3 and deeper**: ❌ Not considered by Fiori elements


Suggested change

- **Level 3 and deeper**: ❌ Not considered by Fiori elements

- **Level 3 and deeper**: ❌ Not considered by SAP fiori elements

lfindlaysap · 2026-09-11T10:56:38Z

+
+Reorganize your facet structure to flatten deeply nested `UI.CollectionFacet` elements. Consider one of these approaches:
+
+1. **Remove unnecessary nesting:** If a `UI.CollectionFacet` contains only one child, replace it with a direct `UI.ReferenceFacet`.


Suggested change

1. **Remove unnecessary nesting:** If a `UI.CollectionFacet` contains only one child, replace it with a direct `UI.ReferenceFacet`.

- **Remove unnecessary nesting:** If a `UI.CollectionFacet` contains only one child, replace it with a direct `UI.ReferenceFacet`.

lfindlaysap · 2026-09-11T10:56:47Z

+
+1. **Remove unnecessary nesting:** If a `UI.CollectionFacet` contains only one child, replace it with a direct `UI.ReferenceFacet`.
+
+2. **Flatten the hierarchy:** Move nested content up to a higher level by combining or reorganizing sections.


Suggested change

2. **Flatten the hierarchy:** Move nested content up to a higher level by combining or reorganizing sections.

- **Flatten the hierarchy:** Move nested content up to a higher level by combining or reorganizing sections.

lfindlaysap · 2026-09-11T10:56:56Z

+
+2. **Flatten the hierarchy:** Move nested content up to a higher level by combining or reorganizing sections.
+
+3. **Use side-by-side facets:** Place facets at the same level rather than nesting them deeply.


Suggested change

3. **Use side-by-side facets:** Place facets at the same level rather than nesting them deeply.

- **Use side-by-side facets:** Place facets at the same level rather than nesting them deeply.

lfindlaysap · 2026-09-11T10:57:43Z

+
+## Bug Report
+
+In case you detect an issue with this rule, please open a GitHub issue [here](https://github.com/SAP/open-ux-tools/issues).


Suggested change

In case you detect an issue with this rule, please open a GitHub issue [here](https://github.com/SAP/open-ux-tools/issues).

If you encounter any problems, open a [GitHub issue](https://github.com/SAP/open-ux-tools/issues).

lfindlaysap · 2026-09-11T10:58:23Z

+
+3. **Use side-by-side facets:** Place facets at the same level rather than nesting them deeply.
+
+**Before (3 levels - violation):**


Suggested change

**Before (3 levels - violation):**

**Before (Three levels: violation):**

lfindlaysap · 2026-09-11T10:58:34Z

+</Record>
+```
+
+**After (2 levels - correct):**


Suggested change

**After (2 levels - correct):**

**After (Two levels: correct):**

feat: UI.CollectionFacets at third level and beyond are not considered

ae7d334

vinayhospete requested review from a team as code owners September 11, 2026 07:50

vinayhospete had a problem deploying to ci-secrets September 11, 2026 07:50 — with GitHub Actions Failure

vinayhospete had a problem deploying to ci-secrets September 11, 2026 07:50 — with GitHub Actions Error

hyperspace-pr-bot Bot reviewed Sep 11, 2026

View reviewed changes

feat: revert accidental change

b94875c

vinayhospete had a problem deploying to ci-secrets September 11, 2026 07:56 — with GitHub Actions Failure

vinayhospete had a problem deploying to ci-secrets September 11, 2026 07:56 — with GitHub Actions Error

vinayhospete had a problem deploying to ci-secrets September 11, 2026 07:56 — with GitHub Actions Failure

vinayhospete had a problem deploying to ci-secrets September 11, 2026 07:56 — with GitHub Actions Error

feat: revert accidental change

1fb58e5

vinayhospete had a problem deploying to ci-secrets September 11, 2026 08:01 — with GitHub Actions Failure

vinayhospete had a problem deploying to ci-secrets September 11, 2026 08:01 — with GitHub Actions Error

vinayhospete had a problem deploying to ci-secrets September 11, 2026 08:01 — with GitHub Actions Failure

vinayhospete had a problem deploying to ci-secrets September 11, 2026 08:01 — with GitHub Actions Error

update doc link

7ee7ed7

vinayhospete had a problem deploying to ci-secrets September 11, 2026 08:07 — with GitHub Actions Failure

vinayhospete had a problem deploying to ci-secrets September 11, 2026 08:07 — with GitHub Actions Error

vinayhospete had a problem deploying to ci-secrets September 11, 2026 08:07 — with GitHub Actions Failure

vinayhospete had a problem deploying to ci-secrets September 11, 2026 08:07 — with GitHub Actions Error

lfindlaysap requested changes Sep 11, 2026

View reviewed changes

		@@ -0,0 +1,208 @@
		# UI.CollectionFacet should not be nested at third level or deeper (sap-no-deep-collection-facets)

	# UI.CollectionFacet should not be nested at third level or deeper (sap-no-deep-collection-facets)
	# UI.CollectionFacet UI.collectionfacet Should Not Be Nested at Third Level or Deeper (`sap-no-deep-collection-facets`)

		@@ -0,0 +1,208 @@
		# UI.CollectionFacet should not be nested at third level or deeper (sap-no-deep-collection-facets)

		SAP Fiori elements does not consider `UI.CollectionFacet` elements that are nested at the third level or deeper within the `UI.Facets` annotation. This rule detects deeply nested collection facets and recommends reorganizing the facet structure to use a maximum of two nesting levels for proper rendering and functionality.

	- Level 3 and deeper: ❌ Not considered by Fiori elements
	- Level 3 and deeper: ❌ Not considered by SAP fiori elements


		Reorganize your facet structure to flatten deeply nested `UI.CollectionFacet` elements. Consider one of these approaches:

		1. Remove unnecessary nesting: If a `UI.CollectionFacet` contains only one child, replace it with a direct `UI.ReferenceFacet`.

	1. Remove unnecessary nesting: If a `UI.CollectionFacet` contains only one child, replace it with a direct `UI.ReferenceFacet`.
	- Remove unnecessary nesting: If a `UI.CollectionFacet` contains only one child, replace it with a direct `UI.ReferenceFacet`.


		1. Remove unnecessary nesting: If a `UI.CollectionFacet` contains only one child, replace it with a direct `UI.ReferenceFacet`.

		2. Flatten the hierarchy: Move nested content up to a higher level by combining or reorganizing sections.

	2. Flatten the hierarchy: Move nested content up to a higher level by combining or reorganizing sections.
	- Flatten the hierarchy: Move nested content up to a higher level by combining or reorganizing sections.


		2. Flatten the hierarchy: Move nested content up to a higher level by combining or reorganizing sections.

		3. Use side-by-side facets: Place facets at the same level rather than nesting them deeply.

	3. Use side-by-side facets: Place facets at the same level rather than nesting them deeply.
	- Use side-by-side facets: Place facets at the same level rather than nesting them deeply.


		## Bug Report

		In case you detect an issue with this rule, please open a GitHub issue [here](https://github.com/SAP/open-ux-tools/issues).

	In case you detect an issue with this rule, please open a GitHub issue [here](https://github.com/SAP/open-ux-tools/issues).
	If you encounter any problems, open a [GitHub issue](https://github.com/SAP/open-ux-tools/issues).


		3. Use side-by-side facets: Place facets at the same level rather than nesting them deeply.

		Before (3 levels - violation):

	Before (3 levels - violation):
	Before (Three levels: violation):

	After (2 levels - correct):
	After (Two levels: correct):

Uh oh!

Conversation

vinayhospete commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How have you tested?

Checklist:

Uh oh!

changeset-bot Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

hyperspace-pr-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

hyperspace-pr-bot Bot Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

hyperspace-pr-bot Bot Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vinayhospete commented Sep 11, 2026 •

edited

Loading

changeset-bot Bot commented Sep 11, 2026 •

edited

Loading