diff --git a/src/linter/messages.ts b/src/linter/messages.ts index 95657621b..8049e6e6f 100644 --- a/src/linter/messages.ts +++ b/src/linter/messages.ts @@ -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, @@ -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"], diff --git a/src/linter/ui5Types/SourceFileLinter.ts b/src/linter/ui5Types/SourceFileLinter.ts index 26c45b725..e86c61420 100644 --- a/src/linter/ui5Types/SourceFileLinter.ts +++ b/src/linter/ui5Types/SourceFileLinter.ts @@ -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"; @@ -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); } @@ -539,6 +540,15 @@ export default class SourceFileLinter { } } + analyzeControlRerenderMethod(node: ts.ClassDeclaration) { + const className = node.name?.getText() ?? ""; + 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") { diff --git a/src/linter/ui5Types/utils.ts b/src/linter/ui5Types/utils.ts index 5aa2fcb08..dfa550bca 100644 --- a/src/linter/ui5Types/utils.ts +++ b/src/linter/ui5Types/utils.ts @@ -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; + }); +} diff --git a/test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/Control.js b/test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/Control.js index e21b1cc59..4849d8e44 100644 --- a/test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/Control.js +++ b/test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/Control.js @@ -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; diff --git a/test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/Element.js b/test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/Element.js index 07fc6c6aa..eee8501bf 100644 --- a/test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/Element.js +++ b/test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/Element.js @@ -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; diff --git a/test/fixtures/linter/rules/renderer/ControlRerenderOverride.js b/test/fixtures/linter/rules/renderer/ControlRerenderOverride.js new file mode 100644 index 000000000..3ee9311e0 --- /dev/null +++ b/test/fixtures/linter/rules/renderer/ControlRerenderOverride.js @@ -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"); + } + } + }); +}); diff --git a/test/fixtures/linter/rules/renderer/ControlRerenderOverrideTypeScript.ts b/test/fixtures/linter/rules/renderer/ControlRerenderOverrideTypeScript.ts new file mode 100644 index 000000000..f58548f0e --- /dev/null +++ b/test/fixtures/linter/rules/renderer/ControlRerenderOverrideTypeScript.ts @@ -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"); + } + } +} diff --git a/test/lib/linter/rules/snapshots/renderer.ts.md b/test/lib/linter/rules/snapshots/renderer.ts.md index ff4bd1f9f..2ba67a1a4 100644 --- a/test/lib/linter/rules/snapshots/renderer.ts.md +++ b/test/lib/linter/rules/snapshots/renderer.ts.md @@ -475,6 +475,263 @@ Generated by [AVA](https://avajs.dev). ], warningCount: 0, }, + { + coverageInfo: [ + { + category: 1, + column: 5, + line: 13, + message: 'Unable to analyze this method call because the type of identifier "openStart" in "oRm.openStart("div", oControl)"" could not be determined', + }, + { + category: 1, + column: 5, + line: 14, + message: 'Unable to analyze this method call because the type of identifier "openEnd" in "oRm.openEnd()"" could not be determined', + }, + { + category: 1, + column: 5, + line: 15, + message: 'Unable to analyze this method call because the type of identifier "close" in "oRm.close("div")"" could not be determined', + }, + { + category: 1, + column: 5, + line: 30, + message: 'Unable to analyze this method call because the type of identifier "openStart" in "oRm.openStart("div", oControl)"" could not be determined', + }, + { + category: 1, + column: 5, + line: 31, + message: 'Unable to analyze this method call because the type of identifier "openEnd" in "oRm.openEnd()"" could not be determined', + }, + { + category: 1, + column: 5, + line: 32, + message: 'Unable to analyze this method call because the type of identifier "close" in "oRm.close("div")"" could not be determined', + }, + { + category: 1, + column: 5, + line: 47, + message: 'Unable to analyze this method call because the type of identifier "openStart" in "oRm.openStart("div", oControl)"" could not be determined', + }, + { + category: 1, + column: 5, + line: 48, + message: 'Unable to analyze this method call because the type of identifier "openEnd" in "oRm.openEnd()"" could not be determined', + }, + { + category: 1, + column: 5, + line: 49, + message: 'Unable to analyze this method call because the type of identifier "close" in "oRm.close("div")"" could not be determined', + }, + { + category: 1, + column: 5, + line: 64, + message: 'Unable to analyze this method call because the type of identifier "openStart" in "oRm.openStart("div", oControl)"" could not be determined', + }, + { + category: 1, + column: 5, + line: 65, + message: 'Unable to analyze this method call because the type of identifier "openEnd" in "oRm.openEnd()"" could not be determined', + }, + { + category: 1, + column: 5, + line: 66, + message: 'Unable to analyze this method call because the type of identifier "close" in "oRm.close("div")"" could not be determined', + }, + { + category: 1, + column: 5, + line: 77, + message: 'Unable to analyze this method call because the type of identifier "openStart" in "oRm.openStart("div", oControl)"" could not be determined', + }, + { + category: 1, + column: 5, + line: 78, + message: 'Unable to analyze this method call because the type of identifier "openEnd" in "oRm.openEnd()"" could not be determined', + }, + { + category: 1, + column: 5, + line: 79, + message: 'Unable to analyze this method call because the type of identifier "close" in "oRm.close("div")"" could not be determined', + }, + { + category: 1, + column: 5, + line: 101, + message: 'Unable to analyze this method call because the type of identifier "openStart" in "oRm.openStart("div", oControl)"" could not be determined', + }, + { + category: 1, + column: 5, + line: 102, + message: 'Unable to analyze this method call because the type of identifier "openEnd" in "oRm.openEnd()"" could not be determined', + }, + { + category: 1, + column: 5, + line: 103, + message: 'Unable to analyze this method call because the type of identifier "close" in "oRm.close("div")"" could not be determined', + }, + { + category: 1, + column: 5, + line: 116, + message: 'Unable to analyze this method call because the type of identifier "openStart" in "oRm.openStart("div", oControl)"" could not be determined', + }, + { + category: 1, + column: 5, + line: 117, + message: 'Unable to analyze this method call because the type of identifier "openEnd" in "oRm.openEnd()"" could not be determined', + }, + { + category: 1, + column: 5, + line: 118, + message: 'Unable to analyze this method call because the type of identifier "close" in "oRm.close("div")"" could not be determined', + }, + ], + errorCount: 7, + fatalErrorCount: 0, + filePath: 'ControlRerenderOverride.js', + messages: [ + { + column: 3, + line: 5, + message: 'Override of deprecated method \'rerender\' in control \'Example1\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 11, + line: 7, + message: 'Use of deprecated property \'rerender\'', + messageDetails: 'Deprecated test message', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 3, + line: 23, + message: 'Override of deprecated method \'rerender\' in control \'Example2\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 3, + line: 40, + message: 'Override of deprecated method \'rerender\' in control \'Example3\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 3, + line: 57, + message: 'Override of deprecated method \'rerender\' in control \'Example4\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 3, + line: 96, + message: 'Override of deprecated method \'rerender\' in control \'Example6\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 3, + line: 111, + message: 'Override of deprecated method \'rerender\' in control \'Example7\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + ], + warningCount: 0, + }, + { + coverageInfo: [], + errorCount: 7, + fatalErrorCount: 0, + filePath: 'ControlRerenderOverrideTypeScript.ts', + messages: [ + { + column: 2, + line: 11, + message: 'Override of deprecated method \'rerender\' in control \'Example1\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 16, + line: 13, + message: 'Call to deprecated function \'rerender\' (super.rerender)', + messageDetails: 'Deprecated test message', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 2, + line: 32, + message: 'Override of deprecated method \'rerender\' in control \'Example2\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 2, + line: 52, + message: 'Override of deprecated method \'rerender\' in control \'Example3\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 2, + line: 72, + message: 'Override of deprecated method \'rerender\' in control \'Example4\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 1, + line: 101, + message: 'Use of deprecated property \'rerender\'', + messageDetails: 'Deprecated test message', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 2, + line: 115, + message: 'Override of deprecated method \'rerender\' in control \'Example6\'', + messageDetails: '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.', + ruleId: 'no-deprecated-api', + severity: 2, + }, + ], + warningCount: 0, + }, { coverageInfo: [], errorCount: 3, diff --git a/test/lib/linter/rules/snapshots/renderer.ts.snap b/test/lib/linter/rules/snapshots/renderer.ts.snap index 5d71d97c6..0e1f909bc 100644 Binary files a/test/lib/linter/rules/snapshots/renderer.ts.snap and b/test/lib/linter/rules/snapshots/renderer.ts.snap differ