Skip to content

Commit f416a0c

Browse files
matz3matz3
committed
feat: Detect override of control "rerender"
JIRA: CPOUI5FOUNDATION-939
1 parent d0760c9 commit f416a0c

File tree

9 files changed

+564
-1
lines changed

9 files changed

+564
-1
lines changed

‎src/linter/messages.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export enum MESSAGE {
4747
HTML_IN_XML,
4848
LIB_INIT_API_VERSION,
4949
MISSING_BOOTSTRAP_PARAM,
50+
NO_CONTROL_RERENDER_OVERRIDE,
5051
NO_DEPRECATED_RENDERER,
5152
NO_DIRECT_DATATYPE_ACCESS,
5253
NO_DIRECT_ENUM_ACCESS,
@@ -316,6 +317,16 @@ export const MESSAGE_INFO = {
316317
details: () => `{@link sap.ui.core.Lib.init Lib.init}`,
317318
},
318319

320+
[MESSAGE.NO_CONTROL_RERENDER_OVERRIDE]: {
321+
severity: LintMessageSeverity.Error,
322+
ruleId: RULES["no-deprecated-api"],
323+
message: ({className}: {className: string}) =>
324+
`Override of deprecated method 'rerender' in control '${className}'`,
325+
details: () => `Starting from UI5 1.121 the framework no longer calls 'rerender', ` +
326+
`so there is no point in overriding it. ` +
327+
`Move any rendering related code to the renderer or into onBeforeRendering/onAfterRendering.`,
328+
},
329+
319330
[MESSAGE.NO_DEPRECATED_RENDERER]: {
320331
severity: LintMessageSeverity.Error,
321332
ruleId: RULES["no-deprecated-api"],

‎src/linter/ui5Types/SourceFileLinter.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import LinterContext, {ResourcePath, CoverageCategory} from "../LinterContext.js
66
import {MESSAGE} from "../messages.js";
77
import analyzeComponentJson from "./asyncComponentFlags.js";
88
import {deprecatedLibraries, deprecatedThemes} from "../../utils/deprecations.js";
9-
import {getPropertyName, getSymbolForPropertyInConstructSignatures} from "./utils.js";
9+
import {findClassInstanceMethod, getPropertyName, getSymbolForPropertyInConstructSignatures} from "./utils.js";
1010
import {taskStart} from "../../utils/perf.js";
1111
import {getPositionsForNode} from "../../utils/nodePosition.js";
1212
import {TraceMap} from "@jridgewell/trace-mapping";
@@ -153,6 +153,7 @@ export default class SourceFileLinter {
153153
ts.forEachChild(node, visitMetadataNodes);
154154
} else if (this.isUi5ClassDeclaration(node, "sap/ui/core/Control")) {
155155
this.analyzeControlRendererDeclaration(node);
156+
this.analyzeControlRerenderMethod(node);
156157
} else if (ts.isPropertyAssignment(node) && removeQuotes(node.name.getText()) === "theme") {
157158
this.analyzeTestsuiteThemeProperty(node);
158159
}
@@ -539,6 +540,15 @@ export default class SourceFileLinter {
539540
}
540541
}
541542

543+
analyzeControlRerenderMethod(node: ts.ClassDeclaration) {
544+
const className = node.name?.getText() ?? "<unknown>";
545+
const rerenderMethod = findClassInstanceMethod(node, "rerender", this.checker);
546+
if (!rerenderMethod) {
547+
return;
548+
}
549+
this.#reporter.addMessage(MESSAGE.NO_CONTROL_RERENDER_OVERRIDE, {className}, rerenderMethod);
550+
}
551+
542552
analyzeMetadataProperty(type: string, node: ts.PropertyAssignment) {
543553
const analyzeMetadataDone = taskStart(`analyzeMetadataProperty: ${type}`, this.resourcePath, true);
544554
if (type === "interfaces") {

‎src/linter/ui5Types/utils.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,35 @@ export function getSymbolForPropertyInConstructSignatures(
2323
}
2424
return undefined;
2525
}
26+
27+
export function findClassInstanceMethod(
28+
node: ts.ClassDeclaration, methodName: string, checker: ts.TypeChecker
29+
): ts.ClassElement | undefined {
30+
return node.members.find((member) => {
31+
if (!member.name) {
32+
return false;
33+
}
34+
const name = getPropertyName(member.name);
35+
if (name !== methodName) {
36+
return false;
37+
}
38+
if (ts.isMethodDeclaration(member)) {
39+
return true;
40+
}
41+
if (ts.isPropertyDeclaration(member)) {
42+
if (!member.initializer) {
43+
return false;
44+
}
45+
if (ts.isFunctionExpression(member.initializer) || ts.isArrowFunction(member.initializer)) {
46+
return true;
47+
};
48+
if (ts.isIdentifier(member.initializer)) {
49+
const symbol = checker.getSymbolAtLocation(member.initializer);
50+
if (symbol?.valueDeclaration && ts.isFunctionDeclaration(symbol.valueDeclaration)) {
51+
return true;
52+
}
53+
}
54+
}
55+
return false;
56+
});
57+
}

‎test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/Control.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ sap.ui.define(["./Element"], function(Element) {
77
library: "sap.ui.core",
88
},
99

10+
// Should not be detected as declaration / override of a deprecated method
11+
rerender: function() {},
12+
1013
renderer : null // Control has no renderer
1114
});
1215
return Control;

‎test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/Element.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ sap.ui.define(["../base/ManagedObject"], function(ManagedObject) {
77
library : "sap.ui.core"
88
},
99

10+
// Should not be detected as declaration / override of a deprecated method
11+
rerender: function() {},
12+
1013
renderer : null // Element has no renderer
1114
});
1215
return Element;

‎test/fixtures/linter/rules/renderer/ControlRerenderOverride.js

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
sap.ui.define(["sap/ui/core/Control"], function(Control) {
2+
const Example1 = Control.extend("sap.ui.demo.linter.controls.Example1", {
3+
metadata: {},
4+
5+
rerender: function() {
6+
console.log("Overriding rerender method");
7+
return Control.prototype.rerender.apply(this, arguments);
8+
},
9+
10+
renderer: {
11+
apiVersion: 2,
12+
render: function(oRm, oControl) {
13+
oRm.openStart("div", oControl);
14+
oRm.openEnd();
15+
oRm.close("div");
16+
}
17+
}
18+
});
19+
20+
const Example2 = Control.extend("sap.ui.demo.linter.controls.Example2", {
21+
metadata: {},
22+
23+
"rerender": function() {
24+
console.log("Overriding rerender method without calling super method");
25+
},
26+
27+
renderer: {
28+
apiVersion: 2,
29+
render: function(oRm, oControl) {
30+
oRm.openStart("div", oControl);
31+
oRm.openEnd();
32+
oRm.close("div");
33+
}
34+
}
35+
});
36+
37+
const Example3 = Control.extend("sap.ui.demo.linter.controls.Example3", {
38+
metadata: {},
39+
40+
rerender: () => {
41+
console.log("Overriding rerender method without calling super method");
42+
},
43+
44+
renderer: {
45+
apiVersion: 2,
46+
render: function(oRm, oControl) {
47+
oRm.openStart("div", oControl);
48+
oRm.openEnd();
49+
oRm.close("div");
50+
}
51+
}
52+
});
53+
54+
const Example4 = Control.extend("sap.ui.demo.linter.controls.Example4", {
55+
metadata: {},
56+
57+
rerender() {
58+
console.log("Overriding rerender method without calling super method");
59+
},
60+
61+
renderer: {
62+
apiVersion: 2,
63+
render: function(oRm, oControl) {
64+
oRm.openStart("div", oControl);
65+
oRm.openEnd();
66+
oRm.close("div");
67+
}
68+
}
69+
});
70+
71+
const Example5 = Control.extend("sap.ui.demo.linter.controls.Example5", {
72+
metadata: {},
73+
74+
renderer: {
75+
apiVersion: 2,
76+
render: function(oRm, oControl) {
77+
oRm.openStart("div", oControl);
78+
oRm.openEnd();
79+
oRm.close("div");
80+
}
81+
}
82+
});
83+
// TODO detect: Check why this override is currently not detected in JavaScript files.
84+
// The same code is detected properly in ControlRerenderOverrideTypeScript.ts.
85+
Example5.prototype.rerender = function() {
86+
console.log("Overriding rerender method without calling super method");
87+
};
88+
89+
function rerender() {
90+
console.log("Overriding rerender method without calling super method");
91+
}
92+
93+
const Example6 = Control.extend("sap.ui.demo.linter.controls.Example6", {
94+
metadata: {},
95+
96+
rerender,
97+
98+
renderer: {
99+
apiVersion: 2,
100+
render: function(oRm, oControl) {
101+
oRm.openStart("div", oControl);
102+
oRm.openEnd();
103+
oRm.close("div");
104+
}
105+
}
106+
});
107+
108+
const Example7 = Control.extend("sap.ui.demo.linter.controls.Example7", {
109+
metadata: {},
110+
111+
rerender: rerender,
112+
113+
renderer: {
114+
apiVersion: 2,
115+
render: function(oRm, oControl) {
116+
oRm.openStart("div", oControl);
117+
oRm.openEnd();
118+
oRm.close("div");
119+
}
120+
}
121+
});
122+
});

‎test/fixtures/linter/rules/renderer/ControlRerenderOverrideTypeScript.ts

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
import Control from "sap/ui/core/Control";
2+
import type { MetadataOptions } from "sap/ui/core/Element";
3+
import RenderManager from "sap/ui/core/RenderManager";
4+
5+
/**
6+
* @namespace sap.ui.demo.linter.controls
7+
*/
8+
class Example1 extends Control {
9+
static readonly metadata: MetadataOptions = {}
10+
11+
rerender() {
12+
console.log("Overriding rerender method");
13+
return super.rerender();
14+
}
15+
16+
static renderer = {
17+
apiVersion: 2,
18+
render: function(rm: RenderManager, control: Example1) {
19+
rm.openStart("div", control);
20+
rm.openEnd();
21+
rm.close("div");
22+
}
23+
}
24+
}
25+
26+
/**
27+
* @namespace sap.ui.demo.linter.controls
28+
*/
29+
class Example2 extends Control {
30+
static readonly metadata: MetadataOptions = {}
31+
32+
rerender() {
33+
console.log("Overriding rerender method without calling super method");
34+
}
35+
36+
static renderer = {
37+
apiVersion: 2,
38+
render: function(rm: RenderManager, control: Example2) {
39+
rm.openStart("div", control);
40+
rm.openEnd();
41+
rm.close("div");
42+
}
43+
}
44+
}
45+
46+
/**
47+
* @namespace sap.ui.demo.linter.controls
48+
*/
49+
class Example3 extends Control {
50+
static readonly metadata: MetadataOptions = {}
51+
52+
rerender = () => {
53+
console.log("Overriding rerender method without calling super method");
54+
}
55+
56+
static renderer = {
57+
apiVersion: 2,
58+
render: function(rm: RenderManager, control: Example3) {
59+
rm.openStart("div", control);
60+
rm.openEnd();
61+
rm.close("div");
62+
}
63+
}
64+
}
65+
66+
/**
67+
* @namespace sap.ui.demo.linter.controls
68+
*/
69+
class Example4 extends Control {
70+
static readonly metadata: MetadataOptions = {}
71+
72+
rerender = function() {
73+
console.log("Overriding rerender method without calling super method");
74+
}
75+
76+
static renderer = {
77+
apiVersion: 2,
78+
render: function(rm: RenderManager, control: Example4) {
79+
rm.openStart("div", control);
80+
rm.openEnd();
81+
rm.close("div");
82+
}
83+
}
84+
}
85+
86+
/**
87+
* @namespace sap.ui.demo.linter.controls
88+
*/
89+
class Example5 extends Control {
90+
static readonly metadata: MetadataOptions = {}
91+
92+
static renderer = {
93+
apiVersion: 2,
94+
render: function(rm: RenderManager, control: Example5) {
95+
rm.openStart("div", control);
96+
rm.openEnd();
97+
rm.close("div");
98+
}
99+
}
100+
}
101+
Example5.prototype.rerender = function() {
102+
console.log("Overriding rerender method without calling super method");
103+
};
104+
105+
function rerender() {
106+
console.log("Overriding rerender method without calling super method");
107+
}
108+
109+
/**
110+
* @namespace sap.ui.demo.linter.controls
111+
*/
112+
class Example6 extends Control {
113+
static readonly metadata: MetadataOptions = {}
114+
115+
rerender = rerender
116+
117+
static renderer = {
118+
apiVersion: 2,
119+
render: function(rm: RenderManager, control: Example6) {
120+
rm.openStart("div", control);
121+
rm.openEnd();
122+
rm.close("div");
123+
}
124+
}
125+
}

0 commit comments

Comments
 (0)