Skip to content

Commit 57d8251

Browse files
matz3matz3
committedFeb 14, 2025
feat: Check event handlers in XML views/fragments
- Detect usage of globals - Detect ambiguous event handlers (not starting with a dot '.') JIRA: CPOUI5FOUNDATION-974 JIRA: CPOUI5FOUNDATION-951
1 parent 76394f8 commit 57d8251

20 files changed

+602
-57
lines changed
 

‎.reuse/dep5

+6-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@ Files: *
2828
Copyright: 2024 SAP SE or an SAP affiliate company and UI5 linter contributors
2929
License: Apache-2.0
3030

31-
Files: src/detectors/transpilers/xml/lib/JSTokenizer.js
31+
Files: src/linter/xmlTemplate/lib/EventHandlerResolver.js
32+
Copyright: 2009-2025 SAP SE or an SAP affiliate company
33+
License: Apache-2.0
34+
Comment: This file is a copy of sap/ui/core/mvc/EventHandlerResolver.js from the OpenUI5 project.
35+
36+
Files: src/linter/xmlTemplate/lib/JSTokenizer.js
3237
Copyright: 2009-2024 SAP SE or an SAP affiliate company
3338
License: Apache-2.0
3439
Comment: This file is a copy of sap/base/util/JSTokenizer.js from the OpenUI5 project.

‎src/linter/binding/BindingLinter.ts

+16-6
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,21 @@ export default class BindingLinter {
122122
}
123123

124124
checkForGlobalReference(ref: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
125+
// Global reference detected
126+
const variableName = this.getGlobalReference(ref, requireDeclarations);
127+
if (!variableName) {
128+
return;
129+
}
130+
this.#context.addLintingMessage(this.#resourcePath, MESSAGE.NO_GLOBALS, {
131+
variableName,
132+
namespace: ref,
133+
}, position);
134+
}
135+
136+
getGlobalReference(ref: string, requireDeclarations: RequireDeclaration[]): string | null {
125137
if (ref.startsWith(".")) {
126138
// Ignore empty reference or reference to the controller (as indicated by the leading dot)
127-
return false;
139+
return null;
128140
}
129141
const parts = ref.split(".");
130142
let variableName;
@@ -137,14 +149,12 @@ export default class BindingLinter {
137149
decl.variableName === variableName ||
138150
decl.moduleName === parts.join("/"));
139151
if (requireDeclaration) {
140-
return false;
152+
// Local reference detected
153+
return null;
141154
}
142155

143156
// Global reference detected
144-
this.#context.addLintingMessage(this.#resourcePath, MESSAGE.NO_GLOBALS, {
145-
variableName,
146-
namespace: ref,
147-
}, position);
157+
return variableName;
148158
}
149159

150160
lintPropertyBinding(bindingDefinition: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {

‎src/linter/messages.ts

+13
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
const RULES = {
44
"async-component-flags": "async-component-flags",
55
"csp-unsafe-inline-script": "csp-unsafe-inline-script",
6+
"no-ambiguous-event-handler": "no-ambiguous-event-handler",
67
"no-deprecated-api": "no-deprecated-api",
78
"no-deprecated-component": "no-deprecated-component",
89
"no-deprecated-control-renderer-declaration": "no-deprecated-control-renderer-declaration",
@@ -51,6 +52,7 @@ export enum MESSAGE {
5152
HTML_IN_XML,
5253
LIB_INIT_API_VERSION,
5354
MISSING_BOOTSTRAP_PARAM,
55+
NO_AMBIGUOUS_EVENT_HANDLER,
5456
NO_CONTROL_RERENDER_OVERRIDE,
5557
NO_DEPRECATED_RENDERER,
5658
NO_DIRECT_DATATYPE_ACCESS,
@@ -340,6 +342,17 @@ export const MESSAGE_INFO = {
340342
details: () => `{@link sap.ui.core.Lib.init Lib.init}`,
341343
},
342344

345+
[MESSAGE.NO_AMBIGUOUS_EVENT_HANDLER]: {
346+
severity: LintMessageSeverity.Warning,
347+
ruleId: RULES["no-ambiguous-event-handler"],
348+
349+
message: ({eventHandler}: {eventHandler: string}) =>
350+
`Event handler '${eventHandler}' must be prefixed by a dot '.' or refer to a local name`,
351+
details: () => `If the handler is defined on the controller, use the leading dot notation. ` +
352+
`Otherwise import the module via core:require and use the handler via the local name. ` +
353+
`See {@link topic:b0fb4de7364f4bcbb053a99aa645affe Handling Events in XML Views}`,
354+
},
355+
343356
[MESSAGE.NO_CONTROL_RERENDER_OVERRIDE]: {
344357
severity: LintMessageSeverity.Error,
345358
ruleId: RULES["no-deprecated-api"],

‎src/linter/xmlTemplate/Parser.ts

+46-9
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {ApiExtract} from "../../utils/ApiExtract.js";
1212
import ControllerByIdInfo from "./ControllerByIdInfo.js";
1313
import BindingLinter from "../binding/BindingLinter.js";
1414
import {Tag as SaxTag} from "sax-wasm";
15+
import EventHandlerResolver from "./lib/EventHandlerResolver.js";
1516
const log = getLogger("linter:xmlTemplate:Parser");
1617

1718
export type Namespace = string;
@@ -557,15 +558,51 @@ export default class Parser {
557558
// Check whether prop is of type "property" (indicating that it can have a binding)
558559
// Note that some aggregations are handled like properties (0..n + alt type). Therefore check
559560
// whether this is a property first. Additional aggregation-specific checks are not needed in that case
560-
if (this.#apiExtract.isProperty(`${namespace}.${moduleName}`, prop.name)) {
561-
this.#bindingLinter.lintPropertyBinding(prop.value, this.#requireDeclarations, {
562-
line: prop.start.line + 1, // Add one to align with IDEs
563-
column: prop.start.column + 1,
564-
});
565-
} else if (this.#apiExtract.isAggregation(`${namespace}.${moduleName}`, prop.name)) {
566-
this.#bindingLinter.lintAggregationBinding(prop.value, this.#requireDeclarations, {
567-
line: prop.start.line + 1, // Add one to align with IDEs
568-
column: prop.start.column + 1,
561+
const symbolName = `${namespace}.${moduleName}`;
562+
const position = {
563+
line: prop.start.line + 1, // Add one to align with IDEs
564+
column: prop.start.column + 1,
565+
};
566+
if (this.#apiExtract.isProperty(symbolName, prop.name)) {
567+
this.#bindingLinter.lintPropertyBinding(prop.value, this.#requireDeclarations, position);
568+
} else if (this.#apiExtract.isAggregation(symbolName, prop.name)) {
569+
this.#bindingLinter.lintAggregationBinding(prop.value, this.#requireDeclarations, position);
570+
} else if (this.#apiExtract.isEvent(symbolName, prop.name)) {
571+
EventHandlerResolver.parse(prop.value).forEach((eventHandler) => {
572+
if (eventHandler.startsWith("cmd:")) {
573+
// No global usage possible via command execution
574+
return;
575+
}
576+
let functionName;
577+
const openBracketIndex = eventHandler.indexOf("(");
578+
if (openBracketIndex !== -1) {
579+
functionName = eventHandler.slice(0, openBracketIndex);
580+
} else {
581+
functionName = eventHandler;
582+
}
583+
const variableName = this.#bindingLinter.getGlobalReference(
584+
functionName, this.#requireDeclarations
585+
);
586+
if (!variableName) {
587+
return;
588+
}
589+
if (!functionName.includes(".")) {
590+
// If the event handler does not include a dot, it is most likely a reference to the
591+
// controller which should be prefixed with a leading dot, but works in UI5 1.x runtime
592+
// without also it.
593+
// Note that this could also be a global function reference, but we can't distinguish
594+
// that here.
595+
this.#context.addLintingMessage(
596+
this.#resourcePath, MESSAGE.NO_AMBIGUOUS_EVENT_HANDLER, {
597+
eventHandler: functionName,
598+
}, position
599+
);
600+
} else {
601+
this.#context.addLintingMessage(this.#resourcePath, MESSAGE.NO_GLOBALS, {
602+
variableName,
603+
namespace: functionName,
604+
}, position);
605+
}
569606
});
570607
}
571608
}

‎src/linter/xmlTemplate/lib/EventHandlerResolver.d.ts

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default abstract class EventHandlerResolver {
2+
static parse(sValue: string): string[];
3+
}

‎src/linter/xmlTemplate/lib/EventHandlerResolver.js

+109
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* This is a copy of the sap/ui/core/mvc/EventHandlerResolver.js module from OpenUI5.
3+
* https://github.com/SAP/openui5/blob/83fd99ee17fddff1616577b9acaa1e0cc0ed412a/src/sap.ui.core/src/sap/ui/core/mvc/EventHandlerResolver.js
4+
*
5+
* It only contains the "parse" function, as all other functionality is not needed.
6+
*/
7+
8+
/* eslint-disable */
9+
/* eslint-enable no-undef */
10+
11+
import JSTokenizer from "./JSTokenizer.js";
12+
13+
// Provides module sap.ui.core.mvc.EventHandlerResolver.
14+
// sap.ui.define([
15+
// "sap/ui/base/BindingParser",
16+
// "sap/ui/core/CommandExecution",
17+
// "sap/ui/model/BindingMode",
18+
// "sap/ui/model/CompositeBinding",
19+
// "sap/ui/model/json/JSONModel",
20+
// "sap/ui/model/base/ManagedObjectModel",
21+
// "sap/base/util/JSTokenizer",
22+
// "sap/base/util/resolveReference",
23+
// "sap/base/future",
24+
// "sap/ui/base/DesignTime"
25+
// ],
26+
// function(
27+
// BindingParser,
28+
// CommandExecution,
29+
// BindingMode,
30+
// CompositeBinding,
31+
// JSONModel,
32+
// MOM,
33+
// JSTokenizer,
34+
// resolveReference,
35+
// future,
36+
// DesignTime
37+
// ) {
38+
// "use strict";
39+
40+
var EventHandlerResolver = {
41+
42+
/**
43+
* Parses and splits the incoming string into meaningful event handler definitions
44+
*
45+
* Examples:
46+
*
47+
* parse(".fnControllerMethod")
48+
* => [".fnControllerMethod"]
49+
*
50+
* parse(".doSomething('Hello World'); .doSomething2('string'); globalFunction")
51+
* => [".doSomething('Hello World')", ".doSomething2('string')", "globalFunction"]
52+
53+
* parse(".fnControllerMethod; .fnControllerMethod(${ path:'/someModelProperty', formatter: '.myFormatter', type: 'sap.ui.model.type.String'} ); globalFunction")
54+
* => [".fnControllerMethod", ".fnControllerMethod(${ path:'/someModelProperty', formatter: '.myFormatter', type: 'sap.ui.model.type.String'} )", "globalFunction"]
55+
*
56+
* @param {string} [sValue] - Incoming string
57+
* @return {string[]} - Array of event handler definitions
58+
*/
59+
parse: function parse(sValue) {
60+
sValue = sValue.trim();
61+
var oTokenizer = new JSTokenizer();
62+
var aResult = [];
63+
var sBuffer = "";
64+
var iParenthesesCounter = 0;
65+
66+
oTokenizer.init(sValue, 0);
67+
for (;;) {
68+
var sSymbol = oTokenizer.next();
69+
if ( sSymbol === '"' || sSymbol === "'" ) {
70+
var pos = oTokenizer.getIndex();
71+
oTokenizer.string();
72+
sBuffer += sValue.slice(pos, oTokenizer.getIndex());
73+
sSymbol = oTokenizer.getCh();
74+
}
75+
if ( !sSymbol ) {
76+
break;
77+
}
78+
switch (sSymbol) {
79+
case "(":
80+
iParenthesesCounter++;
81+
break;
82+
case ")":
83+
iParenthesesCounter--;
84+
break;
85+
default:
86+
break;
87+
}
88+
89+
if (sSymbol === ";" && iParenthesesCounter === 0) {
90+
aResult.push(sBuffer.trim());
91+
sBuffer = "";
92+
} else {
93+
sBuffer += sSymbol;
94+
}
95+
}
96+
97+
if (sBuffer) {
98+
aResult.push(sBuffer.trim());
99+
}
100+
101+
return aResult;
102+
}
103+
};
104+
105+
// return EventHandlerResolver;
106+
// }
107+
// );
108+
109+
export default EventHandlerResolver;

‎test/fixtures/linter/rules/NoGlobals/GlobalEventHandlers.view.xml

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<mvc:View xmlns="sap.m" xmlns:mvc="sap.ui.core.mvc">
2+
3+
<!-- Global event handler -->
4+
<Button press="ui5.walkthrougth.utils.MyHelper.onPressFancyButton" />
5+
6+
<!-- Global event handler (with arguments) -->
7+
<Button press="ui5.walkthrougth.utils.MyHelper.onPressFancyButton(0, ${i18n>TEXT}, $controller.foo.bar, $event)" />
8+
9+
<!-- Multiple global event handler -->
10+
<Button press="global.doSomething; ui5.walkthrougth.utils.MyHelper.onPressFancyButton(0, ${i18n>TEXT}, $controller.foo.bar, $event); global.doSomethingElse" />
11+
12+
<!-- Local event handler without leading dot, which could also be a global reference -->
13+
<Button press="onPressFancyButton" />
14+
15+
<!-- Local event handler without leading dot, which could also be a global reference (with arguments) -->
16+
<Button press="onPressFancyButton(0, ${i18n>TEXT}, $controller.foo.bar, $event)" />
17+
18+
<!-- Mixed: local, local without leading dot, command execution and global handler -->
19+
<Button press="onPressFancyButton(0, ${i18n>TEXT}, $controller.foo.bar, $event); .onPressFancyButton; cmd:Save; global.doSomething($event, $controller)" />
20+
21+
</mvc:View>

‎test/fixtures/linter/rules/NoGlobals/GlobalEventHandlers_Negative.view.xml

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<mvc:View xmlns="sap.m" xmlns:mvc="sap.ui.core.mvc" xmlns:core="sap.ui.core">
2+
3+
<!-- Local event handler -->
4+
<Button press=".onPressFancyButton" />
5+
6+
<!-- Local event handler (nested) -->
7+
<Button press=".eventHandlers.onPressFancyButton" />
8+
9+
<!-- Local event handler (with arguments) -->
10+
<Button press=".onPressFancyButton(0, ${i18n>LABEL_ALL}, $controller.foo.bar, $event)" />
11+
12+
<!-- Command Execution -->
13+
<Button press="cmd:Save" />
14+
15+
<!-- Empty event handler -->
16+
<Button press="" />
17+
18+
<!-- Usage of event handler imported via core:require helper -->
19+
<Button core:require='{ "MyHelper": "ui5/walkthrougth/utils/MyHelper" }' press="MyHelper.onPressFancyButton" />
20+
21+
<!-- Usage of event handler function imported via core:require -->
22+
<Button core:require='{ "doSomething": "ui5/walkthrougth/utils/doSomething" }' press="doSomething" />
23+
24+
</mvc:View>

‎test/fixtures/linter/rules/NoGlobals/XMLTemplatingV2.view.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
<template:if test="{entityType>com.sap.vocabularies.UI.v1.LineItem}">
2727
<Table headerText="Items" includeItemInSelection="true" mode="SingleSelectMaster"
28-
selectionChange="onSelectionChange" items="{:= '{path:\'/' + ${meta>name} + '\', length: 5}' }">
28+
selectionChange=".onSelectionChange" items="{:= '{path:\'/' + ${meta>name} + '\', length: 5}' }">
2929
<template:with path="entityType>com.sap.vocabularies.UI.v1.LineItem" var="target">
3030
<core:Fragment fragmentName="sap.ui.core.sample.ViewTemplate.scenario.Table" type="XML"/>
3131
</template:with>

‎test/fixtures/transpiler/xml/DuplicateAggregations.view.xml

+4-4
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,21 @@
1919
<m:Button
2020
icon="sap-icon://activities"
2121
tooltip="show indices of selected items"
22-
press="getSelectedIndices"/>
22+
press=".getSelectedIndices"/>
2323
<m:Button
2424
icon="sap-icon://activity-items"
2525
tooltip="show context of latest selection item"
26-
press="getContextByIndex"/>
26+
press=".getContextByIndex"/>
2727
<m:Button
2828
icon="sap-icon://decline"
2929
tooltip="clear selection"
30-
press="clearSelection"/>
30+
press=".clearSelection"/>
3131
<m:Switch
3232
state="true"
3333
customTextOn="on"
3434
customTextOff="off"
3535
tooltip="enable select all items"
36-
change="onSwitchChange"/>
36+
change=".onSwitchChange"/>
3737
</m:OverflowToolbar>
3838
</extension>
3939
<columns>

‎test/fixtures/transpiler/xml/XMLFragmentBindings.fragment.xml

+3-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
<!-- text: Deprecated reference of global formatter and binding event handler function -->
66
<!-- tooltip: Aggregation with a 0..1 aggregation and "string" alt type can be used like a property -->
7-
<!-- formatError: Control event handlers should be ignored for now -->
7+
<!-- formatError: Deprecated reference of global event handler -->
88
<ObjectStatus
99
text="{
1010
path: 'invoice>Status',
@@ -35,6 +35,8 @@
3535
dataRequested: 'EventHandler.onMyDataRequested'
3636
}
3737
}"
38+
39+
formatError=".handleEvent"
3840
/>
3941

4042
</VBox>

‎test/fixtures/transpiler/xml/XMLViewBindings.view.xml

+3-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<!-- text: Deprecated reference of global formatter and binding event handler function -->
1010
<!-- tooltip: Aggregation with a 0..1 aggregation and "string" alt type can be used like a property -->
1111
<!-- customData: Aggregation with factory and nested filter -->
12-
<!-- formatError: Control event handlers should be ignored for now -->
12+
<!-- formatError: Deprecated reference of global event handler -->
1313
<ObjectStatus text="{
1414
path: 'invoice>Status',
1515
formatter: 'ui5.walkthrough.model.formatter.statusText',
@@ -83,6 +83,8 @@
8383
}]
8484
}]
8585
}"
86+
87+
formatError=".handleEvent"
8688
/>
8789

8890
</mvc:View>

0 commit comments

Comments
 (0)