refactor: Consider ManagedObject as base class for bind* & applySettings #513
Conversation
| @@ -842,7 +842,7 @@ export default class SourceFileLinter { | |||
| this.#reportTestStarter(node); | |||
| } else if (symbolName === "applySettings" && | |||
| nodeType.symbol?.declarations?.some((declaration) => | |||
| this.isUi5ClassDeclaration(declaration, "sap/ui/core/Control"))) { | |||
| this.isUi5ClassDeclaration(declaration, "sap/ui/base/ManagedObject"))) { | |||
There was a problem hiding this comment.
Please check for all places. There is still a check against Control a few lines below.
There was a problem hiding this comment.
bind in here: https://github.com/SAP/ui5-linter/pull/513/files#diff-7bc0071c1214325807d3a82a222b097360e3b0020aae68d0575dbae4aa43b99aR851 reflects the bind${propertyName} which, according to the findings in the docs, is being generated at the level where control properties are defined.
It would be the most appropriate to check against moduleName, but still the inheritance factor plays a role, so the exact and correct finding which module is, would add complexity and performance penalty.
I agree that bindProperty should be checked aggainst ManagedObject, because it's defined in there, but the case here is more complex and sap/ui/core/Control seems to me a reasonable compromise
There was a problem hiding this comment.
The bind${propertyName} method is generated for a ManagedObject sub-class that defines properties in their metadata. Most case are in fact based on sub-classes of Control, but some are also based on Element and some are just sub-classes of ManagedObject (see "View Subclasses" in https://ui5.sap.com/#/api/sap.ui.base.ManagedObject).
As we have now several places for that check, maybe doing it only once would even improve performance.
There was a problem hiding this comment.
ManagedObjects with bind_XYZ_ methods:
| Class | Aggregation | Method |
|---|---|---|
| sap.gantt.def.SvgDefs | #defs | bindDefs |
| sap.gantt.def.cal.CalendarDefs | #defs1 | bindDefs1 |
| sap.gantt.def.cal.CalendarDefs | #defs2 | bindDefs2 |
| sap.gantt.def.cal.CalendarDefs | #defs3 | bindDefs3 |
| sap.gantt.def.cal.CalendarDefs | #defs4 | bindDefs4 |
| sap.gantt.def.cal.CalendarDefs | #defs5 | bindDefs5 |
Note: the above classes should be elements, they are objects by mistake (see internal change 6410569, file SvgDefs).
Elements with bind_XYZ_ methods:
| Class | Aggregation | Method |
|---|---|---|
| sap.gantt.def.cal.Calendar | #timeIntervals | bindTimeIntervals |
| sap.m.MenuItem | #items | bindItems |
| sap.m.QuickViewGroup | #elements | bindElements |
| sap.m.SelectionDetailsItem | #lines | bindLines |
| sap.m.UploadCollectionItem | #attributes | bindAttributes |
| sap.m.UploadCollectionItem | #markers | bindMarkers |
| sap.m.UploadCollectionItem | #statuses | bindStatuses |
| sap.m.ViewSettingsFilterItem | #items | bindItems |
| sap.m.semantic.SemanticSelect | #items | bindItems |
| sap.makit.Layer | #columns | bindColumns |
| sap.makit.Layer | #rows | bindRows |
| sap.suite.ui.microchart.AreaMicroChartItem | #points | bindPoints |
| sap.suite.ui.microchart.LineMicroChartLine | #points | bindPoints |
| sap.ui.vk.ContentConnector | #contentResources | bindContentResources |
| sap.viz.ui5.data.FlattenedDataset | #data | bindData |
d3xter666
changed the title
d3xter666
changed the title
fixes: #480 (comment)