Skip to content

Commit

Permalink
feat: Check event handlers in XML views/fragments
Browse files Browse the repository at this point in the history
- Detect usage of globals
- Detect ambiguous event handlers (not starting with a dot '.')

JIRA: CPOUI5FOUNDATION-974
JIRA: CPOUI5FOUNDATION-951
  • Loading branch information
matz3 committed Feb 14, 2025
1 parent 76394f8 commit 57d8251
Show file tree
Hide file tree
Showing 20 changed files with 602 additions and 57 deletions.
7 changes: 6 additions & 1 deletion .reuse/dep5
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,12 @@ Files: *
Copyright: 2024 SAP SE or an SAP affiliate company and UI5 linter contributors
License: Apache-2.0

Files: src/detectors/transpilers/xml/lib/JSTokenizer.js
Files: src/linter/xmlTemplate/lib/EventHandlerResolver.js
Copyright: 2009-2025 SAP SE or an SAP affiliate company
License: Apache-2.0
Comment: This file is a copy of sap/ui/core/mvc/EventHandlerResolver.js from the OpenUI5 project.

Files: src/linter/xmlTemplate/lib/JSTokenizer.js
Copyright: 2009-2024 SAP SE or an SAP affiliate company
License: Apache-2.0
Comment: This file is a copy of sap/base/util/JSTokenizer.js from the OpenUI5 project.
Expand Down
22 changes: 16 additions & 6 deletions src/linter/binding/BindingLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,21 @@ export default class BindingLinter {
}

checkForGlobalReference(ref: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
// Global reference detected
const variableName = this.getGlobalReference(ref, requireDeclarations);
if (!variableName) {
return;
}
this.#context.addLintingMessage(this.#resourcePath, MESSAGE.NO_GLOBALS, {
variableName,
namespace: ref,
}, position);
}

getGlobalReference(ref: string, requireDeclarations: RequireDeclaration[]): string | null {
if (ref.startsWith(".")) {
// Ignore empty reference or reference to the controller (as indicated by the leading dot)
return false;
return null;
}
const parts = ref.split(".");
let variableName;
Expand All @@ -137,14 +149,12 @@ export default class BindingLinter {
decl.variableName === variableName ||
decl.moduleName === parts.join("/"));
if (requireDeclaration) {
return false;
// Local reference detected
return null;
}

// Global reference detected
this.#context.addLintingMessage(this.#resourcePath, MESSAGE.NO_GLOBALS, {
variableName,
namespace: ref,
}, position);
return variableName;
}

lintPropertyBinding(bindingDefinition: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
Expand Down
13 changes: 13 additions & 0 deletions src/linter/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const RULES = {
"async-component-flags": "async-component-flags",
"csp-unsafe-inline-script": "csp-unsafe-inline-script",
"no-ambiguous-event-handler": "no-ambiguous-event-handler",
"no-deprecated-api": "no-deprecated-api",
"no-deprecated-component": "no-deprecated-component",
"no-deprecated-control-renderer-declaration": "no-deprecated-control-renderer-declaration",
Expand Down Expand Up @@ -51,6 +52,7 @@ export enum MESSAGE {
HTML_IN_XML,
LIB_INIT_API_VERSION,
MISSING_BOOTSTRAP_PARAM,
NO_AMBIGUOUS_EVENT_HANDLER,
NO_CONTROL_RERENDER_OVERRIDE,
NO_DEPRECATED_RENDERER,
NO_DIRECT_DATATYPE_ACCESS,
Expand Down Expand Up @@ -340,6 +342,17 @@ export const MESSAGE_INFO = {
details: () => `{@link sap.ui.core.Lib.init Lib.init}`,
},

[MESSAGE.NO_AMBIGUOUS_EVENT_HANDLER]: {
severity: LintMessageSeverity.Warning,
ruleId: RULES["no-ambiguous-event-handler"],

message: ({eventHandler}: {eventHandler: string}) =>
`Event handler '${eventHandler}' must be prefixed by a dot '.' or refer to a local name`,
details: () => `If the handler is defined on the controller, use the leading dot notation. ` +
`Otherwise import the module via core:require and use the handler via the local name. ` +
`See {@link topic:b0fb4de7364f4bcbb053a99aa645affe Handling Events in XML Views}`,
},

[MESSAGE.NO_CONTROL_RERENDER_OVERRIDE]: {
severity: LintMessageSeverity.Error,
ruleId: RULES["no-deprecated-api"],
Expand Down
55 changes: 46 additions & 9 deletions src/linter/xmlTemplate/Parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {ApiExtract} from "../../utils/ApiExtract.js";
import ControllerByIdInfo from "./ControllerByIdInfo.js";
import BindingLinter from "../binding/BindingLinter.js";
import {Tag as SaxTag} from "sax-wasm";
import EventHandlerResolver from "./lib/EventHandlerResolver.js";
const log = getLogger("linter:xmlTemplate:Parser");

export type Namespace = string;
Expand Down Expand Up @@ -557,15 +558,51 @@ export default class Parser {
// Check whether prop is of type "property" (indicating that it can have a binding)
// Note that some aggregations are handled like properties (0..n + alt type). Therefore check
// whether this is a property first. Additional aggregation-specific checks are not needed in that case
if (this.#apiExtract.isProperty(`${namespace}.${moduleName}`, prop.name)) {
this.#bindingLinter.lintPropertyBinding(prop.value, this.#requireDeclarations, {
line: prop.start.line + 1, // Add one to align with IDEs
column: prop.start.column + 1,
});
} else if (this.#apiExtract.isAggregation(`${namespace}.${moduleName}`, prop.name)) {
this.#bindingLinter.lintAggregationBinding(prop.value, this.#requireDeclarations, {
line: prop.start.line + 1, // Add one to align with IDEs
column: prop.start.column + 1,
const symbolName = `${namespace}.${moduleName}`;
const position = {
line: prop.start.line + 1, // Add one to align with IDEs
column: prop.start.column + 1,
};
if (this.#apiExtract.isProperty(symbolName, prop.name)) {
this.#bindingLinter.lintPropertyBinding(prop.value, this.#requireDeclarations, position);
} else if (this.#apiExtract.isAggregation(symbolName, prop.name)) {
this.#bindingLinter.lintAggregationBinding(prop.value, this.#requireDeclarations, position);
} else if (this.#apiExtract.isEvent(symbolName, prop.name)) {
EventHandlerResolver.parse(prop.value).forEach((eventHandler) => {
if (eventHandler.startsWith("cmd:")) {
// No global usage possible via command execution
return;
}
let functionName;
const openBracketIndex = eventHandler.indexOf("(");
if (openBracketIndex !== -1) {
functionName = eventHandler.slice(0, openBracketIndex);
} else {
functionName = eventHandler;
}
const variableName = this.#bindingLinter.getGlobalReference(
functionName, this.#requireDeclarations
);
if (!variableName) {
return;
}
if (!functionName.includes(".")) {
// If the event handler does not include a dot, it is most likely a reference to the
// controller which should be prefixed with a leading dot, but works in UI5 1.x runtime
// without also it.
// Note that this could also be a global function reference, but we can't distinguish
// that here.
this.#context.addLintingMessage(
this.#resourcePath, MESSAGE.NO_AMBIGUOUS_EVENT_HANDLER, {
eventHandler: functionName,
}, position
);
} else {
this.#context.addLintingMessage(this.#resourcePath, MESSAGE.NO_GLOBALS, {
variableName,
namespace: functionName,
}, position);
}
});
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/linter/xmlTemplate/lib/EventHandlerResolver.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default abstract class EventHandlerResolver {
static parse(sValue: string): string[];
}
109 changes: 109 additions & 0 deletions src/linter/xmlTemplate/lib/EventHandlerResolver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* This is a copy of the sap/ui/core/mvc/EventHandlerResolver.js module from OpenUI5.
* https://github.com/SAP/openui5/blob/83fd99ee17fddff1616577b9acaa1e0cc0ed412a/src/sap.ui.core/src/sap/ui/core/mvc/EventHandlerResolver.js
*
* It only contains the "parse" function, as all other functionality is not needed.
*/

/* eslint-disable */
/* eslint-enable no-undef */

import JSTokenizer from "./JSTokenizer.js";

// Provides module sap.ui.core.mvc.EventHandlerResolver.
// sap.ui.define([
// "sap/ui/base/BindingParser",
// "sap/ui/core/CommandExecution",
// "sap/ui/model/BindingMode",
// "sap/ui/model/CompositeBinding",
// "sap/ui/model/json/JSONModel",
// "sap/ui/model/base/ManagedObjectModel",
// "sap/base/util/JSTokenizer",
// "sap/base/util/resolveReference",
// "sap/base/future",
// "sap/ui/base/DesignTime"
// ],
// function(
// BindingParser,
// CommandExecution,
// BindingMode,
// CompositeBinding,
// JSONModel,
// MOM,
// JSTokenizer,
// resolveReference,
// future,
// DesignTime
// ) {
// "use strict";

var EventHandlerResolver = {

/**
* Parses and splits the incoming string into meaningful event handler definitions
*
* Examples:
*
* parse(".fnControllerMethod")
* => [".fnControllerMethod"]
*
* parse(".doSomething('Hello World'); .doSomething2('string'); globalFunction")
* => [".doSomething('Hello World')", ".doSomething2('string')", "globalFunction"]
* parse(".fnControllerMethod; .fnControllerMethod(${ path:'/someModelProperty', formatter: '.myFormatter', type: 'sap.ui.model.type.String'} ); globalFunction")
* => [".fnControllerMethod", ".fnControllerMethod(${ path:'/someModelProperty', formatter: '.myFormatter', type: 'sap.ui.model.type.String'} )", "globalFunction"]
*
* @param {string} [sValue] - Incoming string
* @return {string[]} - Array of event handler definitions
*/
parse: function parse(sValue) {
sValue = sValue.trim();
var oTokenizer = new JSTokenizer();
var aResult = [];
var sBuffer = "";
var iParenthesesCounter = 0;

oTokenizer.init(sValue, 0);
for (;;) {
var sSymbol = oTokenizer.next();
if ( sSymbol === '"' || sSymbol === "'" ) {
var pos = oTokenizer.getIndex();
oTokenizer.string();
sBuffer += sValue.slice(pos, oTokenizer.getIndex());
sSymbol = oTokenizer.getCh();
}
if ( !sSymbol ) {
break;
}
switch (sSymbol) {
case "(":
iParenthesesCounter++;
break;
case ")":
iParenthesesCounter--;
break;
default:
break;
}

if (sSymbol === ";" && iParenthesesCounter === 0) {
aResult.push(sBuffer.trim());
sBuffer = "";
} else {
sBuffer += sSymbol;
}
}

if (sBuffer) {
aResult.push(sBuffer.trim());
}

return aResult;
}
};

// return EventHandlerResolver;
// }
// );

export default EventHandlerResolver;
21 changes: 21 additions & 0 deletions test/fixtures/linter/rules/NoGlobals/GlobalEventHandlers.view.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<mvc:View xmlns="sap.m" xmlns:mvc="sap.ui.core.mvc">

<!-- Global event handler -->
<Button press="ui5.walkthrougth.utils.MyHelper.onPressFancyButton" />

<!-- Global event handler (with arguments) -->
<Button press="ui5.walkthrougth.utils.MyHelper.onPressFancyButton(0, ${i18n>TEXT}, $controller.foo.bar, $event)" />

<!-- Multiple global event handler -->
<Button press="global.doSomething; ui5.walkthrougth.utils.MyHelper.onPressFancyButton(0, ${i18n>TEXT}, $controller.foo.bar, $event); global.doSomethingElse" />

<!-- Local event handler without leading dot, which could also be a global reference -->
<Button press="onPressFancyButton" />

<!-- Local event handler without leading dot, which could also be a global reference (with arguments) -->
<Button press="onPressFancyButton(0, ${i18n>TEXT}, $controller.foo.bar, $event)" />

<!-- Mixed: local, local without leading dot, command execution and global handler -->
<Button press="onPressFancyButton(0, ${i18n>TEXT}, $controller.foo.bar, $event); .onPressFancyButton; cmd:Save; global.doSomething($event, $controller)" />

</mvc:View>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<mvc:View xmlns="sap.m" xmlns:mvc="sap.ui.core.mvc" xmlns:core="sap.ui.core">

<!-- Local event handler -->
<Button press=".onPressFancyButton" />

<!-- Local event handler (nested) -->
<Button press=".eventHandlers.onPressFancyButton" />

<!-- Local event handler (with arguments) -->
<Button press=".onPressFancyButton(0, ${i18n>LABEL_ALL}, $controller.foo.bar, $event)" />

<!-- Command Execution -->
<Button press="cmd:Save" />

<!-- Empty event handler -->
<Button press="" />

<!-- Usage of event handler imported via core:require helper -->
<Button core:require='{ "MyHelper": "ui5/walkthrougth/utils/MyHelper" }' press="MyHelper.onPressFancyButton" />

<!-- Usage of event handler function imported via core:require -->
<Button core:require='{ "doSomething": "ui5/walkthrougth/utils/doSomething" }' press="doSomething" />

</mvc:View>
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

<template:if test="{entityType>com.sap.vocabularies.UI.v1.LineItem}">
<Table headerText="Items" includeItemInSelection="true" mode="SingleSelectMaster"
selectionChange="onSelectionChange" items="{:= '{path:\'/' + ${meta>name} + '\', length: 5}' }">
selectionChange=".onSelectionChange" items="{:= '{path:\'/' + ${meta>name} + '\', length: 5}' }">
<template:with path="entityType>com.sap.vocabularies.UI.v1.LineItem" var="target">
<core:Fragment fragmentName="sap.ui.core.sample.ViewTemplate.scenario.Table" type="XML"/>
</template:with>
Expand Down
8 changes: 4 additions & 4 deletions test/fixtures/transpiler/xml/DuplicateAggregations.view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,21 @@
<m:Button
icon="sap-icon://activities"
tooltip="show indices of selected items"
press="getSelectedIndices"/>
press=".getSelectedIndices"/>
<m:Button
icon="sap-icon://activity-items"
tooltip="show context of latest selection item"
press="getContextByIndex"/>
press=".getContextByIndex"/>
<m:Button
icon="sap-icon://decline"
tooltip="clear selection"
press="clearSelection"/>
press=".clearSelection"/>
<m:Switch
state="true"
customTextOn="on"
customTextOff="off"
tooltip="enable select all items"
change="onSwitchChange"/>
change=".onSwitchChange"/>
</m:OverflowToolbar>
</extension>
<columns>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<!-- text: Deprecated reference of global formatter and binding event handler function -->
<!-- tooltip: Aggregation with a 0..1 aggregation and "string" alt type can be used like a property -->
<!-- formatError: Control event handlers should be ignored for now -->
<!-- formatError: Deprecated reference of global event handler -->
<ObjectStatus
text="{
path: 'invoice>Status',
Expand Down Expand Up @@ -35,6 +35,8 @@
dataRequested: 'EventHandler.onMyDataRequested'
}
}"

formatError=".handleEvent"
/>

</VBox>
4 changes: 3 additions & 1 deletion test/fixtures/transpiler/xml/XMLViewBindings.view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<!-- text: Deprecated reference of global formatter and binding event handler function -->
<!-- tooltip: Aggregation with a 0..1 aggregation and "string" alt type can be used like a property -->
<!-- customData: Aggregation with factory and nested filter -->
<!-- formatError: Control event handlers should be ignored for now -->
<!-- formatError: Deprecated reference of global event handler -->
<ObjectStatus text="{
path: 'invoice>Status',
formatter: 'ui5.walkthrough.model.formatter.statusText',
Expand Down Expand Up @@ -83,6 +83,8 @@
}]
}]
}"

formatError=".handleEvent"
/>

</mvc:View>
Loading

0 comments on commit 57d8251

Please sign in to comment.