Skip to content

Commit

Permalink
feat: Detect override of control "rerender"
Browse files Browse the repository at this point in the history
JIRA: CPOUI5FOUNDATION-939
  • Loading branch information
matz3 committed Dec 4, 2024
1 parent 156e747 commit 46a3adb
Show file tree
Hide file tree
Showing 9 changed files with 564 additions and 1 deletion.
11 changes: 11 additions & 0 deletions src/linter/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export enum MESSAGE {
HTML_IN_XML,
LIB_INIT_API_VERSION,
MISSING_BOOTSTRAP_PARAM,
NO_CONTROL_RERENDER_OVERRIDE,
NO_DEPRECATED_RENDERER,
NO_DIRECT_DATATYPE_ACCESS,
NO_DIRECT_ENUM_ACCESS,
Expand Down Expand Up @@ -316,6 +317,16 @@ export const MESSAGE_INFO = {
details: () => `{@link sap.ui.core.Lib.init Lib.init}`,
},

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

[MESSAGE.NO_DEPRECATED_RENDERER]: {
severity: LintMessageSeverity.Error,
ruleId: RULES["no-deprecated-api"],
Expand Down
12 changes: 11 additions & 1 deletion src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import LinterContext, {ResourcePath, CoverageCategory} from "../LinterContext.js
import {MESSAGE} from "../messages.js";
import analyzeComponentJson from "./asyncComponentFlags.js";
import {deprecatedLibraries, deprecatedThemes} from "../../utils/deprecations.js";
import {getPropertyName, getSymbolForPropertyInConstructSignatures} from "./utils.js";
import {findClassInstanceMethod, getPropertyName, getSymbolForPropertyInConstructSignatures} from "./utils.js";
import {taskStart} from "../../utils/perf.js";
import {getPositionsForNode} from "../../utils/nodePosition.js";
import {TraceMap} from "@jridgewell/trace-mapping";
Expand Down Expand Up @@ -153,6 +153,7 @@ export default class SourceFileLinter {
ts.forEachChild(node, visitMetadataNodes);
} else if (this.isUi5ClassDeclaration(node, "sap/ui/core/Control")) {
this.analyzeControlRendererDeclaration(node);
this.analyzeControlRerenderMethod(node);
} else if (ts.isPropertyAssignment(node) && removeQuotes(node.name.getText()) === "theme") {
this.analyzeTestsuiteThemeProperty(node);
}
Expand Down Expand Up @@ -539,6 +540,15 @@ export default class SourceFileLinter {
}
}

analyzeControlRerenderMethod(node: ts.ClassDeclaration) {
const className = node.name?.getText() ?? "<unknown>";
const rerenderMethod = findClassInstanceMethod(node, "rerender", this.checker);
if (!rerenderMethod) {
return;
}
this.#reporter.addMessage(MESSAGE.NO_CONTROL_RERENDER_OVERRIDE, {className}, rerenderMethod);
}

analyzeMetadataProperty(type: string, node: ts.PropertyAssignment) {
const analyzeMetadataDone = taskStart(`analyzeMetadataProperty: ${type}`, this.resourcePath, true);
if (type === "interfaces") {
Expand Down
32 changes: 32 additions & 0 deletions src/linter/ui5Types/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,35 @@ export function getSymbolForPropertyInConstructSignatures(
}
return undefined;
}

export function findClassInstanceMethod(
node: ts.ClassDeclaration, methodName: string, checker: ts.TypeChecker
): ts.ClassElement | undefined {
return node.members.find((member) => {
if (!member.name) {
return false;
}
const name = getPropertyName(member.name);
if (name !== methodName) {
return false;
}
if (ts.isMethodDeclaration(member)) {
return true;
}
if (ts.isPropertyDeclaration(member)) {
if (!member.initializer) {
return false;
}
if (ts.isFunctionExpression(member.initializer) || ts.isArrowFunction(member.initializer)) {
return true;
};
if (ts.isIdentifier(member.initializer)) {
const symbol = checker.getSymbolAtLocation(member.initializer);
if (symbol?.valueDeclaration && ts.isFunctionDeclaration(symbol.valueDeclaration)) {
return true;
}
}
}
return false;
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ sap.ui.define(["./Element"], function(Element) {
library: "sap.ui.core",
},

// Should not be detected as declaration / override of a deprecated method
rerender: function() {},

renderer : null // Control has no renderer
});
return Control;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ sap.ui.define(["../base/ManagedObject"], function(ManagedObject) {
library : "sap.ui.core"
},

// Should not be detected as declaration / override of a deprecated method
rerender: function() {},

renderer : null // Element has no renderer
});
return Element;
Expand Down
122 changes: 122 additions & 0 deletions test/fixtures/linter/rules/renderer/ControlRerenderOverride.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
sap.ui.define(["sap/ui/core/Control"], function(Control) {
const Example1 = Control.extend("sap.ui.demo.linter.controls.Example1", {
metadata: {},

rerender: function() {
console.log("Overriding rerender method");
return Control.prototype.rerender.apply(this, arguments);
},

renderer: {
apiVersion: 2,
render: function(oRm, oControl) {
oRm.openStart("div", oControl);
oRm.openEnd();
oRm.close("div");
}
}
});

const Example2 = Control.extend("sap.ui.demo.linter.controls.Example2", {
metadata: {},

"rerender": function() {
console.log("Overriding rerender method without calling super method");
},

renderer: {
apiVersion: 2,
render: function(oRm, oControl) {
oRm.openStart("div", oControl);
oRm.openEnd();
oRm.close("div");
}
}
});

const Example3 = Control.extend("sap.ui.demo.linter.controls.Example3", {
metadata: {},

rerender: () => {
console.log("Overriding rerender method without calling super method");
},

renderer: {
apiVersion: 2,
render: function(oRm, oControl) {
oRm.openStart("div", oControl);
oRm.openEnd();
oRm.close("div");
}
}
});

const Example4 = Control.extend("sap.ui.demo.linter.controls.Example4", {
metadata: {},

rerender() {
console.log("Overriding rerender method without calling super method");
},

renderer: {
apiVersion: 2,
render: function(oRm, oControl) {
oRm.openStart("div", oControl);
oRm.openEnd();
oRm.close("div");
}
}
});

const Example5 = Control.extend("sap.ui.demo.linter.controls.Example5", {
metadata: {},

renderer: {
apiVersion: 2,
render: function(oRm, oControl) {
oRm.openStart("div", oControl);
oRm.openEnd();
oRm.close("div");
}
}
});
// TODO detect: Check why this override is currently not detected in JavaScript files.
// The same code is detected properly in ControlRerenderOverrideTypeScript.ts.
Example5.prototype.rerender = function() {
console.log("Overriding rerender method without calling super method");
};

function rerender() {
console.log("Overriding rerender method without calling super method");
}

const Example6 = Control.extend("sap.ui.demo.linter.controls.Example6", {
metadata: {},

rerender,

renderer: {
apiVersion: 2,
render: function(oRm, oControl) {
oRm.openStart("div", oControl);
oRm.openEnd();
oRm.close("div");
}
}
});

const Example7 = Control.extend("sap.ui.demo.linter.controls.Example7", {
metadata: {},

rerender: rerender,

renderer: {
apiVersion: 2,
render: function(oRm, oControl) {
oRm.openStart("div", oControl);
oRm.openEnd();
oRm.close("div");
}
}
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import Control from "sap/ui/core/Control";
import type { MetadataOptions } from "sap/ui/core/Element";
import RenderManager from "sap/ui/core/RenderManager";

/**
* @namespace sap.ui.demo.linter.controls
*/
class Example1 extends Control {
static readonly metadata: MetadataOptions = {}

rerender() {
console.log("Overriding rerender method");
return super.rerender();
}

static renderer = {
apiVersion: 2,
render: function(rm: RenderManager, control: Example1) {
rm.openStart("div", control);
rm.openEnd();
rm.close("div");
}
}
}

/**
* @namespace sap.ui.demo.linter.controls
*/
class Example2 extends Control {
static readonly metadata: MetadataOptions = {}

rerender() {
console.log("Overriding rerender method without calling super method");
}

static renderer = {
apiVersion: 2,
render: function(rm: RenderManager, control: Example2) {
rm.openStart("div", control);
rm.openEnd();
rm.close("div");
}
}
}

/**
* @namespace sap.ui.demo.linter.controls
*/
class Example3 extends Control {
static readonly metadata: MetadataOptions = {}

rerender = () => {
console.log("Overriding rerender method without calling super method");
}

static renderer = {
apiVersion: 2,
render: function(rm: RenderManager, control: Example3) {
rm.openStart("div", control);
rm.openEnd();
rm.close("div");
}
}
}

/**
* @namespace sap.ui.demo.linter.controls
*/
class Example4 extends Control {
static readonly metadata: MetadataOptions = {}

rerender = function() {
console.log("Overriding rerender method without calling super method");
}

static renderer = {
apiVersion: 2,
render: function(rm: RenderManager, control: Example4) {
rm.openStart("div", control);
rm.openEnd();
rm.close("div");
}
}
}

/**
* @namespace sap.ui.demo.linter.controls
*/
class Example5 extends Control {
static readonly metadata: MetadataOptions = {}

static renderer = {
apiVersion: 2,
render: function(rm: RenderManager, control: Example5) {
rm.openStart("div", control);
rm.openEnd();
rm.close("div");
}
}
}
Example5.prototype.rerender = function() {
console.log("Overriding rerender method without calling super method");
};

function rerender() {
console.log("Overriding rerender method without calling super method");
}

/**
* @namespace sap.ui.demo.linter.controls
*/
class Example6 extends Control {
static readonly metadata: MetadataOptions = {}

rerender = rerender

static renderer = {
apiVersion: 2,
render: function(rm: RenderManager, control: Example6) {
rm.openStart("div", control);
rm.openEnd();
rm.close("div");
}
}
}
Loading

0 comments on commit 46a3adb

Please sign in to comment.